Re: [PATCH v5 5/5] gdb/testsuite: Test for a backtrace through object without debuginfo

[Guinevere Larsen <guinevere@redhat.com>]

On 10/3/24 3:58 AM, Gerlicher, Klaus wrote:
> Hi Guienvere,
>
> Just one suggestion inline.
>
> Thanks
> Klaus
>
>> -----Original Message-----
>> From: Guinevere Larsen <guinevere@redhat.com>
>> Sent: Tuesday, October 1, 2024 8:43 PM
>> To: gdb-patches@sourceware.org
>> Cc: Guinevere Larsen <blarsen@redhat.com>; Jan Kratochvil
>> <jan@jankratochvil.net>
>> Subject: [PATCH v5 5/5] gdb/testsuite: Test for a backtrace through object
>> without debuginfo
>>
>> From: Guinevere Larsen <blarsen@redhat.com>
>>
>> Fedora has been carrying this test since back in the Project Archer
>> days. A change back then caused GDB to stop being able to backtrace when
>> only some of the object files had debug information. Even though the
>> changed code never seems to have made its way into the main GDB project,
>> I think it makes sense to bring the test along to ensure something like
>> this doesn't pass unnoticed.
>>
>> Co-Authored-By: Jan Kratochvil <jan@jankratochvil.net>
>> ---
>>   .../backtrace-through-cu-nodebug-caller.c     | 28 ++++++
>>   .../backtrace-through-cu-nodebug-main.c       | 32 +++++++
>>   .../gdb.base/backtrace-through-cu-nodebug.exp | 90
>> +++++++++++++++++++
>>   3 files changed, 150 insertions(+)
>>   create mode 100644 gdb/testsuite/gdb.base/backtrace-through-cu-
>> nodebug-caller.c
>>   create mode 100644 gdb/testsuite/gdb.base/backtrace-through-cu-
>> nodebug-main.c
>>   create mode 100644 gdb/testsuite/gdb.base/backtrace-through-cu-
>> nodebug.exp
>>
>> diff --git a/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug-caller.c
>> b/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug-caller.c
>> new file mode 100644
>> index 00000000000..3a63d72a468
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug-caller.c
>> @@ -0,0 +1,28 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 2005-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/>.  */
>> +
>> +typedef int (*callback_t) (void);
>> +
>> +int
>> +caller (callback_t callback)
>> +{
>> +  /* Ensure some frame content to push away the return address.  */
>> +  volatile const long one = 1;
>> +
>> +  /* Modify the return value to prevent any tail-call optimization.  */
>> +  return (*callback) () - one;
>> +}
>> diff --git a/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug-main.c
>> b/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug-main.c
>> new file mode 100644
>> index 00000000000..3e7ac57a166
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug-main.c
>> @@ -0,0 +1,32 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 2005-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/>.  */
>> +
>> +typedef int (*callback_t) (void);
>> +
>> +extern int caller (callback_t callback);
>> +
>> +int
>> +callback (void)
>> +{
>> +  return 1;
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> +  return caller (callback);
>> +}
>> diff --git a/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug.exp
>> b/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug.exp
>> new file mode 100644
>> index 00000000000..7951ab41141
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug.exp
>> @@ -0,0 +1,90 @@
>> +# Copyright 2010-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/>.
>> +
>> +# Test that GDB can generate accurate backtraces even if some of the stack
>> +# trace goes through a function with no debug information.
>> +
>> +standard_testfile -caller.c -main.c
>> +set objmainfile ${testfile}-main.o
>> +set objcallerfile ${testfile}-caller.o
>> +
>> +# recompile the inferior with or without CFI information, then run the
>> +# inferior until the point where the important test starts
>> +# returns TRUE on an ERROR.
>> +proc prepare_test {has_cfi} {
>> +    global srcdir subdir srcfile srcfile2 objmainfile objcallerfile binfile
>> +    if {$has_cfi} {
>> +	set extension "cfi"
>> +	if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" ${objcallerfile} \
>> +	     object [list {additional_flags=-fomit-frame-pointer \
>> +		 -funwind-tables -fasynchronous-unwind-tables}]] != "" } {
>> +	    untested "couldn't compile with cfi"
>> +	    return true
>> +      }
>> +    } else {
>> +	set extension "no-cfi"
>> +	if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" ${objcallerfile} \
>> +	     object [list {additional_flags=-fomit-frame-pointer \
>> +		 -fno-unwind-tables \
>> +		 -fno-asynchronous-unwind-tables}]] != "" } {
>> +	    untested "couldn't compile without cfi"
>> +	    return true
>> +      }
>> +    }
>> +    if {[gdb_compile "${objmainfile} ${objcallerfile}" \
>> +	    "${binfile}-${extension}" binfile {}] != ""} {
>> +	untested "couldn't link object files"
>> +	return true
>> +    }
>> +
>> +    clean_restart "$binfile-${extension}"
>> +
>> +    with_test_prefix "${extension}" {
>> +
>> +	if ![runto callback] then {
>> +	   fail "has_cfi=$has_cfi: Can't run to callback"
>> +	   return true
>> +	}
>> +	gdb_test_no_output "maint frame-unwinder disable ARCH"
>> +	return false
>> +    }
>> +}
>> +
>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile2}" ${objmainfile} \
>> +	object {debug}] != "" } {
>> +    untested "couldn't compile main file"
>> +    return
>> +}
>> +
>> +if { [prepare_test false] } {
>> +     untested ${testfile}.exp
>> +} else {
>> +    gdb_test "bt" "frame_unwind_find_by_frame failed.*" \
>> +	"verify unwind fail without CFI"
>> +}
>> +
>> +if { [prepare_test true] } {
>> +     untested ${testfile}.exp
>> +} else {
>> +    if { [istarget "arm*-*-*"] } {
>> +	setup_kfail backtrace/31950 *-*-*
>> +    }
>> +    # #0  callback () at ...
>> +    # #1  0x00000000004004e9 in caller ()
>> +    # #2  0x00000000004004cd in main () at ...
>> +    gdb_test "bt" \
>> +	"#0 +callback \[^\r\n\]+\r\n#1 \[^\r\n\]+ in caller \[^\r\n\]+\r\n#2
>> \[^\r\n\]+ in main \[^\r\n\]+" \
>> +	"verify unwinding works for CFI without DIEs"
>> +}
> You could do below to reduce the line to under 80 cols, I wonder why we don't have this defined somewhere
> globally like "hex" or "decimal", though it's not really used that often.
>
> set r {[^\r\n]+}
> gdb_test "bt" "#0 +callback $r+\r\n#1 $r+ in caller $r+\r\n#2> $r+ in main $r+"

Thanks for the suggestion! I think calling it "r" makes things pretty 
hard to read, especially with how often it is followed by \r.

I called it "text" instead, which makes the line 84 lines, but much more 
readable IMO.

>
>> --
>> 2.46.2
>>
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de
> Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
>

-- 
Cheers,
Guinevere Larsen
She/Her/Hers