public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Rework Cygwin signal handling
@ 2024-02-20 16:56 Pedro Alves
  2024-02-20 16:56 ` [PATCH 1/2] Teach gdb how to unwind cygwin _sigbe and sigdelayed frames Pedro Alves
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Pedro Alves @ 2024-02-20 16:56 UTC (permalink / raw)
  To: gdb-patches

This upstreams a couple GDB patches that Cygwin has been carrying
downstream.

In my Windows non-stop work, I was having trouble with the
have_saved_context machinery, I couldn't get it to work properly.  The
Cygwin distro gdb was able to unwind signals properly, but my GDB did
not.  After some head banging, I realized that Cygwin gdb may have
some downstream patches.  And indeed it does.  One of those patches
eliminates the have_saved_context machinery...  I got signal handling
and non-stop working properly on top of that patch, which then means
that I need that change upstream as well, if I am to upstream my
non-stop changes...  So here we are.  That patch is patch #2 in this
series.

That have_saved_context patch depends on another downstream patch,
which is patch #1 here.  The version I show here is a polished,
modernized version compared to the downstream version, but the main
logic is the same.  While this is not perfect, it is certainly better
than what we have upstream, which just isn't able to unwind from a
signal handler at all.  Cygwin has been carrying these patches for
many years, so while we could think about improving all this, I see no
reason for holding back the patch as is.  We can always improve on
top, and we should be able to do that upstream.

Jon Turney (2):
  Teach gdb how to unwind cygwin _sigbe and sigdelayed frames
  Drop special way of getting inferior context after a Cygwin signal

 gdb/amd64-windows-tdep.c |  26 ++++++
 gdb/i386-windows-tdep.c  |  20 ++++
 gdb/windows-nat.c        |  52 +++--------
 gdb/windows-tdep.c       | 194 +++++++++++++++++++++++++++++++++++++++
 gdb/windows-tdep.h       |  20 ++++
 5 files changed, 275 insertions(+), 37 deletions(-)


base-commit: 94a75b0363b1e09416e9bd24cac72d98864688d8
-- 
2.43.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] Teach gdb how to unwind cygwin _sigbe and sigdelayed frames
  2024-02-20 16:56 [PATCH 0/2] Rework Cygwin signal handling Pedro Alves
@ 2024-02-20 16:56 ` Pedro Alves
  2024-02-20 16:56 ` [PATCH 2/2] Drop special way of getting inferior context after a Cygwin signal Pedro Alves
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2024-02-20 16:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves

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(+)

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] Drop special way of getting inferior context after a Cygwin signal
  2024-02-20 16:56 [PATCH 0/2] Rework Cygwin signal handling Pedro Alves
  2024-02-20 16:56 ` [PATCH 1/2] Teach gdb how to unwind cygwin _sigbe and sigdelayed frames Pedro Alves
