RFC: libunwind-frame.c -vs- optimized-out

RFC: libunwind-frame.c -vs- optimized-out

[Tom Tromey]

PR 11780 concerns another case where the optimized-out change I made
breaks unwinding.  In this case the failures are on IA-64.

ia64_libunwind_frame_prev_register calls libunwind_frame_prev_register
and then uses the contents of the result, for example:

  /* Let libunwind do most of the work.  */
  val = libunwind_frame_prev_register (this_frame, this_cache, reg);
[...]
      prN_val = extract_bit_field (value_contents_all (val),
				   regnum - VP0_REGNUM, 1);
[...]


However, libunwind_frame_prev_register calls set_value_optimized_out on
this value.

I don't really understand why it is doing this.  It looks like the code
there is computing something, and strangely it seems though that the
error cases return a not-optimized-out value.  E.g., here, unw_get_reg_p
returns a negative value on failure:

            ret = unw_get_reg_p (&cache->cursor, uw_regnum, &intval);
            if (ret < 0)
              return frame_unwind_got_constant (this_frame, regnum, 0);
            val = frame_unwind_got_constant (this_frame, regnum, intval);
          }
        set_value_optimized_out (val, 1);

So, this seems like the logic is possibly backward.

The appended patch fixes the IA-64 regressions.  However, I am reluctant
to commit it without understanding what is going on here.

Tom

2010-07-02  Tom Tromey  <tromey@redhat.com>

	PR exp/11780:
	* libunwind-frame.c (libunwind_frame_prev_register): Don't set
	value as optimized-out.

Index: libunwind-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/libunwind-frame.c,v
retrieving revision 1.26
diff -u -r1.26 libunwind-frame.c
--- libunwind-frame.c	14 May 2010 23:41:04 -0000	1.26
+++ libunwind-frame.c	2 Jul 2010 19:34:58 -0000
@@ -348,7 +348,6 @@
               return frame_unwind_got_constant (this_frame, regnum, 0);
             val = frame_unwind_got_constant (this_frame, regnum, intval);
           }
-        set_value_optimized_out (val, 1);
         break;
       }
     }

Re: RFC: libunwind-frame.c -vs- optimized-out

[Jan Kratochvil]

On Fri, 02 Jul 2010 21:42:36 +0200, Tom Tromey wrote:
> So, this seems like the logic is possibly backward.

Yes, it also seems to me so.  Your change is an incoremental improvement.

If there are no regressions approving the check-in.


> The appended patch fixes the IA-64 regressions.  However, I am reluctant
> to commit it without understanding what is going on here.

	[commit] Update libunwind & ia64 unwinders
	From: Joel Brobecker
	http://sourceware.org/ml/gdb-patches/2008-05/msg00246.html

BTW I sometimes found it difficult myself that value >1< nullifies validity of
a value so I understand the ease of such bug.


Thanks,
Jan

Re: RFC: libunwind-frame.c -vs- optimized-out

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Yes, it also seems to me so.  Your change is an incoremental improvement.

Jan> If there are no regressions approving the check-in.

I will commit it shortly.

I compared this patch against the test results from a revision before
the optimized-out changes.  I had to do this by hand because in the
meantime there were a lot of test suite changes.

(And as an aside, our test suite is still very noisy: we have a lot of
tests named "set print sevenbit-strings", among other things.)

I also compared to HEAD.  Here the patch fixes a lot of regressions.
According to gcc/contrib/compare_tests it also causes:

gdb.mi/mi2-break.exp: list of breakpoints
gdb.mi/mi-break.exp: list of breakpoints
gdb.mi/mi-nonstop.exp: check varobj, w1, 1

... but all of these are duplicate test names and I think the script is
confused about PASS -vs- XFAIL.

(Also the last test there is probably misnamed.)

Tom