[PATCH v2] gdb/s390: Add packed-stack support to the backchain unwinder

[PATCH v2] gdb/s390: Add packed-stack support to the backchain unwinder

[Ilya Leoshkevich]

Regtested on s390x-redhat-linux.  Ok for master?



v1: https://sourceware.org/pipermail/gdb-patches/2023-October/203562.html
    https://sourceware.org/pipermail/gdb-patches/2023-November/204230.html

    Move variable declarations closer to their first usages, add a test
    (Keith).

    Ignore the return value of the 2nd try_backchain() call, it does
    not cause any warnings, since try_backchain() is not marked with
    __attribute__ ((__warn_unused_result__)).  Furthermore, the logic
    does not require any special actions if it fails.



Currently it's not possible to unwind s390 kernel stacks past eBPF
progs.  There is no DWARF debug info or symbols for eBPF progs,
therefore doing so requires using the backchain unwinder.  The kernel
uses the packed-stack ABI, which the backchain unwinder does not
support.

As far as unwinding is concerned, the difference between the
"regular" ABI and the packed-stack ABI are frame offsets for backchain,
stack pointer and program counter.

Parameterize the existing code that does the backchain unwinding by
these three values; try the "regular" ones first, and the packed-stack
ones as a fallback.  The backchain unwinder is tried last, so this
addition does not alter the unwinding flow, unless it's about to fail
anyway.

While at it, move variable declarations closer to their first usages in
order to better match the modern GDB code style.

Add a test.

Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/s390-tdep.c                           | 82 +++++++++++++++--------
 gdb/testsuite/gdb.arch/s390-backchain.S   | 60 +++++++++++++++++
 gdb/testsuite/gdb.arch/s390-backchain.exp | 40 +++++++++++
 3 files changed, 153 insertions(+), 29 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/s390-backchain.S
 create mode 100644 gdb/testsuite/gdb.arch/s390-backchain.exp

diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 54b5c89e5e3..2e97a52690e 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -2520,49 +2520,41 @@ s390_prologue_frame_unwind_cache (frame_info_ptr this_frame,
   return 1;
 }
 
-/* Unwind THIS_FRAME and write the information into unwind cache INFO using
-   back chain unwinding.  Helper for s390_frame_unwind_cache.  */
+/* Unwind THIS_FRAME using back chain unwinding for the ABI described by
+   WORD_SIZE, BYTE_ORDER, BACKCHAIN_IDX, SP_IDX and PC_IDX.  If successful,
+   write the information into unwind cache INFO and return true, otherwise
+   return false.  Helper for s390_backchain_frame_unwind_cache.  */
 
-static void
-s390_backchain_frame_unwind_cache (frame_info_ptr this_frame,
-				   struct s390_unwind_cache *info)
+static bool
+try_backchain (frame_info_ptr this_frame, struct s390_unwind_cache *info,
+	       int word_size, enum bfd_endian byte_order, int backchain_idx,
+	       int sp_idx, int pc_idx)
 {
-  struct gdbarch *gdbarch = get_frame_arch (this_frame);
-  int word_size = gdbarch_ptr_bit (gdbarch) / 8;
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  CORE_ADDR backchain;
-  ULONGEST reg;
-  LONGEST sp, tmp;
-  int i;
-
-  /* Set up ABI call-saved/call-clobbered registers.  */
-  for (i = 0; i < S390_NUM_REGS; i++)
-    if (!s390_register_call_saved (gdbarch, i))
-      info->saved_regs[i].set_unknown ();
-
-  /* CC is always call-clobbered.  */
-  info->saved_regs[S390_PSWM_REGNUM].set_unknown ();
-
   /* Get the backchain.  */
-  reg = get_frame_register_unsigned (this_frame, S390_SP_REGNUM);
-  if (!safe_read_memory_integer (reg, word_size, byte_order, &tmp))
+  ULONGEST reg = get_frame_register_unsigned (this_frame, S390_SP_REGNUM);
+  LONGEST tmp;
+  if (!safe_read_memory_integer (reg + backchain_idx * word_size, word_size,
+				 byte_order, &tmp))
     tmp = 0;
-  backchain = (CORE_ADDR) tmp;
+  CORE_ADDR backchain = (CORE_ADDR) tmp;
 
   /* A zero backchain terminates the frame chain.  As additional
      sanity check, let's verify that the spill slot for SP in the
      save area pointed to by the backchain in fact links back to
      the save area.  */
+  LONGEST sp;
   if (backchain != 0
-      && safe_read_memory_integer (backchain + 15*word_size,
-				   word_size, byte_order, &sp)
+      && safe_read_memory_integer (backchain + sp_idx * word_size, word_size,
+				   byte_order, &sp)
       && (CORE_ADDR)sp == backchain)
     {
       /* We don't know which registers were saved, but it will have
 	 to be at least %r14 and %r15.  This will allow us to continue
 	 unwinding, but other prev-frame registers may be incorrect ...  */
-      info->saved_regs[S390_SP_REGNUM].set_addr (backchain + 15*word_size);
-      info->saved_regs[S390_RETADDR_REGNUM].set_addr (backchain + 14*word_size);
+      info->saved_regs[S390_SP_REGNUM].set_addr (backchain
+						 + sp_idx * word_size);
+      info->saved_regs[S390_RETADDR_REGNUM].set_addr (backchain
+						      + pc_idx * word_size);
 
       /* Function return will set PC to %r14.  */
       info->saved_regs[S390_PSWA_REGNUM]
@@ -2570,10 +2562,42 @@ s390_backchain_frame_unwind_cache (frame_info_ptr this_frame,
 
       /* We use the current value of the frame register as local_base,
 	 and the top of the register save area as frame_base.  */
-      info->frame_base = backchain + 16*word_size + 32;
+      const int gpr_count = 16;
+      const int fpr_count = 4;
+      info->frame_base = backchain + gpr_count * word_size + fpr_count * 8;
       info->local_base = reg;
+
+      return true;
     }
 
+  return false;
+}
+
+/* Unwind THIS_FRAME and write the information into unwind cache INFO using
+   back chain unwinding.  Helper for s390_frame_unwind_cache.  */
+
+static void
+s390_backchain_frame_unwind_cache (frame_info_ptr this_frame,
+				   struct s390_unwind_cache *info)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  int word_size = gdbarch_ptr_bit (gdbarch) / 8;
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  int i;
+
+  /* Set up ABI call-saved/call-clobbered registers.  */
+  for (i = 0; i < S390_NUM_REGS; i++)
+    if (!s390_register_call_saved (gdbarch, i))
+      info->saved_regs[i].set_unknown ();
+
+  /* CC is always call-clobbered.  */
+  info->saved_regs[S390_PSWM_REGNUM].set_unknown ();
+
+  /* Try the regular ABI.  */
+  if (!try_backchain (this_frame, info, word_size, byte_order, 0, 15, 14))
+    /* Try the packed stack ABI.  */
+    try_backchain (this_frame, info, word_size, byte_order, 19, 18, 17);
+
   info->func = get_frame_pc (this_frame);
 }
 
