[PATCH 0/1] Test that frame info/IDs are stable/consistent

[PATCH 0/1] Test that frame info/IDs are stable/consistent

[Pedro Alves]

This commit adds a testcase that would have helped catch a CFI bug in
the AMD ROCm [1] GPU compiler, and helps making sure it stays fixed.
It is written in a way that works for all targets.

[1] https://rocmdocs.amd.com/

Pedro Alves (1):
  Test that frame info/IDs are stable/consistent

 .../gdb.base/frame-info-consistent.exp        | 137 ++++++++++++++++++
 1 file changed, 137 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/frame-info-consistent.exp


base-commit: 5226a6a892f922ea672e5775c61776830aaf27b7
-- 
2.26.2

[PATCH 1/1] Test that frame info/IDs are stable/consistent

[Pedro Alves]

This adds a testcase that tests that the unwinder produces consistent
frame info and frame IDs by making sure that "info frame" shows the
same result when stopped at a function (level == 0), compared to when
we find the same frame in the stack at a level > 0.

E.g., on x86-64, right after running to main, we see:

  (gdb) info frame
  Stack level 0, frame at 0x7fffffffd340:
   rip = 0x555555555168 in main (gdb.base/backtrace.c:41); saved rip = 0x7ffff7dd90b3
   source language c.
   Arglist at 0x7fffffffd330, args:
   Locals at 0x7fffffffd330, Previous frame's sp is 0x7fffffffd340
   Saved registers:
    rbp at 0x7fffffffd330, rip at 0x7fffffffd338
  (gdb)

and then after continuing to a function called by main, and selecting
the "main" frame again, we see:

  (gdb) info frame
  Stack level 3, frame at 0x7fffffffd340:
   rip = 0x555555555172 in main (gdb.base/backtrace.c:41); saved rip = 0x7ffff7dd90b3
   caller of frame at 0x7fffffffd330
   source language c.
   Arglist at 0x7fffffffd330, args:
   Locals at 0x7fffffffd330, Previous frame's sp is 0x7fffffffd340
   Saved registers:
    rbp at 0x7fffffffd330, rip at 0x7fffffffd338
  (gdb)

The only differences should be in the stack level, the 'rip = '
address, and the presence of the "caller of frame at" info.  All the
rest should be the same.  If it isn't, it probably means that the
frame base, the frame ID, etc. aren't stable & consistent.

The testcase exercises both the DWARF and the heuristic unwinders,
using "maint set dwarf unwinder on/off".

Tested on {x86-64 -m64, x86-64 -m32, Aarch64, Power8} GNU/Linux.

Change-Id: I795001c82cc70d543d197415e3f80ce5dc7f3452
---
 .../gdb.base/frame-info-consistent.exp        | 137 ++++++++++++++++++
 1 file changed, 137 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/frame-info-consistent.exp

