[PATCH v8 5/5] gdb/testsuite: Test for a backtrace through object without debuginfo
Guinevere Larsen
guinevere@redhat.com
Thu Jan 16 18:42:12 GMT 2025
On 1/16/25 11:37 AM, Andrew Burgess wrote:
> Guinevere Larsen <guinevere@redhat.com> writes:
>
>> 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>
>> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>> ---
>> .../backtrace-through-cu-nodebug-caller.c | 28 ++++++
>> .../backtrace-through-cu-nodebug-main.c | 32 +++++++
>> .../gdb.base/backtrace-through-cu-nodebug.exp | 95 +++++++++++++++++++
>> 3 files changed, 155 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..c0940b406a8
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/backtrace-through-cu-nodebug.exp
>> @@ -0,0 +1,95 @@
>> +# Copyright 2010-2024 Free Software Foundation, Inc.
> Remember to update the copyright year throughout.
>
>> +
>> +# 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.
> Needs converting to two sentences ('.' after 'starts'). Plus capital
> 'R' for each sentence.
>
> I wonder if it would be better to return TRUE on success, and FALSE on
> error, because .... see below ...
>
>> +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}" \
>> + "${srcdir}/${subdir}/${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}" \
>> + "${srcdir}/${subdir}/${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 [list "${srcdir}/${subdir}/${objmainfile}" \
>> + "${srcdir}/${subdir}/${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}" \
>> + "${srcdir}/${subdir}/${objmainfile}" \
>> + object {debug}] != "" } {
>> + untested "couldn't compile main file"
>> + return
>> +}
>> +
>> +if { [prepare_test false] } {
>> + untested ${testfile}.exp
>> +} else {
>> + gdb_test "bt" "Required frame unwinder may have been disabled.*" \
>> + "verify unwind fail without CFI"
>> +}
> When something goes wrong with the prepare_test call we've already
> emitted an 'untested' or 'fail' message, thus I think we can just write:
>
> if { [prepare_test false] } {
> ... perform the test ...
> }
>
> Of course, this assumes that the true/false return value for
> prepare_test have been switched. Currently you'd write:
>
> if { ![prepare_test false] } {
> ... perform the test ...
> }
>
> which just seems weird.
>
> Also, the test here says "verify unwind fail without CFI". And we _do_
> check for the error message about disabled unwinders. But if the `bt`
> worked this test would still pass just fine. We should probably be
> checking that we only print frame #0. Something like:
>
> gdb_test "bt" \
> [multi_line \
> "\[^\r\n\]+Required frame unwinder may have been disabled, \[^\r\n\]+" \
> "#0\\s+callback \\(\\) \[^\r\n\]+"] \
> "verify unwind fail without CFI"
>
> should do the job I think.
>
>> +
>> +if { [prepare_test true] } {
>> + untested ${testfile}.exp
>> +} else {
> Same suggestion here about avoiding the extra 'untested' call.
>
>> + if { [istarget "arm*-*-*"] } {
>> + setup_kfail backtrace/31950 *-*-*
>> + }
>> + set text {[^\r\n]+}
>> + # #0 callback () at ...
>> + # #1 0x00000000004004e9 in caller ()
>> + # #2 0x00000000004004cd in main () at ...
>> + gdb_test "bt" \
>> + "#0 +callback $text\r\n#1 $text in caller $text\r\n#2 $text in main $text" \
>> + "verify unwinding works for CFI without DIEs"
> The test name here seems weird. Maybe: 'verify unwinding works for CUs
> without CFI' would be better?
I applied all other suggestions.
This naming came directly from the downstream patch, the point is to
parse a range described by a CFI but not by debug information.
What do you think about this wording:
'Verify unwinding works based only on CFI information'
?
>
> OK with these fixes.
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
>
> Thanks,
> Andrew
>
>> +}
>> --
>> 2.47.0
--
Cheers,
Guinevere Larsen
She/Her/Hers
More information about the Gdb-patches
mailing list