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


> Date: Wed, 10 May 2006 14:03:12 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> 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.

And this is exactly the reason why things are done the way they are
done.  People should accept that the unwinder can fail, and we should
provide a way to indicate this.  There are many ways in which an
unwinder can fail and they're not always detectable.  One very
important scemario is where your program is thrashing the stack,
overwriting the return address.  We absulutely need to provide the
user some indication that something is wrong.  Currently this is the
extra frame we're printing.

> On the other hand, that third frame is ugly and generally useless

While the third frame itself doesn't provide any useful information,
its presence is very useful, since it indicates that GDB has lost
track..

> 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.

Our goal shouldn't be nicer looking backtraces.  It should be
providing the user with all information needed to fix bugs in their
programs.  Your patch is removing such a bit of information, and
therefore unacceptable to me.  Sorry :(.

Mark


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