From patchwork Tue Jun 26 19:02:01 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 28059 Received: (qmail 26545 invoked by alias); 26 Jun 2018 19:02:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 26536 invoked by uid 89); 26 Jun 2018 19:02:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_STOCKGEN, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=homes, U*palves, sk:palves, sk:palves@ X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 26 Jun 2018 19:02:05 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4B118402178A; Tue, 26 Jun 2018 19:02:04 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0D5692026D6A; Tue, 26 Jun 2018 19:02:01 +0000 (UTC) Subject: Re: [pushed] Change inline frame breakpoint skipping logic (fix gdb.gdb/selftest.exp) To: Joel Brobecker References: <20180612150620.wloegrt5dgpdugi2@localhost.localdomain> <58a758b7-e4b4-4fee-c7cb-be5a2ab344e6@suse.de> <8c7ace90-f0b8-56f0-0033-5b7827796037@redhat.com> <20180625210436.GA11382@adacore.com> Cc: Tom de Vries , gdb-patches@sourceware.org, Keith Seitz From: Pedro Alves Message-ID: <28957003-ed3e-68d3-6002-71e5e5996204@redhat.com> Date: Tue, 26 Jun 2018 20:02:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180625210436.GA11382@adacore.com> On 06/25/2018 10:04 PM, Joel Brobecker wrote: > Hello, > >> gdb/ChangeLog: >> 2018-06-19 Pedro Alves >> >> * inline-frame.c (stopped_by_user_bp_inline_frame): Replace PC >> parameter with a block parameter. Compare location's block symbol >> with the frame's block instead of addresses. >> (skip_inline_frames): Pass the current block instead of the >> frame's address. Break out as soon as we determine the frame >> should not be skipped. >> >> gdb/testsuite/ChangeLog: >> 2018-06-19 Pedro Alves >> >> * gdb.opt/inline-break.c (func_inline_callee, func_inline_caller) >> (func_extern_caller): New. >> (main): Call func_extern_caller. >> * gdb.opt/inline-break.exp: Add tests for inline frame skipping >> logic change. > > it looks like this patch is causing a crash with the following > example program: > > $ cat -n r.h > 1 /* r.h */ > 2 int counter = 42; > 3 > 4 inline void > 5 callee () { > 6 counter = 0; /* break here */ > 7 } > $ cat -n r.c > 1 /* r.c */ > 2 #include "r.h" > 3 > 4 int > 5 main () > 6 { > 7 callee (); > 8 } > > I compiled it using the following commands: > > $ gcc -c -g -O2 r.c > $ gcc -o r r.o > > Then, trying to put a breakpoint on r.h:6 (inside "callee") causes > a SEGV for me: > > $ gdb -q r > Reading symbols from r...done. > (gdb) b r.h:6 > Breakpoint 1 at 0x4003c0: file r.h, line 6. > (gdb) run > Starting program: /[...]/r > [1] 75618 segmentation fault /[...]/gdb -q r > > Prior to this commit, the behavior is the following for the "run" > command: > > (gdb) run > Starting program: /[...]/r > > Breakpoint 1, callee () at r.h:6 > 6 counter = 0; /* break here */ > > The problem occurs because we apparently assume that a bp_location's > symbols is not NULL: > > Program received signal SIGSEGV, Segmentation fault. > 0x00000000006f42bb in stopped_by_user_bp_inline_frame ( > stop_chain=, frame_block=) > at /homes/brobecke/act/gdb/gdb-head/gdb/inline-frame.c:305 > 305 && frame_block == SYMBOL_BLOCK_VALUE (loc->symbol)) > (gdb) p loc->symbol > $1 = (const symbol *) 0x0 Whoops. I remember actually thinking about loc->symbol potentially being null, but somehow convinced myself that SYMBOL_BLOCK_VALUE would return null in that case... :-P > > I don't know yet whether that's a valid assumption or something > occurred earlier in the process. Any thoughts on this before I start > looking deeper? > > I'm using a version of GCC 7.3.1 on x86_64-linux if anyone wants to > reproduce. If we just add a "loc->symbol != NULL" check, then we end up presenting the stop at the caller of the inline function, where the inline function was inlined, which is not what we want, since that's not where the user set the breakpoint. Recording the symbol in the location (it is copied from the sal that linespec.c creates into the location by add_location_to_breakpoint), like in the patch below, makes gdb present the stop at the "break here" line successfully. I tried to come up with a more complicated testcase that included more nested blocks inside the inlined function, to see if we would need to record the inner inlined block in the sal instead of the function's symbol (does that actually make sense for inlined functions?), but all I came up with worked with this patch as is. So maybe we can defer thinking about that until we find a testcase? From ac746927c3a8078098729dc3256010b2c1e617f8 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 26 Jun 2018 19:14:41 +0100 Subject: [PATCH] inline --- gdb/inline-frame.c | 1 + gdb/linespec.c | 1 + 2 files changed, 2 insertions(+) diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c index 896b0004e4a..3d07f8d0970 100644 --- a/gdb/inline-frame.c +++ b/gdb/inline-frame.c @@ -302,6 +302,7 @@ stopped_by_user_bp_inline_frame (const block *frame_block, bpstat stop_chain) if ((t == bp_loc_software_breakpoint || t == bp_loc_hardware_breakpoint) + && loc->symbol != nullptr && frame_block == SYMBOL_BLOCK_VALUE (loc->symbol)) return true; } diff --git a/gdb/linespec.c b/gdb/linespec.c index ae0200b8133..93e66c389f7 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -2196,6 +2196,7 @@ create_sals_line_offset (struct linespec_state *self, if (self->funfirstline) skip_prologue_sal (&intermediate_results[i]); + intermediate_results[i].symbol = sym; add_sal_to_sals (self, &values, &intermediate_results[i], sym ? SYMBOL_NATURAL_NAME (sym) : NULL, 0); }