[PATCH] gdb: xtensa: fix truncated backtrace for nested noreturn functions

[PATCH] gdb: xtensa: fix truncated backtrace for nested noreturn functions

[Alexey Lapshin]

The problem appears when break in nested noreturn calls.
panic_abort() and esp_system_abort() are noreturn functions:

    #0  0x4008779f in panic_abort ()
    #1  0x40087a78 in esp_system_abort ()
    Backtrace stopped: previous frame identical to this frame (corrupt stack?)

Assembly listing:

    40081ad4 <panic_abort>:
    40081ad4:	004136        	entry	a1, 32
    40081aeb:	ffff06        	j	40081aeb <panic_abort+0x17>
    	...

    40085614 <esp_system_abort>:
    40085614:	004136        	entry	a1, 32
    40085619:	fc4ba5        	call8	40081ad4 <panic_abort>

    4008561c <__ubsan_include>:
    4008561c:	004136        	entry	a1, 32

PC register for frame esp_system_abort points to the next instruction after
instruction with address 40085619.
It is ENTRY instruction for __ubsan_include. This caused wrong unwinding
because we are not in __ubsan_include at this frame. In general for
noreturn functions there should be RET instruction. This is why it works
in all other cases.

PC register can point to entry instruction only for the innermost frame.
It is not possible otherwise.

The fix is making it not possible to go with not innermost frame into
the code block which collects frame cache for frames when PC is on entry
instruction.
---
 gdb/testsuite/gdb.base/backtrace-noreturn.c   | 42 ++++++++++++++++
 gdb/testsuite/gdb.base/backtrace-noreturn.exp | 48 +++++++++++++++++++
 gdb/xtensa-tdep.c                             |  3 +-
 3 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/backtrace-noreturn.c
 create mode 100644 gdb/testsuite/gdb.base/backtrace-noreturn.exp

diff --git a/gdb/testsuite/gdb.base/backtrace-noreturn.c b/gdb/testsuite/gdb.base/backtrace-noreturn.c
new file mode 100644
index 00000000000..bd492013ee8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/backtrace-noreturn.c
@@ -0,0 +1,42 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019-2022 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/>.  */
+
+#include "../lib/attributes.h"
+
+void __attribute__((noreturn)) ATTRIBUTE_NOCLONE
+baz ()
+{
+  while(1); /* Break here.  */
+}
+
+void __attribute__((noreturn)) ATTRIBUTE_NOCLONE
+bar ()
+{
+  baz ();
+}
+
+void __attribute__((noinline)) ATTRIBUTE_NOCLONE
+foo ()
+{
+  bar ();
+}
+
+int
+main ()
+{
+  foo ();
+}
diff --git a/gdb/testsuite/gdb.base/backtrace-noreturn.exp b/gdb/testsuite/gdb.base/backtrace-noreturn.exp
new file mode 100644
index 00000000000..e89efc0241b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/backtrace-noreturn.exp
@@ -0,0 +1,48 @@
+# Copyright 2019-2022 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/>.
+
+# A place for miscellaneous tests related to backtrace.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
+# Run to the breakpoint at return.
+gdb_breakpoint [gdb_get_line_number "Break here."]
+gdb_continue_to_breakpoint "Break here."
+
+# Backtrace with the default options.
+gdb_test "bt" \
+    [multi_line \
+	 "#0\[ \t\]*baz \\(\\) at \[^\r\n\]+" \
+	 "#1\[ \t\]*$hex in bar \\(\\) at \[^\r\n\]+" \
+	 "#2\[ \t\]*$hex in foo \\(\\) at \[^\r\n\]+" \
+	 "#3\[ \t\]*$hex in main \\(\\) at \[^\r\n\]+" ]
+
+# Backtrace with 'set disassemble-next-line on'.  This shouldn't make
+# any difference to the backtrace.
+gdb_test "with disassemble-next-line on -- bt" \
+    [multi_line \
+	 "#0\[ \t\]*baz \\(\\) at \[^\r\n\]+" \
+	 "#1\[ \t\]*$hex in bar \\(\\) at \[^\r\n\]+" \
+	 "#2\[ \t\]*$hex in foo \\(\\) at \[^\r\n\]+" \
+	 "#3\[ \t\]*$hex in main \\(\\) at \[^\r\n\]+" ]
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 5444ebb7f6a..e8a143fadab 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -1262,7 +1262,8 @@ xtensa_frame_cache (frame_info_ptr this_frame, void **this_cache)
       ws = get_frame_register_unsigned (this_frame,
 					tdep->ws_regnum);
 
