From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 420DE38582A5 for ; Thu, 1 Feb 2024 14:49:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 420DE38582A5 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 420DE38582A5 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706798979; cv=none; b=mp2p01xDaSGEWnlnYk7DtN5yDwbi4eR9e5lVi81oLgdCkUZ5qup0xq2P8GscvRgIqHrLBVvmTRpACH0ZyeS0dTZrDuQ94LnVVlvDIGnXq6RCglWy8MVyr2C4RnUy3abKqP29AUEU4stCxxPElIYKYwVFSYQY156l3ffeN1Bm0+Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706798979; c=relaxed/simple; bh=KK6lpRFJS7vxezrmzj7DRmBrCo2CX54J/cJOdhf6xRs=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=XQ2PNsv32o/Qf7Qqzz2OEbAgxkGxIsCV9JKHW7OjPvf5YVmIgn1/6IRK/PL1wLEXUZ+//v2SFUsnn7xufG9SSuG6pbnJun/KtSv1ffwmLhBhHUVCklYW3T7yXDGLmP9bLZ5PLtQ3H7QwE0e4LFitVAJPDdb6V0jSc0r3wY46lXc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706798976; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6VQkXZRTncDUNIlS0JxxujfbxN+wn/MTe5ZN74XA7bM=; b=S1w0Hz23RnCirB/JTr9NpGOsw2kWapn+/YNZ1mIouwxfmneiHjLaSOUr+Z1lVKXk2DQV9K CEljB32LGzIQfqFpDhj28jbeSczclJ27v9keXG5P1Ii/OpDK2K1GkBZNDKsepkWhC++mTR oFBDKcmYtz7ZIlM4KMRggX5tYSzT6P0= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-272-QBzEKqEmPNmlNsPbb96sag-1; Thu, 01 Feb 2024 09:49:34 -0500 X-MC-Unique: QBzEKqEmPNmlNsPbb96sag-1 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-40fb03d8a39so5813765e9.3 for ; Thu, 01 Feb 2024 06:49:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706798973; x=1707403773; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=6VQkXZRTncDUNIlS0JxxujfbxN+wn/MTe5ZN74XA7bM=; b=jo9WX/R6WyH6l6/l+rqwzdjGI0BMDkuN18wzA0ngJREFN2VKPRknJRvN2OvvMKLCTA /xn2RVjZC50tRR3uH3Pcu8R7xfIqHWH4V/6OkjRpGSUem8ZxFNYUPRwqxs4A8dskbJSa LbGsR4hnssl3q/ZsEsUjVM0njAVehBW5It0omqbeDDdb7ubXVQAkiuuCe09OOFFob/Og XrFxynrK6Ns7NSTog4WZ5vPQUQDwkJIs05bp/fqJvwA4dwGrEHoEDagwHmFIGkiyP0JW yWpyU3Jt6I91eAvES2UopF9D33eR3lxTlSfzTIFRd3YDQKbZIy7Sj6ERQyvn0oOXMPL6 YrQw== X-Gm-Message-State: AOJu0YyDmNHZUr9QJh4bYwuYXHabRrl/Dq2MgTXVVyBpufBMUqMKrlhZ ruI5gFeUHeVawVrZguFao7Lt0pMvygHWvBPmWp1g1stKPRN2RbM6fbuGpN7EBMlqifeUOr/3IXU YE8T58HG9RcPzAW30OTCbT/gPcLCMf9IIE9qh2/lHxzJvblDM7gMbjyWrGRGm4BBa+sU= X-Received: by 2002:a05:600c:1f93:b0:40e:f66f:1959 with SMTP id je19-20020a05600c1f9300b0040ef66f1959mr3750006wmb.17.1706798973187; Thu, 01 Feb 2024 06:49:33 -0800 (PST) X-Google-Smtp-Source: AGHT+IENiKDGf2/JLVi5Rcn2/qGMm9ujAywNrIFBnspeJl9xKluFuCHBLtW9+7ziP2xTNTH6wvgvew== X-Received: by 2002:a05:600c:1f93:b0:40e:f66f:1959 with SMTP id je19-20020a05600c1f9300b0040ef66f1959mr3749992wmb.17.1706798972727; Thu, 01 Feb 2024 06:49:32 -0800 (PST) X-Forwarded-Encrypted: i=0; AJvYcCV8x6U3lIpvEYVsehR9I7IB807+g9GTzSdTvOwpZ7MG/Q7PO5lJZkduJpbm9EicBkg9aNWOYH04dXSuEFCihCmNZnCF+YNIgr9oaw== Received: from [192.168.0.129] (ip-94-112-227-180.bb.vodafone.cz. [94.112.227.180]) by smtp.gmail.com with ESMTPSA id t24-20020a1c7718000000b0040f0219c371sm4572015wmi.19.2024.02.01.06.49.32 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 01 Feb 2024 06:49:32 -0800 (PST) Message-ID: <305f15db-70b0-4ee5-b9d8-322ac5ffd646@redhat.com> Date: Thu, 1 Feb 2024 15:49:31 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] gdb: Fix bps on inline callsites stopping in wrong frame To: Toby Lloyd Davies , gdb-patches@sourceware.org References: <20240125145354.1008724-1-tlloyddavies@undo.io> From: Guinevere Larsen In-Reply-To: <20240125145354.1008724-1-tlloyddavies@undo.io> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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 List-Id: On 25/01/2024 15:53, Toby Lloyd Davies wrote: > 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. Hi! I don't understand much of the code in this patch, but it does fix the issue you mentioned. Tested-By: Guinevere Larsen -- Cheers, Guinevere Larsen She/Her/Hers > --- > 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"