diff --git a/gdb/testsuite/gdb.base/frame-info-consistent.exp b/gdb/testsuite/gdb.base/frame-info-consistent.exp
new file mode 100644
index 00000000000..7e4f80d234e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frame-info-consistent.exp
@@ -0,0 +1,137 @@
+# Copyright 2021 Free Software Foundation, Inc.
+# Copyright (C) 2021 Advanced Micro Devices, Inc. All rights reserved.
+
+# 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 the unwinder produces consistent frame info, by making
+# sure that "info frame" shows the same result when stopped at a
+# function (level == 0), compared to when we find the same frame in
+# the stack at a level > 0.  Tests both the DWARF stack unwinder, and
+# the fallback heuristic unwinder.
+
+standard_testfile backtrace.c
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+# Unwind to each function in FRAMES, and compare "info frame" output
+# to what was saved in the 'info_frame_before' array.
+proc compare_frames {frames} {
+    foreach_with_prefix compare_frame $frames {
+	if {[gdb_test \
+		 "frame function $compare_frame" \
+		 " $compare_frame .*"] != 0} {
+	    continue
+	}
+	set info_frame_after ""
+	gdb_test_multiple "info frame" "" {
+	    -re "(.*\r\n$::gdb_prompt $)" {
+		set info_frame_after $expect_out(1,string)
+		pass $gdb_test_name
+	    }
+	}
+
+	# Nuke the PC address, since it'll be different.  The
+	# first time it's the actual PC before the call, the
+	# second time it's the resume address after the call
+	# returns.
+	# E.g., on x86-64:
+	#   rip = 0x555555555168 in main (gdb.base/backtrace.c:41); saved rip = 0x7ffff7dd90b3
+	# vs
+	#   rip = 0x555555555172 in main (gdb.base/backtrace.c:41); saved rip = 0x7ffff7dd90b3
+	#
+	set from \
+	    "= $::hex in $compare_frame "
+	set to \
+	    "= \$hex in $compare_frame "
+	regsub $from $::info_frame_before($compare_frame) $to \
+	    ::info_frame_before($compare_frame)
+	regsub $from $info_frame_after $to \
+	    info_frame_after
+
+	# Remove the "caller of frame at" line, which didn't
+	# appear the first time, since the frame hadn't called any
+	# other function yet then.
+	regsub "\r\n caller of frame at $::hex\r\n" \
+	    $info_frame_after "\r\n" \
+	    info_frame_after
+	regsub ", caller of frame at $::hex" \
+	    $info_frame_after "" \
+	    info_frame_after
+
+	# "Stack level 0/1/2/3" -> "Stack level N"
+	set from \
+	    "Stack level $::decimal"
+	set to \
+	    "Stack level N"
+	regsub $from $::info_frame_before($compare_frame) $to \
+	    ::info_frame_before($compare_frame)
+	regsub $from $info_frame_after $to \
+	    info_frame_after
+
+	# For debugging.
+	verbose -log "BEFORE:\n$::info_frame_before($compare_frame)"
+	verbose -log "AFTER:\n$info_frame_after"
+
+	gdb_assert {[string match \
+			 $::info_frame_before($compare_frame)\
+			 $info_frame_after]} \
+	    "info frame before/after match"
+    }
+}
+
+proc test {dwarf_unwinder} {
+
+    clean_restart $::binfile
+
+    gdb_test_no_output "maint set dwarf unwinder $dwarf_unwinder"
+
+    if ![runto_main] then {
+	fail "can't run to main"
+	return 0
+    }
+
+    array unset ::info_frame_before
+
+    # Run to each function, and record "info frame" output in the
+    # 'info_frame_before' array.  At each stop, unwind to each
+    # already-recorded function, and compare "info frame" output to
+    # what was saved in the 'info_frame_before' array.
+    set funcs {"main" "foo" "bar" "baz"}
+    set idx_funcs 0
+    foreach_with_prefix stop_func $funcs {
+	if {$idx_funcs != 0} {
+	    gdb_breakpoint $stop_func
+	    gdb_continue_to_breakpoint ".*$stop_func \(\).*"
+	}
+
+	set ::info_frame_before($stop_func) ""
+	gdb_test_multiple "info frame" "" {
+	    -re "(.*\r\n$::gdb_prompt $)" {
+		set ::info_frame_before($stop_func) $expect_out(1,string)
+		pass $gdb_test_name
+	    }
+	}
+
+	if {$idx_funcs != 0} {
+	    compare_frames [lreverse [lrange $funcs 0 $idx_funcs-1]]
+	}
+	incr idx_funcs
+    }
+}
+
+foreach_with_prefix dwarf {"off" "on"} {
+    test $dwarf
+}
-- 
2.26.2

Re: [PATCH 1/1] Test that frame info/IDs are stable/consistent

[Simon Marchi]

