* [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