@ 2024-02-20 16:56 ` Pedro Alves
  2024-02-21 13:25 ` [PATCH 0/2] Rework Cygwin signal handling Jon Turney
  2024-02-21 21:14 ` Tom Tromey
  3 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2024-02-20 16:56 UTC (permalink / raw)
  To: gdb-patches

From: Jon Turney <jon.turney@dronecode.org.uk>

Simplify Cygwin signal handling by dropping the special way of getting
inferior context after a Cygwin signal.

I think the reason this existed was because previously we were not
able to unwind through the alternate stack used by _sigfe frames, so
without the hint of the "user" code IP, the backtrace from a signal
was unusable.

Now we can unwind through _sigfe frames, drop all this complexity.

(Restoring this specially obtained context to the inferior (as the
code currently does) skips over the actual signal delivery and
handling.  Cygwin has carried for a long time a patch which clears the
ContextFlags in the signal context, so we never attempt to restore it
to the inferior, but that interfers with gdb's ability to modify that
context e.g. if it decides it wants to turn on FLAG_TRACE_BIT.)

Change-Id: I214edd5a99fd17c1a31ad18138d4a6cc420225e3
---
 gdb/windows-nat.c | 52 ++++++++++++++---------------------------------
 1 file changed, 15 insertions(+), 37 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 7f3044fc61d..a90388922e2 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -98,10 +98,6 @@ struct windows_per_inferior : public windows_process_info
   void handle_unload_dll () override;
   bool handle_access_violation (const EXCEPTION_RECORD *rec) override;
 
-
-  int have_saved_context = 0;	/* True if we've saved context from a
-				   cygwin signal.  */
-
   uintptr_t dr[8] {};
 
   int windows_initialization_done = 0;
@@ -140,9 +136,6 @@ struct windows_per_inferior : public windows_process_info
   std::vector<windows_solib> solibs;
 
 #ifdef __CYGWIN__
-  CONTEXT saved_context {};	/* Contains the saved context from a
-				   cygwin signal.  */
-
   /* The starting and ending address of the cygwin1.dll text segment.  */
   CORE_ADDR cygwin_load_start = 0;
   CORE_ADDR cygwin_load_end = 0;
@@ -718,19 +711,6 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
 
   if (th->reload_context)
     {
-#ifdef __CYGWIN__
-      if (windows_process.have_saved_context)
-	{
-	  /* Lie about where the program actually is stopped since
-	     cygwin has informed us that we should consider the signal
-	     to have occurred at another location which is stored in
-	     "saved_context.  */
-	  memcpy (&th->context, &windows_process.saved_context,
-		  __COPY_CONTEXT_SIZE);
-	  windows_process.have_saved_context = 0;
-	}
-      else
-#endif
 #ifdef __x86_64__
       if (windows_process.wow64_process)
 	{
@@ -1048,33 +1028,32 @@ windows_per_inferior::handle_output_debug_string
 #ifdef __CYGWIN__
   else
     {
-      /* Got a cygwin signal marker.  A cygwin signal is followed by
-	 the signal number itself and then optionally followed by the
-	 thread id and address to saved context within the DLL.  If
-	 these are supplied, then the given thread is assumed to have
-	 issued the signal and the context from the thread is assumed
-	 to be stored at the given address in the inferior.  Tell gdb
-	 to treat this like a real signal.  */
+      /* Got a cygwin signal marker.  A cygwin signal marker is
+	 followed by the signal number itself, and (since Cygwin 1.7)
+	 the thread id, and the address of a saved context in the
+	 inferior (That context has an IP which is the return address
+	 in "user" code of the cygwin internal signal handling code,
+	 but is not otherwise usable).
+
+	 Tell gdb to treat this like the given thread issued a real
+	 signal.  */
       char *p;
       int sig = strtol (s.get () + sizeof (_CYGWIN_SIGNAL_STRING) - 1, &p, 0);
       gdb_signal gotasig = gdb_signal_from_host (sig);
+      LPCVOID x = 0;
 
       if (gotasig)
 	{
-	  LPCVOID x;
-	  SIZE_T n;
-
 	  ourstatus->set_stopped (gotasig);
 	  retval = strtoul (p, &p, 0);
 	  if (!retval)
 	    retval = current_event.dwThreadId;
-	  else if ((x = (LPCVOID) (uintptr_t) strtoull (p, NULL, 0))
-		   && ReadProcessMemory (handle, x,
-					 &saved_context,
-					 __COPY_CONTEXT_SIZE, &n)
-		   && n == __COPY_CONTEXT_SIZE)
-	    have_saved_context = 1;
+	  else
+	    x = (LPCVOID) (uintptr_t) strtoull (p, NULL, 0);
 	}
+
+      DEBUG_EVENTS ("gdb: cygwin signal %d, thread 0x%x, CONTEXT @ %p",
+		    gotasig, retval, x);
     }
 #endif
 
@@ -1607,7 +1586,6 @@ windows_nat_target::get_windows_debug_event
 
   event_code = windows_process.current_event.dwDebugEventCode;
   ourstatus->set_spurious ();
-  windows_process.have_saved_context = 0;
 
   switch (event_code)
     {
-- 
2.43.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Rework Cygwin signal handling
  2024-02-20 16:56 [PATCH 0/2] Rework Cygwin signal handling Pedro Alves
  2024-02-20 16:56 ` [PATCH 1/2] Teach gdb how to unwind cygwin _sigbe and sigdelayed frames Pedro Alves
  2024-02-20 16:56 ` [PATCH 2/2] Drop special way of getting inferior context after a Cygwin signal Pedro Alves
@ 2024-02-21 13:25 ` Jon Turney
  2024-02-23 16:24   ` Pedro Alves
  2024-02-21 21:14 ` Tom Tromey
  3 siblings, 1 reply; 7+ messages in thread