On 2021-09-21 2:45 p.m., Pedro Alves wrote:
> This adds a testcase that tests that the unwinder produces consistent
> frame info and frame IDs by making sure that "info frame" shows the
> same result when stopped at a function (level == 0), compared to when
> we find the same frame in the stack at a level > 0.
> 
> E.g., on x86-64, right after running to main, we see:
> 
>   (gdb) info frame
>   Stack level 0, frame at 0x7fffffffd340:
>    rip = 0x555555555168 in main (gdb.base/backtrace.c:41); saved rip = 0x7ffff7dd90b3
>    source language c.
>    Arglist at 0x7fffffffd330, args:
>    Locals at 0x7fffffffd330, Previous frame's sp is 0x7fffffffd340
>    Saved registers:
>     rbp at 0x7fffffffd330, rip at 0x7fffffffd338
>   (gdb)
> 
> and then after continuing to a function called by main, and selecting
> the "main" frame again, we see:
> 
>   (gdb) info frame
>   Stack level 3, frame at 0x7fffffffd340:
>    rip = 0x555555555172 in main (gdb.base/backtrace.c:41); saved rip = 0x7ffff7dd90b3
>    caller of frame at 0x7fffffffd330
>    source language c.
>    Arglist at 0x7fffffffd330, args:
>    Locals at 0x7fffffffd330, Previous frame's sp is 0x7fffffffd340
>    Saved registers:
>     rbp at 0x7fffffffd330, rip at 0x7fffffffd338
>   (gdb)
> 
> The only differences should be in the stack level, the 'rip = '
> address, and the presence of the "caller of frame at" info.  All the
> rest should be the same.  If it isn't, it probably means that the
> frame base, the frame ID, etc. aren't stable & consistent.
> 
> The testcase exercises both the DWARF and the heuristic unwinders,
> using "maint set dwarf unwinder on/off".
> 
> Tested on {x86-64 -m64, x86-64 -m32, Aarch64, Power8} GNU/Linux.
> 
> Change-Id: I795001c82cc70d543d197415e3f80ce5dc7f3452
> ---
>  .../gdb.base/frame-info-consistent.exp        | 137 ++++++++++++++++++
>  1 file changed, 137 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/frame-info-consistent.exp
> 
> diff --git a/gdb/testsuite/gdb.base/frame-info-consistent.exp b/gdb/testsuite/gdb.base/frame-info-consistent.exp
> new file mode 100644
> index 00000000000..7e4f80d234e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/frame-info-consistent.exp
> @@ -0,0 +1,137 @@
> +# Copyright 2021 Free Software Foundation, Inc.
> +# Copyright (C) 2021 Advanced Micro Devices, Inc. All rights reserved.

Since this is submitted upstream, I guess the second Copyright line
should be removed?

> +
> +# 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 the unwinder produces consistent frame info, by making
> +# sure that "info frame" shows the same result when stopped at a
> +# function (level == 0), compared to when we find the same frame in
> +# the stack at a level > 0.  Tests both the DWARF stack unwinder, and
> +# the fallback heuristic unwinder.
> +
> +standard_testfile backtrace.c
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> +    return -1
> +}

I'd suggest switching to build_executable, since we do a clean_restart
below anyway.

Otherwise, LGTM.

Simon

Re: [PATCH 1/1] Test that frame info/IDs are stable/consistent

[Pedro Alves]

On 2021-09-23 6:28 p.m., Simon Marchi wrote:

>> diff --git a/gdb/testsuite/gdb.base/frame-info-consistent.exp b/gdb/testsuite/gdb.base/frame-info-consistent.exp
>> new file mode 100644
>> index 00000000000..7e4f80d234e
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/frame-info-consistent.exp
>> @@ -0,0 +1,137 @@
>> +# Copyright 2021 Free Software Foundation, Inc.
>> +# Copyright (C) 2021 Advanced Micro Devices, Inc. All rights reserved.
> 
> Since this is submitted upstream, I guess the second Copyright line
> should be removed?

Oh yes, missed that!  Thanks.

>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
>> +    return -1
>> +}
> 
> I'd suggest switching to build_executable, since we do a clean_restart
> below anyway.

Definitely.  Done.

> 
> Otherwise, LGTM.

Thanks, pushed.