Message ID | 20161026002153.1291bd6e@pinnacle.lan |
---|---|
State | New, archived |
Headers |
Received: (qmail 21942 invoked by alias); 26 Oct 2016 07:22:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 21912 invoked by uid 89); 26 Oct 2016 07:21:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.2 spammy=H*M:lan, imposed, hunk, H*F:D*to X-HELO: emailserver1.aplushosting.com Received: from emailserver1.asdf456.com (HELO emailserver1.aplushosting.com) (72.18.207.136) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with SMTP; Wed, 26 Oct 2016 07:21:57 +0000 Received: (qmail 2441 invoked by uid 0); 26 Oct 2016 07:21:54 -0000 Received: from unknown (HELO pinnacle.lan) (70.176.31.165) by emailserver1.asdf456.com with SMTP; Wed, 26 Oct 2016 00:21:54 -0700 Date: Wed, 26 Oct 2016 00:21:53 -0700 From: Kevin Buettner <kevin@buettner.to> To: gdb-patches@sourceware.org Cc: Pedro Alves <palves@redhat.com> Subject: Re: [PATCH 1/4] Distinguish sentinel frame from null frame Message-ID: <20161026002153.1291bd6e@pinnacle.lan> In-Reply-To: <33d72255-944f-50bd-301c-f1365efcd850@redhat.com> References: <20160928014455.438266a2@pinnacle.lan> <20160928014930.076de446@pinnacle.lan> <33d72255-944f-50bd-301c-f1365efcd850@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit |
Commit Message
Kevin Buettner
Oct. 26, 2016, 7:21 a.m. UTC
On Wed, 12 Oct 2016 14:07:13 +0100 Pedro Alves <palves@redhat.com> wrote: > > @@ -323,6 +334,8 @@ fprint_frame_id (struct ui_file *file, struct frame_id id) > > fprintf_unfiltered (file, "!stack"); > > else if (id.stack_status == FID_STACK_UNAVAILABLE) > > fprintf_unfiltered (file, "stack=<unavailable>"); > > + else if (id.stack_status == FID_STACK_SENTINEL) > > + fprintf_unfiltered (file, "stack=<sentinel>"); > > else > > fprintf_unfiltered (file, "stack=%s", hex_string (id.stack_addr)); > > fprintf_unfiltered (file, ","); > > @@ -511,6 +524,9 @@ get_frame_id (struct frame_info *fi) > > if (fi == NULL) > > return null_frame_id; > > > > + if (fi == sentinel_frame) > > + return sentinel_frame_id; > > + > > Why do you need this? When the sentinel frame is created, it's > given a frame id already: You're right - this isn't needed. I've removed it from a new version of the patch which I'm preparing. > > /* Cache for frame addresses already read by gdb. Valid only while > > inferior is stopped. Control variables for the frame cache should > > be local to this module. */ > > @@ -1511,6 +1527,9 @@ static struct frame_info *get_prev_frame_always_1 (struct frame_info *this_frame > > struct frame_info * > > get_current_frame (void) > > { > > + struct frame_info *current_frame; > > + int new_sentinel_p = 0; > > + /* Set the current frame before computing the frame id, to avoid > > + recursion inside compute_frame_id, in case the frame's > > + unwinder decides to do a symbol lookup (which depends on the > > + selected frame's block). > > + > > + This call must always succeed. In particular, nothing inside > > + get_prev_frame_always_1 should try to unwind from the > > + sentinel frame, because that could fail/throw, and we always > > + want to leave with the current frame created and linked in -- > > + we should never end up with the sentinel frame as outermost > > + frame. */ > > + current_frame = get_prev_frame_always_1 (sentinel_frame); > > + gdb_assert (current_frame != NULL); > > + > > + /* The call to get_frame_id, below, is not necessary when the stack is > > + well behaved. But when it's not well behaved, we want to ensure > > + that the frame id for the current frame is known (stashed) prior to > > + finding frame id values for any outer frames. > > + > > + The test gdb.dwarf2/dw2-dup-frame.exp shows an internal error > > + in the "backtrace from stop_frame" test when we don't do this here. */ > > + if (new_sentinel_p) > > + get_frame_id (current_frame); > > I don't understand this. I applied patch #1 locally with this bit > removed, and I don't see said internal error. > > Oh, this only happens when the following patches are applied. > > IMO, it's better to move this hunk to the patch that actually > creates the requirement for it, so that we have the whole > logical change in one patch. Can you elaborate more on > why the test causes the internal "backtrace from stop_frame" > error without this here? Here's the portion of the log file showing the failure/internal error: (gdb) break stop_frame Breakpoint 1 at 0x40059a: file dw2-dup-frame.c, line 22. (gdb) run Starting program: /mesquite2/sourceware-git/mesquite-native-python-unwind-2-split/bld/gdb/testsuite/outputs/gdb.dwarf2/dw2-dup-frame/dw2-dup-frame Breakpoint 1, stop_frame () at dw2-dup-frame.c:22 22 } (gdb) bt /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c:544: internal-error: frame_id get_frame_id(frame_info*): Assertion `stashed' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) FAIL: gdb.dwarf2/dw2-dup-frame.exp: backtrace from stop_frame (GDB internal error) Here's a partial backtrace from the internal error, showing the frames which I think are relevant, plus several extra to provide context: #0 internal_error ( file=0x932b98 "/ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c", line=544, fmt=0x932b20 "%s: Assertion `%s' failed.") at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/common/errors.c:54 #1 0x000000000072207e in get_frame_id (fi=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c:544 #2 0x00000000004eb50d in frame_info_to_frame_object (frame=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/python/py-frame.c:390 #3 0x00000000004ef5be in bootstrap_python_frame_filters (frame=0xe5a760, frame_low=0, frame_high=-1) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/python/py-framefilter.c:1453 #4 0x00000000004ef7a9 in gdbpy_apply_frame_filter ( extlang=0x8857e0 <extension_language_python>, frame=0xe5a760, flags=7, args_type=CLI_SCALAR_VALUES, out=0xf6def0, frame_low=0, frame_high=-1) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/python/py-framefilter.c:1548 #5 0x00000000005f2c5a in apply_ext_lang_frame_filter (frame=0xe5a760, flags=7, args_type=CLI_SCALAR_VALUES, out=0xf6def0, frame_low=0, frame_high=-1) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/extension.c:572 #6 0x00000000005ea896 in backtrace_command_1 (count_exp=0x0, show_locals=0, no_filters=0, from_tty=1) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/stack.c:1834 Examination of the code in frame_info_to_frame_object(), which is in python/py-frame.c, is key to understanding this problem: if (get_prev_frame (frame) == NULL && get_frame_unwind_stop_reason (frame) != UNWIND_NO_REASON && get_next_frame (frame) != NULL) { frame_obj->frame_id = get_frame_id (get_next_frame (frame)); frame_obj->frame_id_is_next = 1; } else { frame_obj->frame_id = get_frame_id (frame); frame_obj->frame_id_is_next = 0; } I will first note that the frame id for frame has not been computed yet. (This was verified by placing a breakpoint on compute_frame_id().) The call to get_prev_frame() causes the the frame id to (eventually) be computed for the previous frame. Here's a backtrace showing how we get there: #0 compute_frame_id (fi=0x10e2810) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c:496 #1 0x0000000000724a67 in get_prev_frame_if_no_cycle (this_frame=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c:1871 #2 0x0000000000725136 in get_prev_frame_always_1 (this_frame=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c:2045 #3 0x000000000072516b in get_prev_frame_always (this_frame=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c:2061 #4 0x000000000072570f in get_prev_frame (this_frame=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c:2303 #5 0x00000000004eb471 in frame_info_to_frame_object (frame=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/python/py-frame.c:381 For this particular case, we end up in the else clause of the code above which calls get_frame_id (frame). It's at this point that the frame id for frame is computed. Again, here's a backtrace: #0 compute_frame_id (fi=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c:496 #1 0x000000000072203d in get_frame_id (fi=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/frame.c:539 #2 0x00000000004eb50d in frame_info_to_frame_object (frame=0xe5a760) at /ironwood1/sourceware-git/mesquite-native-python-unwind-2-split/bld/../../binutils-gdb/gdb/python/py-frame.c:390 The test in question, dw2-dup-frame.exp, deliberately creates a broken (cyclic) stack. So, in this instance, the frame id for the prev `frame' will be the same as that for `frame'. But that particular frame id ended up in the stash during the previous frame operation. When, just a few lines later, we compute the frame id for `frame', the id in question is already in the stash, thus triggering the assertion failure. An alternate "fix" (or perhaps band-aid) for this problem is as follows: I'm not especially fond of this solution though. If it's necessary to create frame ids in a certain sequence, this ordering should be imposed elsewhere. Also, I've caught just one case here. There may also be similar problems lurking which we haven't caught yet. I hope you that you (or someone else) can suggest a better solution... Kevin
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c index 6bdac08..a1d305e 100644 --- a/gdb/python/py-frame.c +++ b/gdb/python/py-frame.c @@ -375,6 +375,10 @@ frame_info_to_frame_object (struct frame_info *frame) TRY { + /* Avoid case where the frame id for previous frame is computed + before that for the frame under consideration. */ + (void) get_frame_id (frame); + /* Try to get the previous frame, to determine if this is the last frame in a corrupt stack. If so, we need to store the frame_id of the next frame and not of this one (which is possibly invalid). */