From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12064 invoked by alias); 6 Jun 2013 19:35:09 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 12053 invoked by uid 89); 6 Jun 2013 19:35:09 -0000 X-Spam-SWARE-Status: No, score=-6.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 06 Jun 2013 19:35:08 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r56JZ7Bc018876 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 6 Jun 2013 15:35:07 -0400 Received: from barimba (ovpn-113-72.phx2.redhat.com [10.3.113.72]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r56JZ55C023438 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 6 Jun 2013 15:35:05 -0400 From: Tom Tromey To: gdb-patches@sourceware.org Subject: RFC: fix PR backtrace/15558 Date: Thu, 06 Jun 2013 19:35:00 -0000 Message-ID: <87li6nghhz.fsf@fleche.redhat.com> MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-06/txt/msg00133.txt.bz2 PR backtrace/15558 concerns an assertion failure in gdb that can be triggered by setting "backtrace limit" to a value that causes gdb to stop unwinding in the middle of a series of inlined frames. In this case, an assertion in inline_frame_this_id will trigger. The bug is essentially that get_prev_frame checks backtrace_limit. However, this is not needed or desirable in most cases. E.g., the Python API to unwinding should probably not be limited by this setting. This patch removes the check from get_prev_frame and introduces a new checking function that is used when printing a stack trace. Pedro had suggested removing all the other checks from get_prev_frame. However, I am not at all confident that this would be correct for all unwinders. E.g., do they all know how to stop when the stack is exhausted? Advice solicited. Meanwhile the appended shows the bug -- the test case is a bit odd, but it was the simplest way I knew of to guarantee that the problem would be seen -- and the fix is reasonably minimal. Built and regtested on x86-64 Fedora 18. Tom 2013-06-06 Tom Tromey PR backtrace/15558: * frame.c (backtrace_limit): No longer 'static'. (get_prev_frame): Don't check backtrace_limit. (get_prev_frame_limited): New function. * frame.h (get_prev_frame_limited, backtrace_limit): Declare. * mi/mi-cmd-stack.c (mi_cmd_stack_list_frames) (mi_cmd_stack_info_depth, mi_cmd_stack_list_args): Use get_prev_frame_limited. * python/py-framefilter.c (py_print_frame): Check frame_relative_level. * stack.c (backtrace_command_1): Use get_prev_frame_limited. 2013-06-06 Tom Tromey * gdb.python/py-frame-inline.c (z, y, x, w): New functions. (main): Call w. * gdb.python/py-frame-inline.exp: Add test. diff --git a/gdb/frame.c b/gdb/frame.c index d52c26a..a9dc820 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -265,7 +265,7 @@ show_backtrace_past_entry (struct ui_file *file, int from_tty, value); } -static unsigned int backtrace_limit = UINT_MAX; +unsigned int backtrace_limit = UINT_MAX; static void show_backtrace_limit (struct ui_file *file, int from_tty, struct cmd_list_element *c, const char *value) @@ -1985,17 +1985,6 @@ get_prev_frame (struct frame_info *this_frame) return NULL; } - /* If the user's backtrace limit has been exceeded, stop. We must - add two to the current level; one of those accounts for backtrace_limit - being 1-based and the level being 0-based, and the other accounts for - the level of the new frame instead of the level of the current - frame. */ - if (this_frame->level + 2 > backtrace_limit) - { - frame_debug_got_null_frame (this_frame, "backtrace limit exceeded"); - return NULL; - } - /* If we're already inside the entry function for the main objfile, then it isn't valid. Don't apply this test to a dummy frame - dummy frame PCs typically land in the entry func. Don't apply @@ -2044,6 +2033,25 @@ get_prev_frame (struct frame_info *this_frame) return get_prev_frame_1 (this_frame); } +/* A wrapper for get_prev_frame that respects the backtrace limit. */ + +struct frame_info * +get_prev_frame_limited (struct frame_info *this_frame) +{ + /* If the user's backtrace limit has been exceeded, stop. We must + add two to the current level; one of those accounts for backtrace_limit + being 1-based and the level being 0-based, and the other accounts for + the level of the new frame instead of the level of the current + frame. */ + if (this_frame->level + 2 > backtrace_limit) + { + frame_debug_got_null_frame (this_frame, "backtrace limit exceeded"); + return NULL; + } + + return get_prev_frame (this_frame); +} + CORE_ADDR get_frame_pc (struct frame_info *frame) { diff --git a/gdb/frame.h b/gdb/frame.h index 31b9cb7..59e68d6 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -275,6 +275,9 @@ extern void select_frame (struct frame_info *); extern struct frame_info *get_prev_frame (struct frame_info *); extern struct frame_info *get_next_frame (struct frame_info *); +/* Like get_prev_frame, but respects the backtrace limit. */ +extern struct frame_info *get_prev_frame_limited (struct frame_info *); + /* Given a frame's ID, relocate the frame. Returns NULL if the frame is not found. */ extern struct frame_info *frame_find_by_id (struct frame_id id); @@ -683,6 +686,7 @@ extern const char print_entry_values_both[]; extern const char print_entry_values_compact[]; extern const char print_entry_values_default[]; extern const char *print_entry_values; +extern unsigned int backtrace_limit; /* Inferior function parameter value read in from a frame. */ diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c index 4b21015..4cf2ffc 100644 --- a/gdb/mi/mi-cmd-stack.c +++ b/gdb/mi/mi-cmd-stack.c @@ -133,7 +133,7 @@ mi_cmd_stack_list_frames (char *command, char **argv, int argc) displaying, or if frame_low is 0. */ for (i = 0, fi = get_current_frame (); fi && i < frame_low; - i++, fi = get_prev_frame (fi)); + i++, fi = get_prev_frame_limited (fi)); if (fi == NULL) error (_("-stack-list-frames: Not enough frames in stack.")); @@ -164,7 +164,7 @@ mi_cmd_stack_list_frames (char *command, char **argv, int argc) frames in the stack. */ for (; fi && (i <= frame_high || frame_high == -1); - i++, fi = get_prev_frame (fi)) + i++, fi = get_prev_frame_limited (fi)) { QUIT; /* Print the location and the address always, even for level 0. @@ -195,7 +195,7 @@ mi_cmd_stack_info_depth (char *command, char **argv, int argc) for (i = 0, fi = get_current_frame (); fi && (i < frame_high || frame_high == -1); - i++, fi = get_prev_frame (fi)) + i++, fi = get_prev_frame_limited (fi)) QUIT; ui_out_field_int (current_uiout, "depth", i); @@ -283,7 +283,7 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc) displaying, or if frame_low is 0. */ for (i = 0, fi = get_current_frame (); fi && i < frame_low; - i++, fi = get_prev_frame (fi)); + i++, fi = get_prev_frame_limited (fi)); if (fi == NULL) error (_("-stack-list-arguments: Not enough frames in stack.")); @@ -315,7 +315,7 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc) frames in the stack. */ for (; fi && (i <= frame_high || frame_high == -1); - i++, fi = get_prev_frame (fi)) + i++, fi = get_prev_frame_limited (fi)) { struct cleanup *cleanup_frame; diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c index d62c596..747471c 100644 --- a/gdb/python/py-framefilter.c +++ b/gdb/python/py-framefilter.c @@ -1037,6 +1037,12 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type, if (frame == NULL) goto error; + if (frame_relative_level (frame) + 1> backtrace_limit) + { + do_cleanups (cleanup_stack); + return PY_BT_COMPLETED; + } + TRY_CATCH (except, RETURN_MASK_ALL) { gdbarch = get_frame_arch (frame); diff --git a/gdb/stack.c b/gdb/stack.c index a4b392e..53a33bc 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -1687,7 +1687,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters, while (current && count--) { QUIT; - current = get_prev_frame (current); + current = get_prev_frame_limited (current); } /* Will stop when CURRENT reaches the top of the stack. @@ -1695,8 +1695,8 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters, while (current) { QUIT; - trailing = get_prev_frame (trailing); - current = get_prev_frame (current); + trailing = get_prev_frame_limited (trailing); + current = get_prev_frame_limited (current); trailing_level++; } @@ -1722,7 +1722,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters, people have strong opinions against reading symbols for backtrace this may have to be an option. */ i = count; - for (fi = trailing; fi != NULL && i--; fi = get_prev_frame (fi)) + for (fi = trailing; fi != NULL && i--; fi = get_prev_frame_limited (fi)) { CORE_ADDR pc; @@ -1755,7 +1755,9 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters, "no-filters" has been specified from the command. */ if (no_filters || result == PY_BT_NO_FILTERS) { - for (i = 0, fi = trailing; fi && count--; i++, fi = get_prev_frame (fi)) + for (i = 0, fi = trailing; + fi && count--; + i++, fi = get_prev_frame_limited (fi)) { QUIT; diff --git a/gdb/testsuite/gdb.python/py-frame-inline.c b/gdb/testsuite/gdb.python/py-frame-inline.c index 68b93db..eda5ec4 100644 --- a/gdb/testsuite/gdb.python/py-frame-inline.c +++ b/gdb/testsuite/gdb.python/py-frame-inline.c @@ -36,8 +36,34 @@ g (void) return f (); } +inline __attribute__((__always_inline__)) int +z (void) +{ + return f (); +} + +inline __attribute__((__always_inline__)) int +y (void) +{ + return z (); +} + +inline __attribute__((__always_inline__)) int +x (void) +{ + return y (); +} + +inline __attribute__((__always_inline__)) int +w (void) +{ + return x (); +} + int main (void) { - return g (); + int x = g (); + x += w (); + return x; } diff --git a/gdb/testsuite/gdb.python/py-frame-inline.exp b/gdb/testsuite/gdb.python/py-frame-inline.exp index 00978b0..386e2d5 100644 --- a/gdb/testsuite/gdb.python/py-frame-inline.exp +++ b/gdb/testsuite/gdb.python/py-frame-inline.exp @@ -37,3 +37,12 @@ gdb_test "info frame" "inlined into frame 1\r\n.*" gdb_test "up" "#1 g .*" gdb_test "python print (gdb.selected_frame().read_var('l'))" "\r\n42" + +# A regression test for having unwinding stop in the middle of a +# sequence of inlined frames. This test case is tricky due to the +# conditions under which it can be reproduced: it needs at least 3 +# inlined frames and gdb needs to try to compute the frame_id. +# See PR backtrace/15558. +gdb_continue_to_breakpoint "Block break here." +gdb_test_no_output "set backtrace limit 1" +gdb_test "python print (gdb.newest_frame())" ".*"