This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] Move the frame zero PC check earlier


It looks good to me.  I'm just curious why we even bother looking up
the frame type for the older frame whose PC is zero.  (I understand
this test is inherited from the existing code.)

Daniel Jacobowitz <drow@false.org> writes:

> In this patch:
>
>   http://sourceware.org/ml/gdb-patches/2004-12/msg00328.html
>
> Andrew added a generic check for two "normal" frames in a row where the
> older one has a saved PC of zero.  This is pretty well understood as a
> convention for terminating backtraces - either intentionally or
> unintentionally.
>
> The problem is, given where that check takes place, it is in my opinion one
> frame off from the actual problem.  You get backtraces that look like this:
>
> (gdb) bt
> #0  catcher (sig=11) at /space/fsf/commit/src/gdb/testsuite/gdb.base/savedregs.c:43
> #1  0x00002ac148ec6e90 in killpg () from /lib/libc.so.6
> #2  0x0000000000000000 in ?? ()
>
> On the one hand, that third frame is a nice marker for this case in that it
> explains noisily that GDB is confused.  In this case, if I point GDB at
> .debug_frame data for my C library (conveniently found by default in
> /usr/lib/debug) it successfully backtraces through to main, so that's good.
>
> On the other hand, that third frame is ugly and generally useless.  The
> attached patch moves the check one frame earlier, so that we only get this
> backtrace:
>
> #0  catcher (sig=11) at /space/fsf/commit/src/gdb/testsuite/gdb.base/savedregs.c:43
> #1  0x00002ac148ec6e90 in killpg () from /lib/libc.so.6
>
> Benefits: cleaner looking backtraces; the check is only done once per frame
> instead of on every call to get_prev_frame.  Disadvantages: for all frames
> at level > 0, it causes this check to be done when we look at THIS frame
> instead of when we unwind to the PREV frame, which forces us to locate the
> correct sniffer earlier.  If you have a sigtramp sniffer which reads memory,
> this might cause an extra memory read.  However, it won't happen in the
> critical stepping path - we don't need this check when "unwinding frame 0
> from the sentinel frame - so I think this is an acceptable tradeoff.  For
> any frame other than the top frame, the thing you're most likely to do with
> it is backtrace right through it.
>
> Tested on x86_64-pc-linux-gnu, and by hand against SymbianOS, where it gives
> much nicer looking backtraces.
>
> Any comments?
>
> -- 
> Daniel Jacobowitz
> CodeSourcery
>
> 2006-05-10  Daniel Jacobowitz  <dan@codesourcery.com>
>
> 	* frame.c (get_prev_frame): Move check for pc == 0 ...
> 	(get_prev_frame_1): ... to here.
>
> Index: frame.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/frame.c,v
> retrieving revision 1.211
> diff -u -p -r1.211 frame.c
> --- frame.c	17 Dec 2005 22:33:59 -0000	1.211
> +++ frame.c	10 May 2006 17:48:32 -0000
> @@ -1123,6 +1123,26 @@ get_prev_frame_1 (struct frame_info *thi
>    this_frame->prev = prev_frame;
>    prev_frame->next = this_frame;
>  
> +  /* Now that the frame chain is in a consistant state, check whether
> +     this frame is useful.  If it is not, unlink it.  Its storage will
> +     be reclaimed the next time the frame cache is flushed, and we
> +     will not try to unwind THIS_FRAME again.  */
> +
> +  /* Assume that the only way to get a zero PC is through something
> +     like a SIGSEGV or a dummy frame, and hence that NORMAL frames
> +     will never unwind a zero PC.  This will look up the unwinder
> +     for the newly created frame, to determine its type.  */
> +  if (prev_frame->level > 0
> +      && get_frame_type (prev_frame) == NORMAL_FRAME
> +      && get_frame_type (this_frame) == NORMAL_FRAME
> +      && get_frame_pc (prev_frame) == 0)
> +    {
> +      if (frame_debug)
> +	fprintf_unfiltered (gdb_stdlog, "-> // zero PC}\n");
> +      this_frame->prev = NULL;
> +      return NULL;
> +    }
> +
>    if (frame_debug)
>      {
>        fprintf_unfiltered (gdb_stdlog, "-> ");
> @@ -1300,18 +1320,6 @@ get_prev_frame (struct frame_info *this_
>        return NULL;
>      }
>  
> -  /* Assume that the only way to get a zero PC is through something
> -     like a SIGSEGV or a dummy frame, and hence that NORMAL frames
> -     will never unwind a zero PC.  */
> -  if (this_frame->level > 0
> -      && get_frame_type (this_frame) == NORMAL_FRAME
> -      && get_frame_type (get_next_frame (this_frame)) == NORMAL_FRAME
> -      && get_frame_pc (this_frame) == 0)
> -    {
> -      frame_debug_got_null_frame (gdb_stdlog, this_frame, "zero PC");
> -      return NULL;
> -    }
> -
>    return get_prev_frame_1 (this_frame);
>  }
>  


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]