[RFC] Move the frame zero PC check earlier

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

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);
 }