[1/2] Teach gdb how to unwind cygwin _sigbe and sigdelayed frames

Message ID 20240220165605.563516-2-pedro@palves.net
State New
Headers
Series Rework Cygwin signal handling |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Pedro Alves Feb. 20, 2024, 4:56 p.m. UTC
  From: Jon Turney <jon.turney@dronecode.org.uk>

The majority of functions in the cygwin DLL are wrapped by routines
which use an an alternate stack to return via a signal handler if a
signal occured while inside the function. (See [1],[2])

At present, these frames cannot be correctly unwound by gdb.  There
doesn't seem to currently be a way to correctly describe these frames
using DWARF CFI.

So instead, write a custom unwinder for _sigbe and sigdelayed frames,
which gets the return address from the alternate stack.

The offset of tls::stackptr from TIB.stacktop is determined by analyzing
the code in _sigbe or sigdelayed.

This can backtrace from _sigbe and from a sighandler through sigdelayed.

Implemented for amd64 and i386

Issues:

1. We should detect if we are in the wrapper after the return address
has been popped off the alternate stack, and if so, fetch the return
address from the register it's been popped into.

2. If there are multiple _sigbe or sigdelayed stack frames to be
unwound, this only unwinds the first one correctly, because we don't
unwind the value of the alternate stack pointer itself.

This is no worse than currently, when we can't even unwind one of
these frame correctly, but isn't quite correct.

I guess this could be handled by defining a pseudo-register to track
its value as we unwind the stack.

[1] https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/gendef
[2] https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/how-signals-work.txt

Co-Authored-By: Pedro Alves <pedro@palves.net>
Change-Id: I4a0d02c1b85d0aadaab2de3abd584eb4bda5b5cc
---
 gdb/amd64-windows-tdep.c |  26 ++++++
 gdb/i386-windows-tdep.c  |  20 ++++
 gdb/windows-tdep.c       | 194 +++++++++++++++++++++++++++++++++++++++
 gdb/windows-tdep.h       |  20 ++++
 4 files changed, 260 insertions(+)
  

Patch

diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 7c5169fd98c..af17886c080 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -1334,11 +1334,37 @@  amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_long_bit (gdbarch, 32);
 }
 
+/* Sigwrapper unwinder instruction patterns for AMD64.  */
+
+static const gdb_byte amd64_sigbe_bytes[] = {
+  0x49, 0xc7, 0xc3, 0xf8, 0xff, 0xff, 0xff,	/* movq $-8,%r11 */
+  0x4d, 0x0f, 0xc1, 0x9a,			/* xaddq %r11,$tls::stackptr(%r10) */
+  /* 4 bytes for tls::stackptr operand.  */
+};
+
+static const gdb_byte amd64_sigdelayed_bytes[] = {
+  0x49, 0xc7, 0xc3, 0xf8, 0xff, 0xff, 0xff,	/* movq $-8,%r11 */
+  0x4d, 0x0f, 0xc1, 0x9c, 0x24,			/* xaddq %r11,$tls::stackptr(%r12) */
+  /* 4 bytes for tls::stackptr operand.  */
+};
+
+static const gdb::array_view<const gdb_byte> amd64_sig_patterns[] {
+  { amd64_sigbe_bytes },
+  { amd64_sigdelayed_bytes },
+};
+
+/* The sigwrapper unwinder on AMD64.  */
+
+static const cygwin_sigwrapper_frame_unwind
+  amd64_cygwin_sigwrapper_frame_unwind (amd64_sig_patterns);
+
 /* gdbarch initialization for Cygwin on AMD64.  */
 
 static void
 amd64_cygwin_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
+  frame_unwind_append_unwinder (gdbarch, &amd64_cygwin_sigwrapper_frame_unwind);
+
   amd64_windows_init_abi_common (info, gdbarch);
   cygwin_init_abi (info, gdbarch);
 }
diff --git a/gdb/i386-windows-tdep.c b/gdb/i386-windows-tdep.c
index cd5c580a3e8..e8ed043be8a 100644
--- a/gdb/i386-windows-tdep.c
+++ b/gdb/i386-windows-tdep.c
@@ -27,6 +27,7 @@ 
 #include "xml-support.h"
 #include "gdbcore.h"
 #include "inferior.h"
+#include "frame-unwind.h"
 
 /* Core file support.  */
 
@@ -169,11 +170,30 @@  i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_push_dummy_call (gdbarch, i386_windows_push_dummy_call);
 }
 
+/* Sigwrapper unwinder instruction patterns for i386.  */
+
+static const gdb_byte i386_sigbe_bytes[] = {
+  0xb8, 0xfc, 0xff, 0xff, 0xff,			/* movl $-4,%eax */
+  0x0f, 0xc1, 0x83,				/* xadd %eax,$tls::stackptr(%ebx) */
+  /* 4 bytes for tls::stackptr operand.  */
+};
+
+static const gdb::array_view<const gdb_byte> i386_sig_patterns[] {
+  { i386_sigbe_bytes },
+};
+
+/* The sigwrapper unwinder on i386.  */
+
+static const cygwin_sigwrapper_frame_unwind
+  i386_cygwin_sigwrapper_frame_unwind (i386_sig_patterns);
+
 /* gdbarch initialization for Cygwin on i386.  */
 
 static void
 i386_cygwin_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
