[v8,5/5] gdb/testsuite: Test for a backtrace through object without debuginfo

  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
  

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?

OK with these fixes.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

> +}
> -- 
> 2.47.0
  

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
  

Guinevere Larsen <guinevere@redhat.com> writes:

> 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.

Ahh, OK.  Not sure why I didn't grok that meaning yesterday.  Maybe it's
OK to just leave it then?  Or maybe just s/DIEs/debug info/?

>
> What do you think about this wording:
> 'Verify unwinding works based only on CFI information'

Yeah, or that.

Sorry, I think I was just being a bit slow when I read that yesterday!

Thanks,
Andrew

>
> ?
>
>>
>> OK with these fixes.
>>
>> Approved-By: Andrew Burgess <aburgess@redhat.com>
>>
>> Thanks,
>> Andrew
>>
>>> +}
>>> -- 
>>> 2.47.0
>
>
> -- 
> Cheers,
> Guinevere Larsen
> She/Her/Hers
  

On 1/16/25 15:37, Andrew Burgess wrote:
> +	if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" \
> +	     "${srcdir}/${subdir}/${objcallerfile}" \


I build gdb and ran the testsuite, and got:
...
$ git sti
On branch master
Your branch is up to date with 'origin/master'.

Ignored files:
   (use "git add -f <file>..." to include in what will be committed)
	gdb/testsuite/gdb.base/backtrace-through-cu-nodebug-caller.o
	gdb/testsuite/gdb.base/backtrace-through-cu-nodebug-main.o

nothing to commit, working tree clean
$
...

The object files should not be generated alongside the sources.

Thanks,
- Tom