From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: Tom de Vries <tdevries@suse.de>,
"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH v5 2/2] [gdb/symtab] Handle two-linetable function in find_epilogue_using_linetable
Date: Wed, 10 Apr 2024 09:28:02 +0200 [thread overview]
Message-ID: <5510b6b7-5834-4a9c-90f0-c80bb48af05b@hotmail.de> (raw)
In-Reply-To: <20240409092753.10567-2-tdevries@suse.de>
Hi Tom,
Nice, just one nit below:
On 4/9/24 11:27, Tom de Vries wrote:
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> Consider the following test-case:
> ...
> $ cat hello.c
> int main()
> {
> printf("hello ");
> #include "world.inc"
> $ cat world.inc
> printf("world\n");
> return 0;
> }
> $ gcc -g hello.c
> ...
>
> The line table for the compilation unit, consisting just of
> function main, is translated into these two gdb line tables, one for hello.c
> and one for world.inc:
> ...
> compunit_symtab: hello.c
> symtab: hello.c
> INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN
> 0 3 0x400557 0x400557 Y
> 1 4 0x40055b 0x40055b Y
> 2 END 0x40056a 0x40056a Y
>
> compunit_symtab: hello.c
> symtab: world.inc
> INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN
> 0 1 0x40056a 0x40056a Y
> 1 2 0x400574 0x400574 Y
> 2 3 0x400579 0x400579 Y
> 3 END 0x40057b 0x40057b Y
> ...
>
> The epilogue of main starts at 0x400579:
> ...
> 400579: 5d pop %rbp
> 40057a: c3 ret
> ...
>
> Now, say we have an epilogue_begin marker in the line table at 0x400579.
>
> We won't find it using find_epilogue_using_linetable, because it does:
> ...
> const struct symtab_and_line sal = find_pc_line (start_pc, 0);
> ...
> which gets us the line table for hello.c.
>
> Fix this by using "find_pc_line (end_pc - 1, 0)" instead.
>
> Tested on x86_64-linux.
>
> Co-Authored-By: Tom de Vries <tdevries@suse.de>
>
> PR symtab/31622
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31622
> ---
> gdb/symtab.c | 11 +-
> .../gdb.dwarf2/dw2-epilogue-begin-2.exp | 20 ++
> .../gdb.dwarf2/dw2-epilogue-begin.c.inc | 51 +++++
> .../gdb.dwarf2/dw2-epilogue-begin.exp | 157 +-------------
> .../gdb.dwarf2/dw2-epilogue-begin.exp.tcl | 199 ++++++++++++++++++
> gdb/testsuite/lib/dwarf.exp | 5 +-
> 6 files changed, 284 insertions(+), 159 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin-2.exp
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.c.inc
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp.tcl
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index e032178aaa6..034f71226b6 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4158,8 +4158,15 @@ find_epilogue_using_linetable (CORE_ADDR func_addr)
>
> /* While the standard allows for multiple points marked with epilogue_begin
> in the same function, for performance reasons, this function will only
> - find the last address that sets this flag for a given block. */
> - const struct symtab_and_line sal = find_pc_line (start_pc, 0);
> + find the last address that sets this flag for a given block.
> +
> + The lines of a function can be described by several line tables in case
> + there are different files involved. There's a corner case where a
> + function epilogue is in a different file than a function start, and using
> + start_pc as argument to find_pc_line will mean we won't find the
> + epilogue. Instead, use "end_pc - 1" to maximize our changes of picking
s/changes/chances/
> + the line table containing an epilogue. */
> + const struct symtab_and_line sal = find_pc_line (end_pc - 1, 0);
> if (sal.symtab != nullptr && sal.symtab->language () != language_asm)
> {
> struct objfile *objfile = sal.symtab->compunit ()->objfile ();
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin-2.exp b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin-2.exp
> new file mode 100644
> index 00000000000..6302ef1ad05
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin-2.exp
> @@ -0,0 +1,20 @@
> +# Copyright 2022-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 <http://www.gnu.org/licenses/>.
> +
> +standard_testfile dw2-epilogue-begin.c dw2-epilogue-begin.S
> +
> +set version 2
> +
> +source $srcdir/$subdir/dw2-epilogue-begin.exp.tcl
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.c.inc b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.c.inc
> new file mode 100644
> index 00000000000..4ff445cf37d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.c.inc
> @@ -0,0 +1,51 @@
> +/* Copyright 2023-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 <http://www.gnu.org/licenses/>. */
> +
> +void
> +__attribute__((used))
> +trivial (void)
> +{
> + asm ("trivial_label: .global trivial_label"); /* trivial function */
> +}
> +
> +char global;
> +
> +void
> +watch (void)
> +{ /* watch start */
> + asm ("watch_label: .global watch_label");
> + asm ("mov $0x0, %rax");
> + int local = 0; /* watch prologue */
> +
> + asm ("watch_start: .global watch_start");
> + asm ("mov $0x1, %rax");
> + local = 1; /* watch assign */
> + asm ("watch_reassign: .global watch_reassign");
> + asm ("mov $0x2, %rax");
> + local = 2; /* watch reassign */
> + asm ("watch_end: .global watch_end"); /* watch end */
> +}
> +
> +int
> +main (void)
> +{ /* main prologue */
> + asm ("main_label: .global main_label");
> + global = 0;
> + asm ("main_fun_call: .global main_fun_call");
> + watch (); /* main function call */
> + asm ("main_epilogue: .global main_epilogue");
> + global = 10;
> + return 0; /* main end */
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp
> index f646e23da62..9b9d6c71de4 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp
> @@ -13,161 +13,8 @@
> # You should have received a copy of the GNU General Public License
> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>
> -# Check that GDB can honor the epilogue_begin flag the compiler can place
> -# in the line-table data.
> -# We test 2 things: 1. that a software watchpoint triggered in an epilogue
> -# is correctly ignored
> -# 2. that GDB can mark the same line as both prologue and epilogue
> -
> -load_lib dwarf.exp
> -
> -# This test can only be run on targets which support DWARF-2 and use gas.
> -require dwarf2_support
> -# restricted to x86 to make it simpler to follow a variable
> -require is_x86_64_m64_target
> -
> standard_testfile .c .S
>
> -set trivial_line [gdb_get_line_number "trivial function"]
> -set main_prologue [gdb_get_line_number "main prologue"]
> -set main_epilogue [gdb_get_line_number "main end"]
> -set watch_start_line [gdb_get_line_number "watch start"]
> -
> -set asm_file [standard_output_file $srcfile2]
> -
> -# The producer will be set to clang because at the time of writing
> -# we only care about epilogues if the producer is clang. When the
> -# producer is GCC, variables use CFA locations, so watchpoints can
> -# continue working even on epilogues.
> -Dwarf::assemble $asm_file {
> - global srcdir subdir srcfile srcfile2
> - global trivial_line main_prologue main_epilogue watch_start_line
> - declare_labels lines_label
> -
> - get_func_info main
> - get_func_info trivial
> - get_func_info watch
> -
> - cu {} {
> - compile_unit {
> - {language @DW_LANG_C}
> - {name dw2-prologue-end.c}
> - {stmt_list ${lines_label} DW_FORM_sec_offset}
> - {producer "clang version 17.0.1"}
> - } {
> - declare_labels char_label
> -
> - char_label: base_type {
> - {name char}
> - {encoding @DW_ATE_signed}
> - {byte_size 1 DW_FORM_sdata}
> - }
> -
> - subprogram {
> - {external 1 flag}
> - {name trivial}
> - {low_pc $trivial_start addr}
> - {high_pc "$trivial_start + $trivial_len" addr}
> - }
> - subprogram {
> - {external 1 flag}
> - {name watch}
> - {low_pc $watch_start addr}
> - {high_pc "$watch_start + $watch_len" addr}
> - } {
> - DW_TAG_variable {
> - {name local}
> - {type :$char_label}
> - {DW_AT_location {DW_OP_reg0} SPECIAL_expr}
> - }
> - }
> - subprogram {
> - {external 1 flag}
> - {name main}
> - {low_pc $main_start addr}
> - {high_pc "$main_start + $main_len" addr}
> - }
> - }
> - }
> -
> - lines {version 5} lines_label {
> - set diridx [include_dir "${srcdir}/${subdir}"]
> - file_name "$srcfile" $diridx
> -
> - program {
> - DW_LNS_set_file $diridx
> - DW_LNE_set_address $trivial_start
> - line $trivial_line
> - DW_LNS_set_prologue_end
> - DW_LNS_set_epilogue_begin
> - DW_LNS_copy
> -
> - DW_LNE_set_address watch
> - line $watch_start_line
> - DW_LNS_copy
> -
> - DW_LNE_set_address watch_start
> - line [gdb_get_line_number "watch assign"]
> - DW_LNS_set_prologue_end
> - DW_LNS_copy
> -
> - DW_LNE_set_address watch_reassign
> - line [gdb_get_line_number "watch reassign"]
> - DW_LNS_set_epilogue_begin
> - DW_LNS_copy
> -
> - DW_LNE_set_address watch_end
> - line [gdb_get_line_number "watch end"]
> - DW_LNS_copy
> -
> - DW_LNE_set_address $main_start
> - line $main_prologue
> - DW_LNS_set_prologue_end
> - DW_LNS_copy
> -
> - DW_LNE_set_address main_fun_call
> - line [gdb_get_line_number "main function call"]
> - DW_LNS_copy
> -
> - DW_LNE_set_address main_epilogue
> - line $main_epilogue
> - DW_LNS_set_epilogue_begin
> - DW_LNS_copy
> -
> - DW_LNE_end_sequence
> - }
> - }
> -}
> -
> -if { [prepare_for_testing "failed to prepare" ${testfile} \
> - [list $srcfile $asm_file] {nodebug}] } {
> - return -1
> -}
> -
> -if ![runto_main] {
> - return -1
> -}
> -
> -# Moving to the scope with a local variable.
> -gdb_breakpoint $watch_start_line
> -gdb_continue_to_breakpoint "continuing to function" ".*"
> -gdb_test "next" "local = 2.*" "stepping to epilogue"
> -
> -# Forcing software watchpoints because hardware ones don't care if we
> -# are in the epilogue or not.
> -gdb_test_no_output "set can-use-hw-watchpoints 0"
> -
> -# Test that the software watchpoint will not trigger in this case
> -gdb_test "watch local" "\[W|w\]atchpoint .: local" "set watchpoint"
> -gdb_test "continue" ".*\[W|w\]atchpoint . deleted.*" \
> - "confirm watchpoint doesn't trigger"
> +set version 1
>
> -# First we test that the trivial function has a line with both a prologue
> -# and an epilogue. Do this by finding a line that has 3 Y columns
> -set sep "\[ \t\]"
> -set hex_number "0x\[0-9a-f\]+"
> -gdb_test_multiple "maint info line-table" "test epilogue in linetable" -lbl {
> - -re "\[0-9\]$sep+$trivial_line$sep+$hex_number$sep+$hex_number$sep+Y$sep+Y$sep+Y" {
> - pass $gdb_test_name
> - }
> -}
> +source $srcdir/$subdir/dw2-epilogue-begin.exp.tcl
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp.tcl b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp.tcl
> new file mode 100644
> index 00000000000..155916b92df
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-epilogue-begin.exp.tcl
> @@ -0,0 +1,199 @@
> +# Copyright 2022-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 <http://www.gnu.org/licenses/>.
> +
> +# Check that GDB can honor the epilogue_begin flag the compiler can place
> +# in the line-table data.
> +# We test 2 things: 1. that a software watchpoint triggered in an epilogue
> +# is correctly ignored
> +# 2. that GDB can mark the same line as both prologue and epilogue
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +require dwarf2_support
> +# restricted to x86 to make it simpler to follow a variable
> +require is_x86_64_m64_target
> +
> +set trivial_line [gdb_get_line_number "trivial function"]
> +set main_prologue [gdb_get_line_number "main prologue"]
> +set main_epilogue [gdb_get_line_number "main end"]
> +set watch_start_line [gdb_get_line_number "watch start"]
> +
> +set asm_file [standard_output_file $srcfile2]
> +
> +# The producer will be set to clang because at the time of writing
> +# we only care about epilogues if the producer is clang. When the
> +# producer is GCC, variables use CFA locations, so watchpoints can
> +# continue working even on epilogues.
> +Dwarf::assemble $asm_file {
> + global srcdir subdir srcfile srcfile2
> + global trivial_line main_prologue main_epilogue watch_start_line
> + declare_labels lines_label
> +
> + get_func_info main
> + get_func_info trivial
> + get_func_info watch
> +
> + if { $::version == 1 } {
> + set switch_file {}
> + } elseif { $::version == 2 } {
> + set switch_file { set f $f2 }
> + } else {
> + error "Unhandled version: $::version"
> + }
> +
> + cu {} {
> + compile_unit {
> + {language @DW_LANG_C}
> + {name dw2-prologue-end.c}
> + {stmt_list ${lines_label} DW_FORM_sec_offset}
> + {producer "clang version 17.0.1"}
> + } {
> + declare_labels char_label
> +
> + char_label: base_type {
> + {name char}
> + {encoding @DW_ATE_signed}
> + {byte_size 1 DW_FORM_sdata}
> + }
> +
> + subprogram {
> + {external 1 flag}
> + {name trivial}
> + {low_pc $trivial_start addr}
> + {high_pc "$trivial_start + $trivial_len" addr}
> + }
> + subprogram {
> + {external 1 flag}
> + {name watch}
> + {low_pc $watch_start addr}
> + {high_pc "$watch_start + $watch_len" addr}
> + } {
> + DW_TAG_variable {
> + {name local}
> + {type :$char_label}
> + {DW_AT_location {DW_OP_reg0} SPECIAL_expr}
> + }
> + }
> + subprogram {
> + {external 1 flag}
> + {name main}
> + {low_pc $main_start addr}
> + {high_pc "$main_start + $main_len" addr}
> + }
> + }
> + }
> +
> + lines {version 5} lines_label {
> + set diridx [include_dir "${srcdir}/${subdir}"]
> + set f1 [file_name "$srcfile" $diridx]
> + set f2 [file_name "$srcfile.inc" $diridx]
> +
> + set f $f1
> + program {
> + DW_LNS_set_file $f
> +
> + DW_LNE_set_address $trivial_start
> + line $trivial_line
> + DW_LNS_set_prologue_end
> + DW_LNS_set_epilogue_begin
> + DW_LNS_copy
> +
> + DW_LNE_set_address $trivial_end
> + DW_LNE_end_sequence
> +
> +
> + DW_LNS_set_file $f
> +
> + DW_LNE_set_address $watch_start
> + line $watch_start_line
> + DW_LNS_copy
> +
> + DW_LNE_set_address watch_start
> + line [gdb_get_line_number "watch assign"]
> + DW_LNS_set_prologue_end
> + DW_LNS_copy
> +
> + eval $switch_file
> + DW_LNS_set_file $f
> +
> + DW_LNE_set_address watch_reassign
> + line [gdb_get_line_number "watch reassign"]
> + DW_LNS_set_epilogue_begin
> + DW_LNS_copy
> +
> + DW_LNE_set_address watch_end
> + line [gdb_get_line_number "watch end"]
> + DW_LNS_copy
> +
> + DW_LNE_set_address $watch_end
> + DW_LNE_end_sequence
> +
> +
> + DW_LNS_set_file $f
> +
> + DW_LNE_set_address $main_start
> + line $main_prologue
> + DW_LNS_set_prologue_end
> + DW_LNS_copy
> +
> + DW_LNE_set_address main_fun_call
> + line [gdb_get_line_number "main function call"]
> + DW_LNS_copy
> +
> + DW_LNE_set_address main_epilogue
> + line $main_epilogue
> + DW_LNS_set_epilogue_begin
> + DW_LNS_copy
> +
> + DW_LNE_set_address $main_end
> + DW_LNE_end_sequence
> + }
> + }
> +}
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> + [list $srcfile $asm_file] {nodebug}] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +# Moving to the scope with a local variable.
> +
> +gdb_breakpoint $srcfile:$watch_start_line
> +gdb_continue_to_breakpoint "continuing to function" ".*"
> +gdb_test "next" "local = 2.*" "stepping to epilogue"
> +
> +# Forcing software watchpoints because hardware ones don't care if we
> +# are in the epilogue or not.
> +gdb_test_no_output "set can-use-hw-watchpoints 0"
> +
> +# Test that the software watchpoint will not trigger in this case
> +gdb_test "watch local" "\[W|w\]atchpoint .: local" "set watchpoint"
> +gdb_test "continue" ".*\[W|w\]atchpoint . deleted.*" \
> + "confirm watchpoint doesn't trigger"
> +
> +# First we test that the trivial function has a line with both a prologue
> +# and an epilogue. Do this by finding a line that has 3 Y columns
> +set sep "\[ \t\]"
> +set hex_number "0x\[0-9a-f\]+"
> +gdb_test_multiple "maint info line-table" "test epilogue in linetable" -lbl {
> + -re "\[0-9\]$sep+$trivial_line$sep+$hex_number$sep+$hex_number$sep+Y$sep+Y$sep+Y" {
> + pass $gdb_test_name
> + }
> +}
> diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
> index d085f835f07..adc3a18ee4f 100644
> --- a/gdb/testsuite/lib/dwarf.exp
> +++ b/gdb/testsuite/lib/dwarf.exp
> @@ -2427,10 +2427,11 @@ namespace eval Dwarf {
> variable _line_file_names
> lappend _line_file_names $filename $diridx
>
> + set nr_filenames [expr [llength $_line_file_names] / 2]
> if { $Dwarf::_line_unit_version >= 5 } {
> - return [expr [llength $_line_file_names] - 1]
> + return [expr $nr_filenames - 1]
> } else {
> - return [llength $_line_file_names]
> + return $nr_filenames
> }
> }
>
next prev parent reply other threads:[~2024-04-10 7:26 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 9:27 [PATCH v5 1/2] [gdb/symtab] Fix an out of bounds array access " Tom de Vries
2024-04-09 9:27 ` [PATCH v5 2/2] [gdb/symtab] Handle two-linetable function " Tom de Vries
2024-04-10 7:28 ` Bernd Edlinger [this message]
2024-04-22 13:39 ` Andrew Burgess
2024-04-11 13:59 ` [PATCH v5 1/2] [gdb/symtab] Fix an out of bounds array access " Bernd Edlinger
2024-04-22 13:22 ` Andrew Burgess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5510b6b7-5834-4a9c-90f0-c80bb48af05b@hotmail.de \
--to=bernd.edlinger@hotmail.de \
--cc=gdb-patches@sourceware.org \
--cc=tdevries@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).