From patchwork Thu Jan 25 14:53:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Toby Lloyd Davies X-Patchwork-Id: 84738 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id F1A37385842F for ; Thu, 25 Jan 2024 14:56:48 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by sourceware.org (Postfix) with ESMTPS id 27FFB3858D28 for ; Thu, 25 Jan 2024 14:56:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 27FFB3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=undo.io Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=undo.io ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 27FFB3858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::434 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706194582; cv=none; b=Hn/VpFK7b+I8sULSb7EKO/pidRVlLvm6ARlEhUicuPJTLFrOneDKvlQUWVkkROUbj9sHxGKrw8awxzkJbUdkH5E3f1HhuaW/YNEwKnk/FXbH2iIyqOIGRgzSYI8hFDPefoMpkFmDrxtc/6Y7ccixdHBKpyp7J2S7zLh767eD2ns= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706194582; c=relaxed/simple; bh=i/U1vWkldl1MXD4iaNm2FnbZoxsAKqioAfjRgNLku4g=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=gIBoZT+DCfQgEiJcL24nrI5nzzYyKKQ3DjHokQJ1MDkIAsTpN7hrBllxegWXkhf5yF1zApTn0pbUzEnr0y4CWy//oDNWk/mkabxAMuENRoM4daTJZWWU5M/uPIjW9f6lTcmCZUazWeckOGbiaa2qMhaSV10DPYvPYf/fFB7eIdY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-x434.google.com with SMTP id ffacd0b85a97d-3367a304091so6903266f8f.3 for ; Thu, 25 Jan 2024 06:56:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=undo.io; s=google2; t=1706194577; x=1706799377; darn=sourceware.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=VFWLL7JTSh9726X+LKe+VhDEA82qRj0+p60WFpEvd8E=; b=L8el6oEkv3yyPWWQLmX+NOfvENaZGqWrAMbAeeyVBui7w68pPP0wK5gKSyUGJzLM6X +rAUFphLhemAauFCTYQYOCipPUAJcfl4ndWq//XPORL+X73FpYCwC0ZoZ5q5FB+sPI3Y UPspIpeLBDlo4b8kk3+XkMCUjmwMQbbUmAv9Fnom9iZdW+zaNh4MpAwYbzcDOQVlihTU F5YfypAabiovqtbYG8MgDMtTJdIYCHf9GgsiP7tTITR6Rp4tq1Ve1Xd0mOWZeZxQjGBB ctLm7eofITy4Kw8kfLVYETRCYienjFg5RY5DvmbNVADL0JeXT6Azewc9OA5wIk6wVYsW dtVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706194577; x=1706799377; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=VFWLL7JTSh9726X+LKe+VhDEA82qRj0+p60WFpEvd8E=; b=tmXm1sXUcvnl0QISonPAeA4ptg+84Mh6V6Lx0up0Zg75PYnDnx1VtNkvWX1DwvL0Z6 S+Jcm0T3UImKILUzR363oPVnbBWupiG8ECb4IyWZf59ai3lWLVInxq3cvGtdW14JZuy7 eZu/G4G0ETbFT388Z7gzzXw1CeAl7fqukKRDFYgWMab0W4lxBKF9v1ePETSTytLa2vrF HaBdJ9jBL4kK/3wXoKxlzGtcws+W2fS0QtfcVTE4kc1oHI2N+Pzqff1u8ce+rH8WusKw yE5L4WiWmzw/6//5IH2sUpKgQ2RxYh95geUqJoNfZaWM0RpPlnnrwd41G0H+RgG760V7 a9Hg== X-Gm-Message-State: AOJu0YxOPaQ/hPkCvg2asx0+8xslxLgotRhG1QzMIq34r37rmwHedW9z OSKXsWwor8NdWZsvG/+vSTom08uTZdiSTupbIicxvf+hfWw6cNGF51Q2R52kz4vAtOz/97fNsDk iCd43jeELDKIbwt4q6eYu01H/HIX3nnCA+vshrGucXRw39wKRZo6+KksiYW97zYkmg5O310edVv zflyL35sDBe1C1hicsHgLNA+6E1xTo/U8AY6vqdm9ww2k= X-Google-Smtp-Source: AGHT+IE7PU5DRMZl46GCNg+20pKM1Tg4J911995oVecwMl/fCVKFy8Yi6OCP6yT3byw8ROEbF0Bgfw== X-Received: by 2002:adf:f549:0:b0:337:c582:c410 with SMTP id j9-20020adff549000000b00337c582c410mr506858wrp.73.1706194577247; Thu, 25 Jan 2024 06:56:17 -0800 (PST) Received: from redhawk-thinkpad.home (cfbb000029.b.cam.camfibre.uk. [185.219.110.154]) by smtp.gmail.com with ESMTPSA id e7-20020a5d4e87000000b00337d56264d4sm18141521wru.8.2024.01.25.06.56.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jan 2024 06:56:16 -0800 (PST) From: Toby Lloyd Davies To: gdb-patches@sourceware.org Cc: Toby Lloyd Davies Subject: [PATCH] gdb: Fix bps on inline callsites stopping in wrong frame Date: Thu, 25 Jan 2024 14:53:54 +0000 Message-Id: <20240125145354.1008724-1-tlloyddavies@undo.io> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Breakpoints on inline callsite lines were sometimes stopping in the wrong frame. This would happen when the callsite line contains no instructions. In this case GCC (but not Clang) will emit a line for the callsite which means that the user can set a breakpoint there. However, as the callsite contains no instructions it will be at the start of the inline function, and thus when getting the block for this PC GDB will get a block inside the inline function rather than in the caller function. This caused the breakpoint's symbol to be set to that of the innermost function. Instead the breakpoint's symbol should be set to the caller. This will then cause skip_inline_frames to correctly hide the inline frame leaving us at the callsite when we hit the breakpoint. This is achieved by modifying create_sals_line_offset. This processes linespecs created from user input into sals which are then used to create breakpoints. For every sal this function finds we check if it matches an inline callsite line at its PC. If it does we modify its block to the superblock of the called function's block. This results in the sal using the function symbol of the callsite rather than that of the innermost function. Note that GDB will currently only keep an empty callsite line if it is in the same source file as the inline function. When this is the case then the user can create a breakpoint at the callsite line, otherwise an attempt to create a breakpoint there will cause gdb to create a breakpoint at a different line. Tested-By: Guinevere Larsen --- gdb/blockframe.c | 26 +++ gdb/inline-frame.c | 14 +- gdb/inline-frame.h | 3 + gdb/linespec.c | 19 ++- gdb/symtab.h | 8 + .../gdb.dwarf2/dw2-inline-callsite-break.c | 43 +++++ .../gdb.dwarf2/dw2-inline-callsite-break.exp | 151 ++++++++++++++++++ 7 files changed, 257 insertions(+), 7 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-callsite-break.c create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-callsite-break.exp diff --git a/gdb/blockframe.c b/gdb/blockframe.c index 6076ad64a4a..a64409c53e2 100644 --- a/gdb/blockframe.c +++ b/gdb/blockframe.c @@ -477,3 +477,29 @@ block_innermost_frame (const struct block *block) return NULL; } + +const struct block * +find_sal_inline_callsite_block (const struct block *bl, + const struct symtab_and_line *sal) +{ + const block *cur_block = bl; + while (cur_block->superblock () != nullptr) + { + if (cur_block-> inlined_p ()) + { + if (inline_block_entry_point_at_pc (bl, sal->pc)) + { + if (sal->line == cur_block->function ()->line () + && sal->symtab == cur_block->function ()->symtab ()) + return cur_block; + } + else + break; + } + else if (cur_block->function () != nullptr) + break; + + cur_block = cur_block->superblock (); + } + return nullptr; +} diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c index 02dbcd12af1..8a218b6484c 100644 --- a/gdb/inline-frame.c +++ b/gdb/inline-frame.c @@ -339,6 +339,15 @@ stopped_by_user_bp_inline_frame (const block *frame_block, bpstat *stop_chain) /* See inline-frame.h. */ +bool +inline_block_entry_point_at_pc (const struct block *bl, CORE_ADDR pc) +{ + /* See comments in inline_frame_this_id about this use of BLOCK_ENTRY_PC. */ + return bl->entry_pc () == pc || block_starting_point_at (pc, bl); +} + +/* See inline-frame.h. */ + void skip_inline_frames (thread_info *thread, bpstat *stop_chain) { @@ -359,10 +368,7 @@ skip_inline_frames (thread_info *thread, bpstat *stop_chain) { if (cur_block->inlined_p ()) { - /* See comments in inline_frame_this_id about this use - of BLOCK_ENTRY_PC. */ - if (cur_block->entry_pc () == this_pc - || block_starting_point_at (this_pc, cur_block)) + if (inline_block_entry_point_at_pc (cur_block, this_pc)) { /* Do not skip the inlined frame if execution stopped in an inlined frame because of a user diff --git a/gdb/inline-frame.h b/gdb/inline-frame.h index e19b60e010f..3e9cc93d855 100644 --- a/gdb/inline-frame.h +++ b/gdb/inline-frame.h @@ -70,4 +70,7 @@ struct symbol *inline_skipped_symbol (thread_info *thread); int frame_inlined_callees (frame_info_ptr this_frame); +/* Return true if PC is at an entry point to inline block BL. */ +bool inline_block_entry_point_at_pc (const struct block *bl, CORE_ADDR pc); + #endif /* !defined (INLINE_FRAME_H) */ diff --git a/gdb/linespec.c b/gdb/linespec.c index 2c54ed55d93..a012458b51d 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -2129,9 +2129,22 @@ create_sals_line_offset (struct linespec_state *self, for (i = 0; i < intermediate_results.size (); ++i) if (filter[i]) { - struct symbol *sym = (blocks[i] - ? blocks[i]->containing_function () - : NULL); + if (blocks[i] != nullptr) + { + /* Check if this line matches any inline callsite at this pc. */ + const block *inline_block + = find_sal_inline_callsite_block (blocks[i], + &intermediate_results[i]); + if (inline_block != nullptr) + { + /* Use the superblock of the inline block. + block_containing_function will then give us the calling + function. */ + blocks[i] = inline_block->superblock (); + } + } + struct symbol *sym + = (blocks[i] ? blocks[i]->containing_function () : nullptr); if (self->funfirstline) skip_prologue_sal (&intermediate_results[i]); diff --git a/gdb/symtab.h b/gdb/symtab.h index 729c003f1c1..36252003b70 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -2261,6 +2261,14 @@ extern bool find_function_entry_range_from_pc (CORE_ADDR pc, CORE_ADDR *address, CORE_ADDR *endaddr); +/* Find if SAL is an inline callsite for block BL or any of its superblocks. + If it is return the inline block it is an inline callsite for. Otherwise + return nullptr. */ + +extern const struct block * +find_sal_inline_callsite_block (const struct block *bl, + const struct symtab_and_line *sal); + /* Return the type of a function with its first instruction exactly at the PC address. Return NULL otherwise. */ diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-callsite-break.c b/gdb/testsuite/gdb.dwarf2/dw2-inline-callsite-break.c new file mode 100644 index 00000000000..b2aa05d2538 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-callsite-break.c @@ -0,0 +1,43 @@ +/* Copyright 2019-2024 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +/* This test relies on both foo and bar being inlined into main. */ + +volatile int global_var; + +static inline int __attribute__ ((always_inline)) +bar () +{ /* bar prologue */ + asm ("bar_label: .globl bar_label"); + return global_var; /* bar return global_var */ +} /* bar end */ + +static inline int __attribute__ ((always_inline)) +foo () +{ /* foo prologue */ + return bar (); /* foo call bar */ +} /* foo end */ + +int +main () +{ /* main prologue */ + int ans; + asm ("main_label: .globl main_label"); + global_var = 0; /* main set global_var */ + asm ("main_label2: .globl main_label2"); + ans = foo (); /* main call foo */ + asm ("main_label3: .globl main_label3"); + return ans; +} /* main end */ diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-callsite-break.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-callsite-break.exp new file mode 100644 index 00000000000..95cdec64a69 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-callsite-break.exp @@ -0,0 +1,151 @@ +# Copyright 2019-2024 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test that breakpoints on empty inline callsite lines stop in the +# correct frame. + +load_lib dwarf.exp + +# This test can only be run on targets which support DWARF-2 and use gas. +if {![dwarf2_support]} { + verbose "Skipping $gdb_test_file_name." + return 0 +} + +# The .c files use __attribute__. +if ![is_c_compiler_gcc] { + verbose "Skipping $gdb_test_file_name." + return 0 +} + +standard_testfile .c .S + +set asm_file [standard_output_file $srcfile2] +Dwarf::assemble $asm_file { + global srcdir subdir srcfile srcfile2 + declare_labels ranges_label lines_label foo_prog bar_prog + + lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \ + main_start main_len + set main_end "$main_start + $main_len" + + set foo_call_line [gdb_get_line_number "main call foo"] + set bar_call_line [gdb_get_line_number "foo call bar"] + + cu {} { + compile_unit { + {language @DW_LANG_C} + {name dw2-inline-callsite-break.c} + {low_pc 0 addr} + {stmt_list ${lines_label} DW_FORM_sec_offset} + {ranges ${ranges_label} DW_FORM_sec_offset} + } { + foo_prog: subprogram { + {name foo} + {inline 3 data1} + } + bar_prog: subprogram { + {name bar} + {inline 3 data1} + } + subprogram { + {external 1 flag} + {name main} + {low_pc $main_start addr} + {high_pc "$main_start + $main_len" addr} + } { + inlined_subroutine { + {abstract_origin %$foo_prog} + {low_pc main_label2 addr} + {high_pc main_label3 addr} + {call_file 1 data1} + {call_line $foo_call_line data1} + } { + inlined_subroutine { + {abstract_origin %$bar_prog} + {low_pc main_label2 addr} + {high_pc main_label3 addr} + {call_file 1 data1} + {call_line $bar_call_line data1} + } + } + } + } + } + + lines {version 2} lines_label { + include_dir "${srcdir}/${subdir}" + file_name "$srcfile" 1 + + program { + DW_LNE_set_address $main_start + line [gdb_get_line_number "main prologue"] + DW_LNS_copy + + DW_LNE_set_address main_label + line [gdb_get_line_number "main set global_var"] + DW_LNS_copy + + DW_LNE_set_address main_label2 + line [gdb_get_line_number "main call foo"] + DW_LNS_copy + + DW_LNE_set_address main_label2 + line [gdb_get_line_number "foo call bar"] + DW_LNS_copy + + DW_LNE_set_address bar_label + line [gdb_get_line_number "bar return global_var"] + DW_LNS_copy + + DW_LNE_set_address $main_end + DW_LNE_end_sequence + } + } + + ranges {is_64 [is_64_target]} { + ranges_label: sequence { + range ${main_start} ${main_end} + } + } +} + +if { [prepare_for_testing "failed to prepare" ${testfile} \ + [list $srcfile $asm_file] {nodebug}] } { + return -1 +} + +if ![runto_main] { + return -1 +} + +set bp_location [gdb_get_line_number "main call foo"] +gdb_test "break $bp_location" ".*Breakpoint.*" \ + "set breakpoint on outer callsite line" + +gdb_test "continue" ".*Breakpoint.*main call foo.*" \ + "continue to outer callsite breakpoint" + +delete_breakpoints +if ![runto_main] { + return -1 +} + +set bp_location [gdb_get_line_number "foo call bar"] +gdb_test "break $bp_location" ".*Breakpoint.*" \ + "set breakpoint on inner callsite line" + +gdb_test "continue" ".*Breakpoint.*foo call bar.*" \ + "continue to inner callsite breakpoint"