From: Jon Turney @ 2024-02-21 13:25 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 20/02/2024 16:56, Pedro Alves wrote:
> This upstreams a couple GDB patches that Cygwin has been carrying
> downstream.
> 
> In my Windows non-stop work, I was having trouble with the
> have_saved_context machinery, I couldn't get it to work properly.  The
> Cygwin distro gdb was able to unwind signals properly, but my GDB did
> not.  After some head banging, I realized that Cygwin gdb may have
> some downstream patches.  And indeed it does.  One of those patches
> eliminates the have_saved_context machinery...  I got signal handling
> and non-stop working properly on top of that patch, which then means
> that I need that change upstream as well, if I am to upstream my
> non-stop changes...  So here we are.  That patch is patch #2 in this
> series.
> 
> That have_saved_context patch depends on another downstream patch,
> which is patch #1 here.  The version I show here is a polished,
> modernized version compared to the downstream version, but the main
> logic is the same.  While this is not perfect, it is certainly better
> than what we have upstream, which just isn't able to unwind from a
> signal handler at all.  Cygwin has been carrying these patches for

Thanks very much for polishing and modernising this patch.

Just to note that the situation is even worse than noted here: as well 
as being unable to unwind in a signal handler, without this patch gdb 
also can't unwind from functions inside the cygwin DLL where the call 
stack passes through these wrappers (which is nearly all of them, which 
makes debugging problems there even more painful).

Obviously, I am very much in favour of this being applied. :)

> many years, so while we could think about improving all this, I see no
> reason for holding back the patch as is.  We can always improve on
> top, and we should be able to do that upstream.
> 
> Jon Turney (2):
>    Teach gdb how to unwind cygwin _sigbe and sigdelayed frames
>    Drop special way of getting inferior context after a Cygwin signal
> 
>   gdb/amd64-windows-tdep.c |  26 ++++++
>   gdb/i386-windows-tdep.c  |  20 ++++
>   gdb/windows-nat.c        |  52 +++--------
>   gdb/windows-tdep.c       | 194 +++++++++++++++++++++++++++++++++++++++
>   gdb/windows-tdep.h       |  20 ++++
>   5 files changed, 275 insertions(+), 37 deletions(-)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Rework Cygwin signal handling
  2024-02-20 16:56 [PATCH 0/2] Rework Cygwin signal handling Pedro Alves
                   ` (2 preceding siblings ...)
  2024-02-21 13:25 ` [PATCH 0/2] Rework Cygwin signal handling Jon Turney
@ 2024-02-21 21:14 ` Tom Tromey
  2024-02-23 16:25   ` Pedro Alves
  3 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2024-02-21 21:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> This upstreams a couple GDB patches that Cygwin has been carrying
Pedro> downstream.

I don't know anything about Cygwin, but considering that these seem
fairly local to that code; and Jon reports their necessity; and they
remove some special handling from windows-nat.c, I'm in favor of you
putting them in.

thanks,
Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Rework Cygwin signal handling
  2024-02-21 13:25 ` [PATCH 0/2] Rework Cygwin signal handling Jon Turney
@ 2024-02-23 16:24   ` Pedro Alves
  0 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2024-02-23 16:24 UTC (permalink / raw)
  To: Jon Turney, gdb-patches

On 2024-02-21 13:25, Jon Turney wrote:

> Thanks very much for polishing and modernising this patch.
> 

Thanks for writing it in the first place.  :-)

> Just to note that the situation is even worse than noted here: as well as being unable to unwind in a signal handler, without this patch gdb also can't unwind from functions inside the cygwin DLL where the call stack passes through these wrappers (which is nearly all of them, which makes debugging problems there even more painful).

Oh, indeed.  I was too focused on the tree, missed the forest.

> Obviously, I am very much in favour of this being applied. :)

:-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Rework Cygwin signal handling
  2024-02-21 21:14 ` Tom Tromey
@ 2024-02-23 16:25   ` Pedro Alves
  0 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2024-02-23 16:25 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Jon Turney

On 2024-02-21 21:14, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> This upstreams a couple GDB patches that Cygwin has been carrying
> Pedro> downstream.
> 
> I don't know anything about Cygwin, but considering that these seem
> fairly local to that code; and Jon reports their necessity; and they
> remove some special handling from windows-nat.c, I'm in favor of you
> putting them in.
> 

Great, thanks.  I've merged this now.

Pedro Alves

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-02-23 16:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-20 16:56 [PATCH 0/2] Rework Cygwin signal handling Pedro Alves
2024-02-20 16:56 ` [PATCH 1/2] Teach gdb how to unwind cygwin _sigbe and sigdelayed frames Pedro Alves
2024-02-20 16:56 ` [PATCH 2/2] Drop special way of getting inferior context after a Cygwin signal Pedro Alves
2024-02-21 13:25 ` [PATCH 0/2] Rework Cygwin signal handling Jon Turney
2024-02-23 16:24   ` Pedro Alves
2024-02-21 21:14 ` Tom Tromey
2024-02-23 16:25   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).