From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14703 invoked by alias); 10 May 2006 18:03:20 -0000 Received: (qmail 14695 invoked by uid 22791); 10 May 2006 18:03:19 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Wed, 10 May 2006 18:03:14 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1Fdt1k-0003K2-H8 for gdb-patches@sourceware.org; Wed, 10 May 2006 14:03:12 -0400 Date: Wed, 10 May 2006 18:03:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: [RFC] Move the frame zero PC check earlier Message-ID: <20060510180312.GA12606@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.8i X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-05/txt/msg00196.txt.bz2 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 * 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); }