From patchwork Tue Jun 12 17:38:34 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 27756 Received: (qmail 11060 invoked by alias); 12 Jun 2018 17:38:39 -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 11049 invoked by uid 89); 12 Jun 2018 17:38:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No 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=Inline 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, 12 Jun 2018 17:38:37 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C604840201A3; Tue, 12 Jun 2018 17:38:35 +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 12B98111CB8C; Tue, 12 Jun 2018 17:38:34 +0000 (UTC) Subject: Change inline frame breakpoint skipping logic (Re: [PATCH] Ensure captured_main has unique address) To: Tom de Vries , gdb-patches@sourceware.org References: <20180612150620.wloegrt5dgpdugi2@localhost.localdomain> From: Pedro Alves Cc: Keith Seitz Message-ID: Date: Tue, 12 Jun 2018 18:38:34 +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: <20180612150620.wloegrt5dgpdugi2@localhost.localdomain> On 06/12/2018 04:06 PM, Tom de Vries wrote: > Hi, > > atm selftest.exp fails for me. > > One of the reasons is that after setting a breakpoint in captured_main, we > stop at: > ... > Breakpoint 1, captured_main_1 (context=) at src/gdb/main.c:492 > ... > while selftest_setup expects to stop at captured_main. > > The problem is that captured_main_1 has been inlined into captured_main, and > captured_main has been inlined into gdb_main: > ... > $ nm ./build/gdb/gdb | egrep ' [tT] .*captured_main|gdb_main' | c++filt > 000000000061b950 T gdb_main(captured_main_args*) > ... > > The reason that we seem to be stopping at inline function captured_main_1 has > probably something to do with commit "Don't elide all inlined frames", Yes, sounds like it. But the selftest.exp explicitly asks to stop at "captured_main", not "captured_main_1", so I'm thinking that it's gdb's behavior that might be wrong: (top-gdb) b captured_main Breakpoint 3 at 0x792f99: file src/gdb/main.c, line 492. (top-gdb) r Starting program: build/gdb/gdb Breakpoint 3, captured_main_1 (context=) at /home/pedro/gdb/binutils-gdb/src/gdb/main.c:492 492 lim_at_start = (char *) sbrk (0); (top-gdb) With the patch below, we instead get: (top-gdb) b captured_main Breakpoint 6 at 0x791339: file src/gdb/main.c, line 492. (top-gdb) r Starting program: build/gdb/gdb Breakpoint 6, captured_main (data=) at src/gdb/main.c:1147 1147 captured_main_1 (context); (top-gdb) and: (top-gdb) b captured_main_1 Breakpoint 7 at 0x791339: file src/gdb/main.c, line 492. (top-gdb) r Starting program: build/gdb/gdb Breakpoint 7, captured_main_1 (context=) at src/gdb/main.c:492 492 lim_at_start = (char *) sbrk (0); (top-gdb) Note that both captured_main and captured_main_1 resolved to the same address, 0x791339. The gdb.base/inline-break.exp testcase currently does not exercise that, but the new test added by the patch below does. That new test fails without the patch and passes with the patch. No regressions on x86-64 GNU/Linux. WDYT? Also, while looking at stopped_by_user_bp_inline_frame I noticed that comparing the THIS_PC is basically a nop -- if a software or hardware breakpoint explains the stop, then it must be that it was installed at the current PC. From cf6924a2cfca397b80c7c935e048771de77d3105 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 12 Jun 2018 18:22:25 +0100 Subject: [PATCH] Inline breakpoints gdb/ChangeLog: yyyy-mm-dd 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: yyyy-mm-dd Pedro Alves * gdb.opt/inline-break.c (func_callee, func_caller): New. (main): Call func_caller. --- gdb/inline-frame.c | 23 +++++++++++------------ gdb/testsuite/gdb.opt/inline-break.c | 21 +++++++++++++++++++++ gdb/testsuite/gdb.opt/inline-break.exp | 17 +++++++++++++++++ 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c index 1ac5835438d..3edd5b2b20b 100644 --- a/gdb/inline-frame.c +++ b/gdb/inline-frame.c @@ -286,11 +286,10 @@ block_starting_point_at (CORE_ADDR pc, const struct block *block) } /* Loop over the stop chain and determine if execution stopped in an - inlined frame because of a user breakpoint. THIS_PC is the current - frame's PC. */ + inlined frame because of a user breakpoint set at FRAME_BLOCK. */ static bool -stopped_by_user_bp_inline_frame (CORE_ADDR this_pc, bpstat stop_chain) +stopped_by_user_bp_inline_frame (const block *frame_block, bpstat stop_chain) { for (bpstat s = stop_chain; s != NULL; s = s->next) { @@ -301,9 +300,9 @@ stopped_by_user_bp_inline_frame (CORE_ADDR this_pc, bpstat stop_chain) bp_location *loc = s->bp_location_at; enum bp_loc_type t = loc->loc_type; - if (loc->address == this_pc - && (t == bp_loc_software_breakpoint - || t == bp_loc_hardware_breakpoint)) + if ((t == bp_loc_software_breakpoint + || t == bp_loc_hardware_breakpoint) + && frame_block == SYMBOL_BLOCK_VALUE (loc->symbol)) return true; } } @@ -340,12 +339,12 @@ skip_inline_frames (ptid_t ptid, bpstat stop_chain) { /* Do not skip the inlined frame if execution stopped in an inlined frame because of a user - breakpoint. */ - if (!stopped_by_user_bp_inline_frame (this_pc, stop_chain)) - { - skip_count++; - last_sym = BLOCK_FUNCTION (cur_block); - } + breakpoint for this inline function. */ + if (stopped_by_user_bp_inline_frame (cur_block, stop_chain)) + break; + + skip_count++; + last_sym = BLOCK_FUNCTION (cur_block); } else break; diff --git a/gdb/testsuite/gdb.opt/inline-break.c b/gdb/testsuite/gdb.opt/inline-break.c index 922102debb6..6ad331fcc63 100644 --- a/gdb/testsuite/gdb.opt/inline-break.c +++ b/gdb/testsuite/gdb.opt/inline-break.c @@ -176,6 +176,25 @@ not_inline_func3 (int x) return y + inline_func3 (x); } +/* A static inlined function that is called by another static inlined + function. */ + +static inline ATTR int +func_callee (int x) +{ + return x * 23; +} + +/* A static inlined function that calls another static inlined + function. The body of the function is a simple as possible so that + both functions are inlined to the same PC address. */ + +static int +func_caller (int x) +{ + return func_callee (x); +} + /* Entry point. */ int @@ -205,5 +224,7 @@ main (int argc, char *argv[]) x = not_inline_func3 (-21); + func_caller (1); + return x; } diff --git a/gdb/testsuite/gdb.opt/inline-break.exp b/gdb/testsuite/gdb.opt/inline-break.exp index 008ff1ac33a..f1ab3d23715 100644 --- a/gdb/testsuite/gdb.opt/inline-break.exp +++ b/gdb/testsuite/gdb.opt/inline-break.exp @@ -231,4 +231,21 @@ foreach_with_prefix cmd [list "break" "tbreak"] { } } +# func_caller and func_callee are both inline functions, and one calls +# the other. Test that setting a breakpoint on the caller reports the +# stop at the caller, and that setting a breakpoint at the callee +# reports a stop at the callee. +foreach_with_prefix func {"func_callee" "func_caller"} { + clean_restart $binfile + + if {![runto main]} { + untested "could not run to main" + continue + } + + gdb_breakpoint $func + gdb_test "continue" "Breakpoint .* $func .*at .*$srcfile.*" \ + "continue to inline function" +} + unset -nocomplain results