Re: [RFC 00/12] Merge value optimized_out and unavailable

[Pedro Alves <palves@redhat.com>]

On 08/12/2013 01:15 PM, Andrew Burgess wrote:
> This patch set merges together how gdb handles values that are
> optimized out and values that are unavailable.
> 
> I think that in most cases gdb should not care why the contents of
> a value are not fetch-able, it is only when we need to display
> something to the user that we should have to figure out was this
> optimized-out or unavailable?

Kind of, but not quite.  Going through the series, and thinking about
it a lot, I'm not convinced the parts that handle unavailable and
optimized out errors the same way are right.  The frame machinery
handles unavailable registers especially, with the _if_available
wrappers, because it's possible to have a traceframe with no
collected PC, or trimmed core file with no registers info,
and consequently, a frame #0 with an <unavailable> PC / function.
I'm not seeing how it's possible to end up with a frame_info that
has an optimized out PC.  If unwinding the PC results in a not
saved PC, we use that as indication that we've reached the
outermost frame, so we stop the backtrace before that could happen.

So all the _if_available functions that _don't_ try to unwind the
prev frame (which are most, except frame_unwind_caller_pc_if_available),
shouldn't ever throw an optimized error for PC/function.

Since frame_unwind_caller_pc_if_available is the only
_if_available wrapper that unwinds, that's the case where the
caller would need to likewise handle optimized out / not saved
PCs.  But then, after the series, "info frame", the only caller
of frame_unwind_caller_pc_if_available, ends printing <unavailable>
instead of <not saved>, because these different errors passed
through the same sieve hole:

  if (frame_unwind_caller_pc_if_available (fi, &caller_pc))
    fputs_filtered (paddress (gdbarch, caller_pc), gdb_stdout);
  else
    fputs_filtered ("<unavailable>", gdb_stdout);

(gdb) info frame
Stack level 2, frame at 0x0:
 rip = 0x323d4f168d in clone (../sysdeps/unix/sysv/linux/x86_64/clone.S:115); saved rip <unavailable>
 Outermost frame: outermost
 caller of frame at 0x7ffff7fcafc0
 source language asm.
 Arglist at 0x7ffff7fcafb8, args:
 Locals at 0x7ffff7fcafb8, Previous frame's sp is 0x7ffff7fcafc8
(gdb)

Another place showing the issue with "merging" the errors
is here:

 static CORE_ADDR
 frame_unwind_pc (struct frame_info *this_frame)
 {
   CORE_ADDR pc;

   if (!frame_unwind_pc_if_available (this_frame, &pc))
     throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));
   else
     return pc;
 }

This throws the wrong error with the wrong string if
frame_unwind_pc_if_available returned false due to
OPTIMIZED_OUT_ERROR.

So for that part of the series, I'd rather not go around and
sprinkle the is_unavailable_error wrapper function wherever
we use catch NOT_AVAILABLE_ERROR, but instead handle
OPTIMIZED_OUT_ERROR as needed.

I've posted a mini series that fixes the "info frame" case
above:

  https://sourceware.org/ml/gdb-patches/2013-11/msg00943.html

(I had actually tried to fix that before by making
frame_unwind_caller_pc return struct value, and gave up as that
grew quite messy quick.  Making use of your new OPTIMIZED_OUT_ERROR
ends up much simpler.)

> After this patch set there will be a single unified interface to ask
> if a value is available (either fully, partially, or for a range of
> bit/bytes), this will answer in terms of both optimized out and
> unavailable state.

On terminology: I'd much rather not overload the "available/unavailable"
words for this. It'll end up confusing, like "This value is
not available, because it was unavailable?  No, because it
was optimized out.".  Etc.

Thanks,
-- 
Pedro Alves