-      if (safe_read_memory_integer (pc, 1, byte_order, &op1)
+      if (frame_relative_level (this_frame) == 0
+	  && safe_read_memory_integer (pc, 1, byte_order, &op1)
 	  && XTENSA_IS_ENTRY (gdbarch, op1))
 	{
 	  int callinc = CALLINC (ps);
-- 
2.34.1

Re: [PATCH] gdb: xtensa: fix truncated backtrace for nested noreturn functions

[John Baldwin]

On 4/3/24 11:04 AM, Alexey Lapshin wrote:
> The problem appears when break in nested noreturn calls.
> panic_abort() and esp_system_abort() are noreturn functions:
> 
>      #0  0x4008779f in panic_abort ()
>      #1  0x40087a78 in esp_system_abort ()
>      Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> 
> Assembly listing:
> 
>      40081ad4 <panic_abort>:
>      40081ad4:	004136        	entry	a1, 32
>      40081aeb:	ffff06        	j	40081aeb <panic_abort+0x17>
>      	...
> 
>      40085614 <esp_system_abort>:
>      40085614:	004136        	entry	a1, 32
>      40085619:	fc4ba5        	call8	40081ad4 <panic_abort>
> 
>      4008561c <__ubsan_include>:
>      4008561c:	004136        	entry	a1, 32
> 
> PC register for frame esp_system_abort points to the next instruction after
> instruction with address 40085619.
> It is ENTRY instruction for __ubsan_include. This caused wrong unwinding
> because we are not in __ubsan_include at this frame. In general for
> noreturn functions there should be RET instruction. This is why it works
> in all other cases.
> 
> PC register can point to entry instruction only for the innermost frame.
> It is not possible otherwise.
> 
> The fix is making it not possible to go with not innermost frame into
> the code block which collects frame cache for frames when PC is on entry
> instruction.
> ---
>   gdb/testsuite/gdb.base/backtrace-noreturn.c   | 42 ++++++++++++++++
>   gdb/testsuite/gdb.base/backtrace-noreturn.exp | 48 +++++++++++++++++++
>   gdb/xtensa-tdep.c                             |  3 +-
>   3 files changed, 92 insertions(+), 1 deletion(-)
>   create mode 100644 gdb/testsuite/gdb.base/backtrace-noreturn.c
>   create mode 100644 gdb/testsuite/gdb.base/backtrace-noreturn.exp
> 
> diff --git a/gdb/testsuite/gdb.base/backtrace-noreturn.c b/gdb/testsuite/gdb.base/backtrace-noreturn.c
> new file mode 100644
> index 00000000000..bd492013ee8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/backtrace-noreturn.c
> @@ -0,0 +1,42 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019-2022 Free Software Foundation, Inc.

This should probably be 2024?

> +
> +   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/>.  */
> +
> +#include "../lib/attributes.h"
> +
> +void __attribute__((noreturn)) ATTRIBUTE_NOCLONE
> +baz ()
> +{
> +  while(1); /* Break here.  */
> +}
> +
> +void __attribute__((noreturn)) ATTRIBUTE_NOCLONE
> +bar ()
> +{
> +  baz ();
> +}
> +
> +void __attribute__((noinline)) ATTRIBUTE_NOCLONE
> +foo ()
> +{
> +  bar ();
> +}
> +
> +int
> +main ()
> +{
> +  foo ();
> +}
> diff --git a/gdb/testsuite/gdb.base/backtrace-noreturn.exp b/gdb/testsuite/gdb.base/backtrace-noreturn.exp
> new file mode 100644
> index 00000000000..e89efc0241b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/backtrace-noreturn.exp
> @@ -0,0 +1,48 @@
> +# Copyright 2019-2022 Free Software Foundation, Inc.

2024 here as well?

> +# 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/>.
> +
> +# A place for miscellaneous tests related to backtrace.
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> +    return -1
> +}
> +
> +if ![runto_main] then {
> +    fail "can't run to main"
> +    return 0
> +}
> +
> +# Run to the breakpoint at return.
> +gdb_breakpoint [gdb_get_line_number "Break here."]
> +gdb_continue_to_breakpoint "Break here."
> +
> +# Backtrace with the default options.
> +gdb_test "bt" \
> +    [multi_line \
> +	 "#0\[ \t\]*baz \\(\\) at \[^\r\n\]+" \
> +	 "#1\[ \t\]*$hex in bar \\(\\) at \[^\r\n\]+" \
> +	 "#2\[ \t\]*$hex in foo \\(\\) at \[^\r\n\]+" \
> +	 "#3\[ \t\]*$hex in main \\(\\) at \[^\r\n\]+" ]
> +
> +# Backtrace with 'set disassemble-next-line on'.  This shouldn't make
> +# any difference to the backtrace.
> +gdb_test "with disassemble-next-line on -- bt" \
> +    [multi_line \
> +	 "#0\[ \t\]*baz \\(\\) at \[^\r\n\]+" \
> +	 "#1\[ \t\]*$hex in bar \\(\\) at \[^\r\n\]+" \
> +	 "#2\[ \t\]*$hex in foo \\(\\) at \[^\r\n\]+" \
> +	 "#3\[ \t\]*$hex in main \\(\\) at \[^\r\n\]+" ]
> diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
> index 5444ebb7f6a..e8a143fadab 100644
> --- a/gdb/xtensa-tdep.c
> +++ b/gdb/xtensa-tdep.c
> @@ -1262,7 +1262,8 @@ xtensa_frame_cache (frame_info_ptr this_frame, void **this_cache)
>         ws = get_frame_register_unsigned (this_frame,
>   					tdep->ws_regnum);
>   
> -      if (safe_read_memory_integer (pc, 1, byte_order, &op1)
> +      if (frame_relative_level (this_frame) == 0
> +	  && safe_read_memory_integer (pc, 1, byte_order, &op1)
>   	  && XTENSA_IS_ENTRY (gdbarch, op1))
>   	{
>   	  int callinc = CALLINC (ps);

I think this is ok.  In other places I believe the strategy to handle
tail calling frames where the return address is beyond the end of the
function is to use 'PC - 1' to lookup debug info instead, but you would
not want to do that for the first frame either in case you are stopped
at a function entry at the time of a stop.

-- 
John Baldwin