+  frame_unwind_append_unwinder (gdbarch, &i386_cygwin_sigwrapper_frame_unwind);
+
   i386_windows_init_abi_common (info, gdbarch);
   cygwin_init_abi (info, gdbarch);
 }
diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index 19a42a49a98..361ccdd0dcb 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -32,6 +32,7 @@ 
 #include "gdb_bfd.h"
 #include "solib.h"
 #include "solib-target.h"
+#include "frame-unwind.h"
 #include "gdbcore.h"
 #include "coff/internal.h"
 #include "libcoff.h"
@@ -1212,3 +1213,196 @@  even if their meaning is unknown."),
      isn't another convenience variable of the same name.  */
   create_internalvar_type_lazy ("_tlb", &tlb_funcs, NULL);
 }
+
+/* Frame cache data for the cygwin sigwrapper unwinder.  */
+
+struct cygwin_sigwrapper_frame_cache
+{
+  CORE_ADDR prev_pc;
+  int tlsoffset;
+};
+
+/* Return true if the instructions at PC match the instructions bytes
+   in PATTERN.  Returns false otherwise.  */
+
+static bool
+insns_match_pattern (CORE_ADDR pc,
+		     const gdb::array_view<const gdb_byte> pattern)
+{
+  for (size_t i = 0; i < pattern.size (); i++)
+    {
+      gdb_byte buf;
+      if (target_read_code (pc + i, &buf, 1) != 0)
+	return false;
+      if (buf != pattern[i])
+	return false;
+    }
+  return true;
+}
+
+/* Helper for cygwin_sigwrapper_frame_cache.  Search for one of the
+   patterns in PATTERNS_LIST within [START, END).  If found, record
+   the tls offset found after the matched pattern in the instruction
+   stream, in *TLSOFFSET.  */
+
+static void
+cygwin_sigwrapper_frame_analyze
+  (struct gdbarch *gdbarch,
+   CORE_ADDR start, CORE_ADDR end,
+   gdb::array_view<const gdb::array_view<const gdb_byte>> patterns_list,
+   int *tlsoffset)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  *tlsoffset = 0;
+
+  for (CORE_ADDR addr = start; addr < end; addr++)
+    {
+      for (auto patterns : patterns_list)
+	{
+	  if (insns_match_pattern (addr, patterns))
+	    {
+	      /* The instruction sequence is followed by 4 bytes for
+		 tls::stackptr.  */
+	      gdb_byte tls_stackptr[4];
+	      if (target_read_code (addr + patterns.size (), tls_stackptr, 4) == 0)
+		{
+		  *tlsoffset = extract_signed_integer (tls_stackptr, 4, byte_order);
+
+		  frame_debug_printf ("matched pattern at %s, sigstackptroffset=%x",
+				      paddress (gdbarch, addr),
+				      *tlsoffset);
+		  break;
+		}
+	    }
+	}
+    }
+
+  /* XXX: Perhaps we should also note the address of the xaddq
+     instruction which pops the RA from the sigstack.  If PC is after
+     that, we should look in the appropriate register to get the RA,
+     not on the sigstack.  */
+}
+
+/* Fill THIS_CACHE using the cygwin sigwrapper unwinding data for
+   THIS_FRAME.  */
+
+static cygwin_sigwrapper_frame_cache *
+cygwin_sigwrapper_frame_cache (frame_info_ptr this_frame, void **this_cache)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  auto *cache = (struct cygwin_sigwrapper_frame_cache *) *this_cache;
+  const int len = gdbarch_addr_bit (gdbarch) / 8;
+
+  /* Get address of top of stack from thread information block.  */
+  CORE_ADDR thread_local_base;
+  target_get_tib_address (inferior_ptid, &thread_local_base);
+
+  CORE_ADDR stacktop
+    = read_memory_unsigned_integer (thread_local_base + len, len, byte_order);
+
+  frame_debug_printf ("TEB.stacktop=%s", paddress (gdbarch, stacktop));
+
+  /* Find cygtls, relative to stacktop, and read signalstackptr from
+     cygtls.  */
+  CORE_ADDR signalstackptr
+    = read_memory_unsigned_integer (stacktop + cache->tlsoffset,
+				    len, byte_order);
+
+  frame_debug_printf ("sigsp=%s", paddress (gdbarch, signalstackptr));
+
+  /* Read return address from signal stack.  */
+  cache->prev_pc
+    = read_memory_unsigned_integer (signalstackptr - len, len, byte_order);
+
+  frame_debug_printf ("ra=%s", paddress (gdbarch, cache->prev_pc));
+
+  return cache;
+}
+
+static struct value *
+cygwin_sigwrapper_frame_prev_register (const frame_info_ptr &this_frame,
+				       void **this_cache,
+				       int regnum)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  struct cygwin_sigwrapper_frame_cache *cache
+    = cygwin_sigwrapper_frame_cache (this_frame, this_cache);
+
+  frame_debug_printf ("%s for pc=%s",
+		      gdbarch_register_name (gdbarch, regnum),
+		      paddress (gdbarch, cache->prev_pc));
+
+  if (regnum == gdbarch_pc_regnum (gdbarch))
+    return frame_unwind_got_address (this_frame, regnum, cache->prev_pc);
+
+  return frame_unwind_got_register (this_frame, regnum, regnum);
+}
+
+static void
+cygwin_sigwrapper_frame_this_id (const frame_info_ptr &this_frame,
+				 void **this_cache,
+				 struct frame_id *this_id)
+{
+  *this_id = frame_id_build_unavailable_stack (get_frame_func (this_frame));
+}
+
+static int
+cygwin_sigwrapper_frame_sniffer (const struct frame_unwind *self_,
+				 const frame_info_ptr &this_frame,
+				 void **this_cache)
+{
+  const auto *self = (const struct cygwin_sigwrapper_frame_unwind *) self_;
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+
+  CORE_ADDR pc = get_frame_pc (this_frame);
+  const char *name;
+  CORE_ADDR start, end;
+  find_pc_partial_function (pc, &name, &start, &end);
+
+  if (name == nullptr)
+    return 0;
+
+  if (strcmp (name, "_sigbe") != 0
+      && strcmp (name, "__sigbe") != 0
+      && strcmp (name, "sigdelayed") != 0
+      && strcmp (name, "_sigdelayed") != 0)
+    return 0;
+
+  frame_debug_printf ("name=%s, start=%s, end=%s",
+		      name,
+		      paddress (gdbarch, start),
+		      paddress (gdbarch, end));
+
+  int tlsoffset;
+  cygwin_sigwrapper_frame_analyze (gdbarch, start, end, self->patterns_list,
+				   &tlsoffset);
+  if (tlsoffset == 0)
+    return 0;
+
+  frame_debug_printf ("sigstackptroffset=%x", tlsoffset);
+
+  auto *cache = FRAME_OBSTACK_ZALLOC (struct cygwin_sigwrapper_frame_cache);
+  cache->tlsoffset = tlsoffset;
+
+  *this_cache = cache;
+  cygwin_sigwrapper_frame_cache (this_frame, this_cache);
+
+  return 1;
+}
+
+/* Cygwin sigwapper unwinder.  */
+
+cygwin_sigwrapper_frame_unwind::cygwin_sigwrapper_frame_unwind
+  (gdb::array_view<const gdb::array_view<const gdb_byte>> patterns_list)
+    : frame_unwind (),
+      patterns_list (patterns_list)
+{
+  name = "cygwin sigwrapper";
+  type = NORMAL_FRAME;
+  stop_reason = default_frame_unwind_stop_reason;
+  this_id = cygwin_sigwrapper_frame_this_id;
+  prev_register = cygwin_sigwrapper_frame_prev_register;
+  sniffer = cygwin_sigwrapper_frame_sniffer;
+}
diff --git a/gdb/windows-tdep.h b/gdb/windows-tdep.h
index e786a1d3eb9..f122f7aaa61 100644
--- a/gdb/windows-tdep.h
+++ b/gdb/windows-tdep.h
@@ -18,6 +18,8 @@ 
 #ifndef WINDOWS_TDEP_H
 #define WINDOWS_TDEP_H
 
+#include "frame-unwind.h"
+
 struct gdbarch;
 
 extern struct cmd_list_element *info_w32_cmdlist;
@@ -55,4 +57,22 @@  extern void cygwin_init_abi (struct gdbarch_info info,
 
 extern bool is_linked_with_cygwin_dll (bfd *abfd);
 
+/* Cygwin sigwapper unwinder.  Unwinds signal frames over
+   sigbe/sigdelayed.  */
+
+struct cygwin_sigwrapper_frame_unwind : public frame_unwind
+{
+  explicit cygwin_sigwrapper_frame_unwind
+    (gdb::array_view<const gdb::array_view<const gdb_byte>> patterns_list);
+
+  /* Architecture-specific list of instruction patterns to match.
+     It's a list of patterns instead of single pattern because some
+     architectures want to match more than one function
+     (sigbe/sigdelayed & friends).  Each potential instruction
+     sequence is assumed to be followed by 4 bytes for tls::stackptr.
+     If any pattern in the list matches, then the frame is assumed to
+     be a sigwrapper frame.  */
+  gdb::array_view<const gdb::array_view<const gdb_byte>> patterns_list;
+};
+
 #endif