diff --git a/gdb/testsuite/gdb.arch/s390-backchain.S b/gdb/testsuite/gdb.arch/s390-backchain.S
new file mode 100644
index 00000000000..e006705fff9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/s390-backchain.S
@@ -0,0 +1,60 @@
+/* Copyright 2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+ok:
+	j	0f
+	nop
+0:
+	xgr	%r2,%r2
+	br	%r14
+
+f:
+	j	0f
+	nop
+0:
+	stmg	%r14,%r15,112(%r15)
+	lgr	%r14,%r15
+	lay	%r15,-160(%r15)
+	stg	%r14,0(%r15)
+	brasl	%r14,ok
+	lmg	%r14,%r15,272(%r15)
+	br	%r14
+
+f_packed:
+	j	0f
+	nop
+0:
+	stmg	%r14,%r15,136(%r15)
+	lgr	%r14,%r15
+	lay	%r15,-24(%r15)
+	stg	%r14,152(%r15)
+	brasl	%r14,f
+	lmg	%r14,%r15,160(%r15)
+	br	%r14
+
+	.globl main
+main:
+	j	0f
+	nop
+0:
+	stmg	%r14,%r15,112(%r15)
+	lgr	%r14,%r15
+	lay	%r15,-160(%r15)
+	stg	%r14,0(%r15)
+	brasl	%r14,f_packed
+	lmg	%r14,%r15,272(%r15)
+	br	%r14
diff --git a/gdb/testsuite/gdb.arch/s390-backchain.exp b/gdb/testsuite/gdb.arch/s390-backchain.exp
new file mode 100644
index 00000000000..9dd2e876238
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/s390-backchain.exp
@@ -0,0 +1,40 @@
+# Copyright 2023 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 unwinding of mixed normal and packed-stack functions with backchain.
+# This is an extreme case that does not occur in practice, but there is no
+# reason for GDB not to support it.
+# The actual use case is JITed eBPF code in the Linux kernel.  The testcase
+# reflects all its peculiarities: packed stack, no debuginfo, non-standard
+# prologue that confuses s390_analyze_prologue().
+
+require {istarget "s390*-*-*"}
+
+standard_testfile .S
+
+# Compile without the debug option.
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile {}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint "ok"
+gdb_continue_to_breakpoint "ok"
+gdb_test "backtrace" \
+    "#0\[ \t\]*$hex in ok .*\n#1\[ \t\]*$hex in f .*\n#2\[ \t\]*$hex in f_packed .*\n#3\[ \t\]*$hex in main .*" \
+    "backtrace"
-- 
2.43.0

Re: [PATCH v2] gdb/s390: Add packed-stack support to the backchain unwinder

[Keith Seitz]

Hi,

Thank you for your patience. It comes sometimes be a very lengthy
process to get patches accepted.

Since I've already "granted" (???) my Reviewed-by, this is effectively
just a "ping" for you.

On 11/28/23 14:36, Ilya Leoshkevich wrote:
> Regtested on s390x-redhat-linux.  Ok for master?
> 
> 
> 
> v1: https://sourceware.org/pipermail/gdb-patches/2023-October/203562.html
>      https://sourceware.org/pipermail/gdb-patches/2023-November/204230.html
> 
>      Move variable declarations closer to their first usages, add a test
>      (Keith).
> 
>      Ignore the return value of the 2nd try_backchain() call, it does
>      not cause any warnings, since try_backchain() is not marked with
>      __attribute__ ((__warn_unused_result__)).  Furthermore, the logic
>      does not require any special actions if it fails.

I've retested this internally and reviewed the revision, and everything
looks okay.

I hope a global  or area maintainer will be able to help push this 
forward. forward.

Keith