[PATCH v3 00/29] Windows code sharing + bug fix

[PATCH v3 00/29] Windows code sharing + bug fix

[Tom Tromey]

This is v3 of the series to share a lot of the Windows code between
gdb and gdbserver.  It also fixes a bug that a customer reported; in
fact this fix was the origin of this series.  See patch #11 for
details on the bug.

I think this addresses all the review comments.  It's somewhat hard to
be sure since they were done in gerrit, and extracting comments from
that is a pain.  Also I think I had sent v2 before I updated my
scripts to preserve the gerrit change ID, so it is in gerrit twice.
Anyway, the reviews happened around Nov 2019, so you can find some in
the mailing list archives.

I tested this largely by hand.  I also ran it through the AdaCore test
suite, where it did ok.  (There were some issues, but I think they are
largely unrelated; some things like gdb printing paths that the test
suite didn't really recognize.)

I've also updated this to handle the WOW64 stuff, but only in a
minimal way -- I didn't try to make this work in gdbserver.

Let me know what you think.

Tom

[PATCH v3 01/29] Remove the "next" field from windows_thread_info

[Tom Tromey]

This changes windows_thread_info to remove the "next" field, replacing
the linked list of threads with a vector.  This is a prerequisite to
sharing this structure with gdbserver, which manages threads
differently.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (struct windows_thread_info): Remove typedef.
	(thread_head): Remove.
	(thread_list): New global.
	(thread_rec, windows_add_thread, windows_init_thread_list)
	(windows_delete_thread, windows_continue): Update.
---
 gdb/ChangeLog     |  8 ++++++++
 gdb/windows-nat.c | 52 +++++++++++++++++++----------------------------
 2 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 614b235edea..8b993912713 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -43,6 +43,7 @@
 #include <cygwin/version.h>
 #endif
 #include <algorithm>
+#include <vector>
 
 #include "filenames.h"
 #include "symfile.h"
@@ -245,9 +246,8 @@ static enum gdb_signal last_sig = GDB_SIGNAL_0;
 
 /* Thread information structure used to track information that is
    not available in gdb's thread structure.  */
-typedef struct windows_thread_info_struct
+struct windows_thread_info
   {
-    struct windows_thread_info_struct *next;
     DWORD id;
     HANDLE h;
     CORE_ADDR thread_local_base;
@@ -261,10 +261,9 @@ typedef struct windows_thread_info_struct
 	WOW64_CONTEXT wow64_context;
 #endif
       };
-  }
-windows_thread_info;
+  };
 
-static windows_thread_info thread_head;
+static std::vector<windows_thread_info *> thread_list;
 
 /* The process and thread handles for the above context.  */
 
@@ -429,9 +428,7 @@ check (BOOL ok, const char *file, int line)
 static windows_thread_info *
 thread_rec (DWORD id, int get_context)
 {
-  windows_thread_info *th;
-
-  for (th = &thread_head; (th = th->next) != NULL;)
+  for (windows_thread_info *th : thread_list)
     if (th->id == id)
       {
 	if (!th->suspended && get_context)
@@ -499,8 +496,7 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
   if (wow64_process)
     th->thread_local_base += 0x2000;
 #endif
-  th->next = thread_head.next;
-  thread_head.next = th;
+  thread_list.push_back (th);
 
   /* Add this new thread to the list of threads.
 
@@ -554,17 +550,13 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
 static void
 windows_init_thread_list (void)
 {
-  windows_thread_info *th = &thread_head;
-
   DEBUG_EVENTS (("gdb: windows_init_thread_list\n"));
   init_thread_list ();
-  while (th->next != NULL)
-    {
-      windows_thread_info *here = th->next;
-      th->next = here->next;
-      xfree (here);
-    }
-  thread_head.next = NULL;
+
+  for (windows_thread_info *here : thread_list)
+    xfree (here);
+
+  thread_list.clear ();
 }
 
 /* Delete a thread from the list of threads.
@@ -577,7 +569,6 @@ windows_init_thread_list (void)
 static void
 windows_delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p)
 {
-  windows_thread_info *th;
   DWORD id;
 
   gdb_assert (ptid.tid () != 0);
@@ -600,17 +591,17 @@ windows_delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p)
 
   delete_thread (find_thread_ptid (&the_windows_nat_target, ptid));
 
-  for (th = &thread_head;
-       th->next != NULL && th->next->id != id;
-       th = th->next)
-    continue;
+  auto iter = std::find_if (thread_list.begin (), thread_list.end (),
+			    [=] (windows_thread_info *th)
+			    {
+			      return th->id == id;
+			    });
 
-  if (th->next != NULL)
+  if (iter != thread_list.end ())
     {
-      windows_thread_info *here = th->next;
-      th->next = here->next;
-      xfree (here->name);
-      xfree (here);
+      xfree ((*iter)->name);
+      xfree (*iter);
+      thread_list.erase (iter);
     }
 }
 
@@ -1477,7 +1468,6 @@ handle_exception (struct target_waitstatus *ourstatus)
 static BOOL
 windows_continue (DWORD continue_status, int id, int killed)
 {
-  windows_thread_info *th;
   BOOL res;
 
   DEBUG_EVENTS (("ContinueDebugEvent (cpid=%d, ctid=0x%x, %s);\n",
@@ -1486,7 +1476,7 @@ windows_continue (DWORD continue_status, int id, int killed)
 		  continue_status == DBG_CONTINUE ?
 		  "DBG_CONTINUE" : "DBG_EXCEPTION_NOT_HANDLED"));
 
-  for (th = &thread_head; (th = th->next) != NULL;)
+  for (windows_thread_info *th : thread_list)
     if ((id == -1 || id == (int) th->id)
 	&& th->suspended)
       {
-- 
2.21.1

[PATCH v3 02/29] Rename win32_thread_info to windows_thread_info

[Tom Tromey]

This renames win32_thread_info to windows_thread_info in gdbserver.
This renaming helps make it possible to share some code between gdb
and gdbserver.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.h (struct windows_thread_info): Rename from
	win32_thread_info.  Remove typedef.
	(struct win32_target_ops, win32_require_context): Update.
	* win32-low.c (win32_get_thread_context)
	(win32_set_thread_context, win32_prepare_to_resume)
	(win32_require_context, thread_rec, child_add_thread)
	(delete_thread_info, continue_one_thread)
	(child_fetch_inferior_registers, child_store_inferior_registers)
	(win32_resume, suspend_one_thread, win32_get_tib_address):
	Update.
	* win32-i386-low.c (update_debug_registers)
	(win32_get_current_dr, i386_get_thread_context)
	(i386_prepare_to_resume, i386_thread_added, i386_single_step)
	(i386_fetch_inferior_register, i386_store_inferior_register):
	Update.
	* win32-arm-low.c (arm_get_thread_context)
	(arm_fetch_inferior_register, arm_store_inferior_register):
	Update.
---
 gdbserver/ChangeLog         | 21 +++++++++++++++++++++
 gdbserver/win32-arm-low.cc  |  6 +++---
 gdbserver/win32-i386-low.cc | 18 +++++++++---------
 gdbserver/win32-low.cc      | 32 ++++++++++++++++----------------
 gdbserver/win32-low.h       | 18 +++++++++---------
 5 files changed, 58 insertions(+), 37 deletions(-)

diff --git a/gdbserver/win32-arm-low.cc b/gdbserver/win32-arm-low.cc
index 619847d10fc..c50c5dfdbf2 100644
--- a/gdbserver/win32-arm-low.cc
+++ b/gdbserver/win32-arm-low.cc
@@ -27,7 +27,7 @@ void init_registers_arm (void);
 extern const struct target_desc *tdesc_arm;
 
 static void
-arm_get_thread_context (win32_thread_info *th)
+arm_get_thread_context (windows_thread_info *th)
 {
   th->context.ContextFlags = \
     CONTEXT_FULL | \
@@ -88,7 +88,7 @@ regptr (CONTEXT* c, int r)
 /* Fetch register from gdbserver regcache data.  */
 static void
 arm_fetch_inferior_register (struct regcache *regcache,
-			     win32_thread_info *th, int r)
+			     windows_thread_info *th, int r)
 {
   char *context_offset = regptr (&th->context, r);
   supply_register (regcache, r, context_offset);
@@ -97,7 +97,7 @@ arm_fetch_inferior_register (struct regcache *regcache,
 /* Store a new register value into the thread context of TH.  */
 static void
 arm_store_inferior_register (struct regcache *regcache,
-			     win32_thread_info *th, int r)
+			     windows_thread_info *th, int r)
 {
   collect_register (regcache, r, regptr (&th->context, r));
 }
diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
index f5f09e96a57..29ee49fcd03 100644
--- a/gdbserver/win32-i386-low.cc
+++ b/gdbserver/win32-i386-low.cc
@@ -40,7 +40,7 @@ static struct x86_debug_reg_state debug_reg_state;
 static void
 update_debug_registers (thread_info *thread)
 {
-  win32_thread_info *th = (win32_thread_info *) thread_target_data (thread);
+  windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
 
   /* The actual update is done later just before resuming the lwp,
      we just mark that the registers need updating.  */
@@ -73,8 +73,8 @@ x86_dr_low_set_control (unsigned long control)
 static DWORD64
 win32_get_current_dr (int dr)
 {
-  win32_thread_info *th
-    = (win32_thread_info *) thread_target_data (current_thread);
+  windows_thread_info *th
+    = (windows_thread_info *) thread_target_data (current_thread);
 
   win32_require_context (th);
 
@@ -210,7 +210,7 @@ i386_initial_stuff (void)
 }
 
 static void
-i386_get_thread_context (win32_thread_info *th)
+i386_get_thread_context (windows_thread_info *th)
 {
   /* Requesting the CONTEXT_EXTENDED_REGISTERS register set fails if
      the system doesn't support extended registers.  */
@@ -237,7 +237,7 @@ i386_get_thread_context (win32_thread_info *th)
 }
 
 static void
-i386_prepare_to_resume (win32_thread_info *th)
+i386_prepare_to_resume (windows_thread_info *th)
 {
   if (th->debug_registers_changed)
     {
@@ -258,13 +258,13 @@ i386_prepare_to_resume (win32_thread_info *th)
 }
 
 static void
-i386_thread_added (win32_thread_info *th)
+i386_thread_added (windows_thread_info *th)
 {
   th->debug_registers_changed = 1;
 }
 
 static void
-i386_single_step (win32_thread_info *th)
+i386_single_step (windows_thread_info *th)
 {
   th->context.EFlags |= FLAG_TRACE_BIT;
 }
@@ -398,7 +398,7 @@ static const int mappings[] =
 /* Fetch register from gdbserver regcache data.  */
 static void
 i386_fetch_inferior_register (struct regcache *regcache,
-			      win32_thread_info *th, int r)
+			      windows_thread_info *th, int r)
 {
   char *context_offset = (char *) &th->context + mappings[r];
 
@@ -420,7 +420,7 @@ i386_fetch_inferior_register (struct regcache *regcache,
 /* Store a new register value into the thread context of TH.  */
 static void
 i386_store_inferior_register (struct regcache *regcache,
-			      win32_thread_info *th, int r)
+			      windows_thread_info *th, int r)
 {
   char *context_offset = (char *) &th->context + mappings[r];
   collect_register (regcache, r, context_offset);
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 8b2a16e86dc..55e8322cebb 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -125,7 +125,7 @@ debug_event_ptid (DEBUG_EVENT *event)
 /* Get the thread context of the thread associated with TH.  */
 
 static void
-win32_get_thread_context (win32_thread_info *th)
+win32_get_thread_context (windows_thread_info *th)
 {
   memset (&th->context, 0, sizeof (CONTEXT));
   (*the_low_target.get_thread_context) (th);
@@ -137,7 +137,7 @@ win32_get_thread_context (win32_thread_info *th)
 /* Set the thread context of the thread associated with TH.  */
 
 static void
-win32_set_thread_context (win32_thread_info *th)
+win32_set_thread_context (windows_thread_info *th)
 {
 #ifdef _WIN32_WCE
   /* Calling SuspendThread on a thread that is running kernel code
@@ -158,7 +158,7 @@ win32_set_thread_context (win32_thread_info *th)
 /* Set the thread context of the thread associated with TH.  */
 
 static void
-win32_prepare_to_resume (win32_thread_info *th)
+win32_prepare_to_resume (windows_thread_info *th)
 {
   if (the_low_target.prepare_to_resume != NULL)
     (*the_low_target.prepare_to_resume) (th);
@@ -167,7 +167,7 @@ win32_prepare_to_resume (win32_thread_info *th)
 /* See win32-low.h.  */
 
 void
-win32_require_context (win32_thread_info *th)
+win32_require_context (windows_thread_info *th)
 {
   if (th->context.ContextFlags == 0)
     {
@@ -189,30 +189,30 @@ win32_require_context (win32_thread_info *th)
 
 /* Find a thread record given a thread id.  If GET_CONTEXT is set then
    also retrieve the context for this thread.  */
-static win32_thread_info *
+static windows_thread_info *
 thread_rec (ptid_t ptid, int get_context)
 {
   thread_info *thread = find_thread_ptid (ptid);
   if (thread == NULL)
     return NULL;
 
-  win32_thread_info *th = (win32_thread_info *) thread_target_data (thread);
+  windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
   if (get_context)
     win32_require_context (th);
   return th;
 }
 
 /* Add a thread to the thread list.  */
-static win32_thread_info *
+static windows_thread_info *
 child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
 {
-  win32_thread_info *th;
+  windows_thread_info *th;
   ptid_t ptid = ptid_t (pid, tid, 0);
 
   if ((th = thread_rec (ptid, FALSE)))
     return th;
 
-  th = XCNEW (win32_thread_info);
+  th = XCNEW (windows_thread_info);
   th->tid = tid;
   th->h = h;
   th->thread_local_base = (CORE_ADDR) (uintptr_t) tlb;
@@ -229,7 +229,7 @@ child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
 static void
 delete_thread_info (thread_info *thread)
 {
-  win32_thread_info *th = (win32_thread_info *) thread_target_data (thread);
+  windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
 
   remove_thread (thread);
   CloseHandle (th->h);
@@ -424,7 +424,7 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
 static void
 continue_one_thread (thread_info *thread, int thread_id)
 {
-  win32_thread_info *th = (win32_thread_info *) thread_target_data (thread);
+  windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
 
   if (thread_id == -1 || thread_id == th->tid)
     {
@@ -473,7 +473,7 @@ static void
 child_fetch_inferior_registers (struct regcache *regcache, int r)
 {
   int regno;
-  win32_thread_info *th = thread_rec (current_thread_ptid (), TRUE);
+  windows_thread_info *th = thread_rec (current_thread_ptid (), TRUE);
   if (r == -1 || r > NUM_REGS)
     child_fetch_inferior_registers (regcache, NUM_REGS);
   else
@@ -487,7 +487,7 @@ static void
 child_store_inferior_registers (struct regcache *regcache, int r)
 {
   int regno;
-  win32_thread_info *th = thread_rec (current_thread_ptid (), TRUE);
+  windows_thread_info *th = thread_rec (current_thread_ptid (), TRUE);
   if (r == -1 || r == 0 || r > NUM_REGS)
     child_store_inferior_registers (regcache, NUM_REGS);
   else
@@ -911,7 +911,7 @@ win32_process_target::resume (thread_resume *resume_info, size_t n)
   DWORD tid;
   enum gdb_signal sig;
   int step;
-  win32_thread_info *th;
+  windows_thread_info *th;
   DWORD continue_status = DBG_CONTINUE;
   ptid_t ptid;
 
@@ -1349,7 +1349,7 @@ handle_exception (struct target_waitstatus *ourstatus)
 static void
 suspend_one_thread (thread_info *thread)
 {
-  win32_thread_info *th = (win32_thread_info *) thread_target_data (thread);
+  windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
 
   if (!th->suspended)
     {
@@ -1835,7 +1835,7 @@ win32_process_target::supports_get_tib_address ()
 int
 win32_process_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
 {
-  win32_thread_info *th;
+  windows_thread_info *th;
   th = thread_rec (ptid, 0);
   if (th == NULL)
     return 0;
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index 9d2f0b4fbec..2bd94e85288 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -29,7 +29,7 @@ extern const struct target_desc *win32_tdesc;
 
 /* Thread information structure used to track extra information about
    each thread.  */
-typedef struct win32_thread_info
+struct windows_thread_info
 {
   /* The Win32 thread identifier.  */
   DWORD tid;
@@ -54,7 +54,7 @@ typedef struct win32_thread_info
   /* Whether debug registers changed since we last set CONTEXT back to
      the thread.  */
   int debug_registers_changed;
-} win32_thread_info;
+};
 
 struct win32_target_ops
 {
@@ -68,23 +68,23 @@ struct win32_target_ops
   void (*initial_stuff) (void);
 
   /* Fetch the context from the inferior.  */
-  void (*get_thread_context) (win32_thread_info *th);
+  void (*get_thread_context) (windows_thread_info *th);
 
   /* Called just before resuming the thread.  */
-  void (*prepare_to_resume) (win32_thread_info *th);
+  void (*prepare_to_resume) (windows_thread_info *th);
 
   /* Called when a thread was added.  */
-  void (*thread_added) (win32_thread_info *th);
+  void (*thread_added) (windows_thread_info *th);
 
   /* Fetch register from gdbserver regcache data.  */
   void (*fetch_inferior_register) (struct regcache *regcache,
-				   win32_thread_info *th, int r);
+				   windows_thread_info *th, int r);
 
   /* Store a new register value into the thread context of TH.  */
   void (*store_inferior_register) (struct regcache *regcache,
-				   win32_thread_info *th, int r);
+				   windows_thread_info *th, int r);
 
-  void (*single_step) (win32_thread_info *th);
+  void (*single_step) (windows_thread_info *th);
 
   const unsigned char *breakpoint;
   int breakpoint_len;
@@ -171,7 +171,7 @@ class win32_process_target : public process_stratum_target
 };
 
 /* Retrieve the context for this thread, if not already retrieved.  */
-extern void win32_require_context (win32_thread_info *th);
+extern void win32_require_context (windows_thread_info *th);
 
 /* Map the Windows error number in ERROR to a locale-dependent error
    message string and return a pointer to it.  Typically, the values
-- 
2.21.1

[PATCH v3 03/29] Rename windows_thread_info::id to "tid"

[Tom Tromey]

This changes the name of a field in windows_thread_info, bringing gdb
and gdbserver closer into sync.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (struct windows_thread_info) <tid>: Rename from "id".
	(thread_rec, windows_add_thread, windows_delete_thread)
	(windows_continue): Update.
---
 gdb/ChangeLog     |  6 ++++++
 gdb/windows-nat.c | 10 +++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 8b993912713..69930b00a11 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -248,7 +248,7 @@ static enum gdb_signal last_sig = GDB_SIGNAL_0;
    not available in gdb's thread structure.  */
 struct windows_thread_info
   {
-    DWORD id;
+    DWORD tid;
     HANDLE h;
     CORE_ADDR thread_local_base;
     char *name;
@@ -429,7 +429,7 @@ static windows_thread_info *
 thread_rec (DWORD id, int get_context)
 {
   for (windows_thread_info *th : thread_list)
-    if (th->id == id)
+    if (th->tid == id)
       {
 	if (!th->suspended && get_context)
 	  {
@@ -487,7 +487,7 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
     return th;
 
   th = XCNEW (windows_thread_info);
-  th->id = id;
+  th->tid = id;
   th->h = h;
   th->thread_local_base = (CORE_ADDR) (uintptr_t) tlb;
 #ifdef __x86_64__
@@ -594,7 +594,7 @@ windows_delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p)
   auto iter = std::find_if (thread_list.begin (), thread_list.end (),
 			    [=] (windows_thread_info *th)
 			    {
-			      return th->id == id;
+			      return th->tid == id;
 			    });
 
   if (iter != thread_list.end ())
@@ -1477,7 +1477,7 @@ windows_continue (DWORD continue_status, int id, int killed)
 		  "DBG_CONTINUE" : "DBG_EXCEPTION_NOT_HANDLED"));
 
   for (windows_thread_info *th : thread_list)
-    if ((id == -1 || id == (int) th->id)
+    if ((id == -1 || id == (int) th->tid)
 	&& th->suspended)
       {
 #ifdef __x86_64__
-- 
2.21.1

[PATCH v3 04/29] Share windows_thread_info between gdb and gdbserver

[Tom Tromey]

This introduces a new file, nat/windows-nat.h, which holds the
definition of windows_thread_info.  This is now shared between gdb and
gdbserver.

Note that the two implementations different slightly.  gdb had a
couple of fields ("name" and "reload_context") that gdbserver did not;
while gdbserver had one field ("base_context") that gdb did not, plus
better comments.  The new file preserves all the fields, and the
comments.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (struct windows_thread_info): Remove.
	* nat/windows-nat.h: New file.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.h (struct windows_thread_info): Remove.
---
 gdb/ChangeLog         |  5 ++++
 gdb/nat/windows-nat.h | 66 +++++++++++++++++++++++++++++++++++++++++++
 gdb/windows-nat.c     | 20 +------------
 gdbserver/ChangeLog   |  4 +++
 gdbserver/win32-low.h | 30 +-------------------
 5 files changed, 77 insertions(+), 48 deletions(-)
 create mode 100644 gdb/nat/windows-nat.h

diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
new file mode 100644
index 00000000000..71df097ed0b
--- /dev/null
+++ b/gdb/nat/windows-nat.h
@@ -0,0 +1,66 @@
+/* Internal interfaces for the Windows code
+   Copyright (C) 1995-2020 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/>.  */
+
+#ifndef NAT_WINDOWS_NAT_H
+#define NAT_WINDOWS_NAT_H
+
+#include <windows.h>
+
+/* Thread information structure used to track extra information about
+   each thread.  */
+struct windows_thread_info
+{
+  /* The Win32 thread identifier.  */
+  DWORD tid;
+
+  /* The handle to the thread.  */
+  HANDLE h;
+
+  /* Thread Information Block address.  */
+  CORE_ADDR thread_local_base;
+
+  /* Non zero if SuspendThread was called on this thread.  */
+  int suspended;
+
+#ifdef _WIN32_WCE
+  /* The context as retrieved right after suspending the thread. */
+  CONTEXT base_context;
+#endif
+
+  /* The context of the thread, including any manipulations.  */
+  union
+  {
+    CONTEXT context;
+#ifdef __x86_64__
+    WOW64_CONTEXT wow64_context;
+#endif
+  };
+
+  /* Whether debug registers changed since we last set CONTEXT back to
+     the thread.  */
+  int debug_registers_changed;
+
+  /* Nonzero if CONTEXT is invalidated and must be re-read from the
+     inferior thread.  */
+  int reload_context;
+
+  /* The name of the thread, allocated by xmalloc.  */
+  char *name;
+};
+
+#endif
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 69930b00a11..d2c4ecd6daf 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -70,6 +70,7 @@
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/pathstuff.h"
 #include "gdbsupport/gdb_wait.h"
+#include "nat/windows-nat.h"
 
 #define STATUS_WX86_BREAKPOINT 0x4000001F
 #define STATUS_WX86_SINGLE_STEP 0x4000001E
@@ -244,25 +245,6 @@ static unsigned long cygwin_get_dr7 (void);
 static enum gdb_signal last_sig = GDB_SIGNAL_0;
 /* Set if a signal was received from the debugged process.  */
 
-/* Thread information structure used to track information that is
-   not available in gdb's thread structure.  */
-struct windows_thread_info
-  {
-    DWORD tid;
-    HANDLE h;
-    CORE_ADDR thread_local_base;
-    char *name;
-    int suspended;
-    int reload_context;
-    union
-      {
-	CONTEXT context;
-#ifdef __x86_64__
-	WOW64_CONTEXT wow64_context;
-#endif
-      };
-  };
-
 static std::vector<windows_thread_info *> thread_list;
 
 /* The process and thread handles for the above context.  */
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index 2bd94e85288..28ac2b082d9 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -20,6 +20,7 @@
 #define GDBSERVER_WIN32_LOW_H
 
 #include <windows.h>
+#include "nat/windows-nat.h"
 
 struct target_desc;
 
@@ -27,35 +28,6 @@ struct target_desc;
    Windows ports support neither bi-arch nor multi-process.  */
 extern const struct target_desc *win32_tdesc;
 
-/* Thread information structure used to track extra information about
-   each thread.  */
-struct windows_thread_info
-{
-  /* The Win32 thread identifier.  */
-  DWORD tid;
-
-  /* The handle to the thread.  */
-  HANDLE h;
-
-  /* Thread Information Block address.  */
-  CORE_ADDR thread_local_base;
-
-  /* Non zero if SuspendThread was called on this thread.  */
-  int suspended;
-
-#ifdef _WIN32_WCE
-  /* The context as retrieved right after suspending the thread. */
-  CONTEXT base_context;
-#endif
-
-  /* The context of the thread, including any manipulations.  */
-  CONTEXT context;
-
-  /* Whether debug registers changed since we last set CONTEXT back to
-     the thread.  */
-  int debug_registers_changed;
-};
-
 struct win32_target_ops
 {
   /* Architecture-specific setup.  */
-- 
2.21.1

[PATCH v3 05/29] Use new and delete for windows_thread_info

[Tom Tromey]

This adds a constructor, destructor, and member initializers to
windows_thread_info, and changes gdb and gdbserver to use new and
delete.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (windows_add_thread): Use new.
	(windows_init_thread_list, windows_delete_thread): Use delete.
	(get_windows_debug_event): Update.
	* nat/windows-nat.h (struct windows_thread_info): Add constructor,
	destructor, and initializers.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (child_add_thread): Use new.
	(delete_thread_info): Use delete.
---
 gdb/ChangeLog          |  8 ++++++++
 gdb/nat/windows-nat.h  | 26 ++++++++++++++++++++------
 gdb/windows-nat.c      | 15 ++++++---------
 gdbserver/ChangeLog    |  5 +++++
 gdbserver/win32-low.cc |  7 ++-----
 5 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 71df097ed0b..a3da2686422 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -25,6 +25,20 @@
    each thread.  */
 struct windows_thread_info
 {
+  windows_thread_info (DWORD tid_, HANDLE h_, CORE_ADDR tlb)
+    : tid (tid_),
+      h (h_),
+      thread_local_base (tlb)
+  {
+  }
+
+  ~windows_thread_info ()
+  {
+    xfree (name);
+  }
+
+  DISABLE_COPY_AND_ASSIGN (windows_thread_info);
+
   /* The Win32 thread identifier.  */
   DWORD tid;
 
@@ -35,17 +49,17 @@ struct windows_thread_info
   CORE_ADDR thread_local_base;
 
   /* Non zero if SuspendThread was called on this thread.  */
-  int suspended;
+  int suspended = 0;
 
 #ifdef _WIN32_WCE
   /* The context as retrieved right after suspending the thread. */
-  CONTEXT base_context;
+  CONTEXT base_context {};
 #endif
 
   /* The context of the thread, including any manipulations.  */
   union
   {
-    CONTEXT context;
+    CONTEXT context {};
 #ifdef __x86_64__
     WOW64_CONTEXT wow64_context;
 #endif
@@ -53,14 +67,14 @@ struct windows_thread_info
 
   /* Whether debug registers changed since we last set CONTEXT back to
      the thread.  */
-  int debug_registers_changed;
+  int debug_registers_changed = 0;
 
   /* Nonzero if CONTEXT is invalidated and must be re-read from the
      inferior thread.  */
-  int reload_context;
+  int reload_context = 0;
 
   /* The name of the thread, allocated by xmalloc.  */
-  char *name;
+  char *name = nullptr;
 };
 
 #endif
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index d2c4ecd6daf..095a0da75dc 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -468,16 +468,14 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
   if ((th = thread_rec (id, FALSE)))
     return th;
 
-  th = XCNEW (windows_thread_info);
-  th->tid = id;
-  th->h = h;
-  th->thread_local_base = (CORE_ADDR) (uintptr_t) tlb;
+  CORE_ADDR base = (CORE_ADDR) (uintptr_t) tlb;
 #ifdef __x86_64__
   /* For WOW64 processes, this is actually the pointer to the 64bit TIB,
      and the 32bit TIB is exactly 2 pages after it.  */
   if (wow64_process)
-    th->thread_local_base += 0x2000;
+    base += 0x2000;
 #endif
+  th = new windows_thread_info (id, h, base);
   thread_list.push_back (th);
 
   /* Add this new thread to the list of threads.
@@ -536,7 +534,7 @@ windows_init_thread_list (void)
   init_thread_list ();
 
   for (windows_thread_info *here : thread_list)
-    xfree (here);
+    delete here;
 
   thread_list.clear ();
 }
@@ -581,8 +579,7 @@ windows_delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p)
 
   if (iter != thread_list.end ())
     {
-      xfree ((*iter)->name);
-      xfree (*iter);
+      delete *iter;
       thread_list.erase (iter);
     }
 }
@@ -1718,7 +1715,7 @@ windows_nat_target::get_windows_debug_event (int pid,
   BOOL debug_event;
   DWORD continue_status, event_code;
   windows_thread_info *th;
-  static windows_thread_info dummy_thread_info;
+  static windows_thread_info dummy_thread_info (0, 0, 0);
   DWORD thread_id = 0;
 
   last_sig = GDB_SIGNAL_0;
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 55e8322cebb..1284ed177cb 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -212,10 +212,7 @@ child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
   if ((th = thread_rec (ptid, FALSE)))
     return th;
 
-  th = XCNEW (windows_thread_info);
-  th->tid = tid;
-  th->h = h;
-  th->thread_local_base = (CORE_ADDR) (uintptr_t) tlb;
+  th = new windows_thread_info (tid, h, (CORE_ADDR) (uintptr_t) tlb);
 
   add_thread (ptid, th);
 
@@ -233,7 +230,7 @@ delete_thread_info (thread_info *thread)
 
   remove_thread (thread);
   CloseHandle (th->h);
-  free (th);
+  delete th;
 }
 
 /* Delete a thread from the list of threads.  */
-- 
2.21.1

[PATCH v3 06/29] Change two windows_thread_info members to "bool"

[Tom Tromey]

This changes a couple of fields of windows_thread_info to have type
"bool".  It also updates the comment of another field, to clarify the
possible values it can hold.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (thread_rec)
	(windows_nat_target::fetch_registers): Update.
	* nat/windows-nat.h (struct windows_thread_info) <suspended>:
	Update comment.
	<debug_registers_changed, reload_context>: Now bool.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-i386-low.c (update_debug_registers)
	(i386_prepare_to_resume, i386_thread_added): Update.
---
 gdb/ChangeLog               | 8 ++++++++
 gdb/nat/windows-nat.h       | 9 ++++++---
 gdb/windows-nat.c           | 4 ++--
 gdbserver/ChangeLog         | 5 +++++
 gdbserver/win32-i386-low.cc | 6 +++---
 5 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index a3da2686422..27fd7ed19da 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -48,7 +48,10 @@ struct windows_thread_info
   /* Thread Information Block address.  */
   CORE_ADDR thread_local_base;
 
-  /* Non zero if SuspendThread was called on this thread.  */
+  /* This keeps track of whether SuspendThread was called on this
+     thread.  -1 means there was a failure or that the thread was
+     explicitly not suspended, 1 means it was called, and 0 means it
+     was not.  */
   int suspended = 0;
 
 #ifdef _WIN32_WCE
@@ -67,11 +70,11 @@ struct windows_thread_info
 
   /* Whether debug registers changed since we last set CONTEXT back to
      the thread.  */
-  int debug_registers_changed = 0;
+  bool debug_registers_changed = false;
 
   /* Nonzero if CONTEXT is invalidated and must be re-read from the
      inferior thread.  */
-  int reload_context = 0;
+  bool reload_context = false;
 
   /* The name of the thread, allocated by xmalloc.  */
   char *name = nullptr;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 095a0da75dc..866afffa262 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -439,7 +439,7 @@ thread_rec (DWORD id, int get_context)
 	      }
 	    else if (get_context < 0)
 	      th->suspended = -1;
-	    th->reload_context = 1;
+	    th->reload_context = true;
 	  }
 	return th;
       }
@@ -695,7 +695,7 @@ windows_nat_target::fetch_registers (struct regcache *regcache, int r)
 	      dr[7] = th->context.Dr7;
 	    }
 	}
-      th->reload_context = 0;
+      th->reload_context = false;
     }
 
   if (r < 0)
diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
index 29ee49fcd03..1b78cf98b33 100644
--- a/gdbserver/win32-i386-low.cc
+++ b/gdbserver/win32-i386-low.cc
@@ -44,7 +44,7 @@ update_debug_registers (thread_info *thread)
 
   /* The actual update is done later just before resuming the lwp,
      we just mark that the registers need updating.  */
-  th->debug_registers_changed = 1;
+  th->debug_registers_changed = true;
 }
 
 /* Update the inferior's debug register REGNUM from STATE.  */
@@ -253,14 +253,14 @@ i386_prepare_to_resume (windows_thread_info *th)
 	 FIXME: should we set dr6 also ?? */
       th->context.Dr7 = dr->dr_control_mirror;
 
-      th->debug_registers_changed = 0;
+      th->debug_registers_changed = false;
     }
 }
 
 static void
 i386_thread_added (windows_thread_info *th)
 {
-  th->debug_registers_changed = 1;
+  th->debug_registers_changed = true;
 }
 
 static void
-- 
2.21.1

[PATCH v3 07/29] Make windows_thread_info::name a unique_xmalloc_ptr

[Tom Tromey]

This changes windows_thread_info::name to be a unique_xmalloc_ptr,
removing some manual memory management.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (handle_exception)
	(windows_nat_target::thread_name): Update.
	* nat/windows-nat.h (windows_thread_info): Remove destructor.
	<name>: Now unique_xmalloc_ptr.
---
 gdb/ChangeLog         | 7 +++++++
 gdb/nat/windows-nat.h | 7 +------
 gdb/windows-nat.c     | 5 ++---
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 27fd7ed19da..543de895e77 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -32,11 +32,6 @@ struct windows_thread_info
   {
   }
 
-  ~windows_thread_info ()
-  {
-    xfree (name);
-  }
-
   DISABLE_COPY_AND_ASSIGN (windows_thread_info);
 
   /* The Win32 thread identifier.  */
@@ -77,7 +72,7 @@ struct windows_thread_info
   bool reload_context = false;
 
   /* The name of the thread, allocated by xmalloc.  */
-  char *name = nullptr;
+  gdb::unique_xmalloc_ptr<char> name;
 };
 
 #endif
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 866afffa262..4fe5dfe7db0 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1414,8 +1414,7 @@ handle_exception (struct target_waitstatus *ourstatus)
 	      if (thread_name_len > 0)
 		{
 		  thread_name.get ()[thread_name_len - 1] = '\0';
-		  xfree (named_thread->name);
-		  named_thread->name = thread_name.release ();
+		  named_thread->name = std::move (thread_name);
 		}
 	    }
 	  ourstatus->value.sig = GDB_SIGNAL_TRAP;
@@ -3351,7 +3350,7 @@ windows_nat_target::get_ada_task_ptid (long lwp, long thread)
 const char *
 windows_nat_target::thread_name (struct thread_info *thr)
 {
-  return thread_rec (thr->ptid.tid (), 0)->name;
+  return thread_rec (thr->ptid.tid (), 0)->name.get ();
 }
 
 
-- 
2.21.1

[PATCH v3 08/29] Use lwp, not tid, for Windows thread id

[Tom Tromey]

This changes windows-nat.c to put the Windows thread id into the "lwp"
field of ptid_t, not the "tid" field.  This is done for two reasons.

First, ptid.h has this to say:

   process_stratum targets that handle threading themselves should
   prefer using the ptid.lwp field, leaving the ptid.tid field for any
   thread_stratum target that might want to sit on top.

Second, this change brings gdb and gdbserver into sync here, which
makes sharing code simpler.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (windows_add_thread, windows_delete_thread)
	(windows_nat_target::fetch_registers)
	(windows_nat_target::store_registers, fake_create_process)
	(windows_nat_target::resume, windows_nat_target::resume)
	(get_windows_debug_event, windows_nat_target::wait)
	(windows_nat_target::pid_to_str)
	(windows_nat_target::get_tib_address)
	(windows_nat_target::get_ada_task_ptid)
	(windows_nat_target::thread_name)
	(windows_nat_target::thread_alive): Use lwp, not tid.
---
 gdb/ChangeLog     | 13 ++++++++++++
 gdb/windows-nat.c | 54 +++++++++++++++++++++++------------------------
 2 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 4fe5dfe7db0..155682ba107 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -461,9 +461,9 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
   windows_thread_info *th;
   DWORD id;
 
-  gdb_assert (ptid.tid () != 0);
+  gdb_assert (ptid.lwp () != 0);
 
-  id = ptid.tid ();
+  id = ptid.lwp ();
 
   if ((th = thread_rec (id, FALSE)))
     return th;
@@ -551,9 +551,9 @@ windows_delete_thread (ptid_t ptid, DWORD exit_code, bool main_thread_p)
 {
   DWORD id;
 
-  gdb_assert (ptid.tid () != 0);
+  gdb_assert (ptid.lwp () != 0);
 
-  id = ptid.tid ();
+  id = ptid.lwp ();
 
   /* Emit a notification about the thread being deleted.
 
@@ -636,7 +636,7 @@ windows_fetch_one_register (struct regcache *regcache,
 void
 windows_nat_target::fetch_registers (struct regcache *regcache, int r)
 {
-  DWORD tid = regcache->ptid ().tid ();
+  DWORD tid = regcache->ptid ().lwp ();
   windows_thread_info *th = thread_rec (tid, TRUE);
 
   /* Check if TH exists.  Windows sometimes uses a non-existent
@@ -732,7 +732,7 @@ windows_store_one_register (const struct regcache *regcache,
 void
 windows_nat_target::store_registers (struct regcache *regcache, int r)
 {
-  DWORD tid = regcache->ptid ().tid ();
+  DWORD tid = regcache->ptid ().lwp ();
   windows_thread_info *th = thread_rec (tid, TRUE);
 
   /* Check if TH exists.  Windows sometimes uses a non-existent
@@ -1549,8 +1549,8 @@ fake_create_process (void)
       /*  We can not debug anything in that case.  */
     }
   current_thread
-    = windows_add_thread (ptid_t (current_event.dwProcessId, 0,
-				  current_event.dwThreadId),
+    = windows_add_thread (ptid_t (current_event.dwProcessId,
+				  current_event.dwThreadId, 0),
 			  current_event.u.CreateThread.hThread,
 			  current_event.u.CreateThread.lpThreadLocalBase,
 			  true /* main_thread_p */);
@@ -1607,10 +1607,10 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
   last_sig = GDB_SIGNAL_0;
 
   DEBUG_EXEC (("gdb: windows_resume (pid=%d, tid=0x%x, step=%d, sig=%d);\n",
-	       ptid.pid (), (unsigned) ptid.tid (), step, sig));
+	       ptid.pid (), (unsigned) ptid.lwp (), step, sig));
 
   /* Get context for currently selected thread.  */
-  th = thread_rec (inferior_ptid.tid (), FALSE);
+  th = thread_rec (inferior_ptid.lwp (), FALSE);
   if (th)
     {
 #ifdef __x86_64__
@@ -1675,7 +1675,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
   if (resume_all)
     windows_continue (continue_status, -1, 0);
   else
-    windows_continue (continue_status, ptid.tid (), 0);
+    windows_continue (continue_status, ptid.lwp (), 0);
 }
 
 /* Ctrl-C handler used when the inferior is not run in the same console.  The
@@ -1754,7 +1754,7 @@ windows_nat_target::get_windows_debug_event (int pid,
       /* Record the existence of this thread.  */
       thread_id = current_event.dwThreadId;
       th = windows_add_thread
-        (ptid_t (current_event.dwProcessId, 0, current_event.dwThreadId),
+        (ptid_t (current_event.dwProcessId, current_event.dwThreadId, 0),
 	 current_event.u.CreateThread.hThread,
 	 current_event.u.CreateThread.lpThreadLocalBase,
 	 false /* main_thread_p */);
@@ -1766,8 +1766,8 @@ windows_nat_target::get_windows_debug_event (int pid,
 		     (unsigned) current_event.dwProcessId,
 		     (unsigned) current_event.dwThreadId,
 		     "EXIT_THREAD_DEBUG_EVENT"));
-      windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
-				     current_event.dwThreadId),
+      windows_delete_thread (ptid_t (current_event.dwProcessId,
+				     current_event.dwThreadId, 0),
 			     current_event.u.ExitThread.dwExitCode,
 			     false /* main_thread_p */);
       th = &dummy_thread_info;
@@ -1785,8 +1785,8 @@ windows_nat_target::get_windows_debug_event (int pid,
       current_process_handle = current_event.u.CreateProcessInfo.hProcess;
       /* Add the main thread.  */
       th = windows_add_thread
-        (ptid_t (current_event.dwProcessId, 0,
-		 current_event.dwThreadId),
+        (ptid_t (current_event.dwProcessId,
+		 current_event.dwThreadId, 0),
 	 current_event.u.CreateProcessInfo.hThread,
 	 current_event.u.CreateProcessInfo.lpThreadLocalBase,
 	 true /* main_thread_p */);
@@ -1807,8 +1807,8 @@ windows_nat_target::get_windows_debug_event (int pid,
 	}
       else if (saw_create == 1)
 	{
-	  windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
-					 current_event.dwThreadId),
+	  windows_delete_thread (ptid_t (current_event.dwProcessId,
+					 current_event.dwThreadId, 0),
 				 0, true /* main_thread_p */);
 	  DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
 	  /* If the exit status looks like a fatal exception, but we
@@ -1907,7 +1907,7 @@ windows_nat_target::get_windows_debug_event (int pid,
     }
   else
     {
-      inferior_ptid = ptid_t (current_event.dwProcessId, 0, thread_id);
+      inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
       current_thread = th;
       if (!current_thread)
 	current_thread = thread_rec (thread_id, TRUE);
@@ -1965,7 +1965,7 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
       SetConsoleCtrlHandler (&ctrl_c_handler, FALSE);
 
       if (retval)
-	return ptid_t (current_event.dwProcessId, 0, retval);
+	return ptid_t (current_event.dwProcessId, retval, 0);
       else
 	{
 	  int detach = 0;
@@ -3194,8 +3194,8 @@ windows_nat_target::close ()
 std::string
 windows_nat_target::pid_to_str (ptid_t ptid)
 {
-  if (ptid.tid () != 0)
-    return string_printf ("Thread %d.0x%lx", ptid.pid (), ptid.tid ());
+  if (ptid.lwp () != 0)
+    return string_printf ("Thread %d.0x%lx", ptid.pid (), ptid.lwp ());
 
   return normal_pid_to_str (ptid);
 }
@@ -3329,7 +3329,7 @@ windows_nat_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
 {
   windows_thread_info *th;
 
-  th = thread_rec (ptid.tid (), 0);
+  th = thread_rec (ptid.lwp (), 0);
   if (th == NULL)
     return false;
 
@@ -3342,7 +3342,7 @@ windows_nat_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
 ptid_t
 windows_nat_target::get_ada_task_ptid (long lwp, long thread)
 {
-  return ptid_t (inferior_ptid.pid (), 0, lwp);
+  return ptid_t (inferior_ptid.pid (), lwp, 0);
 }
 
 /* Implementation of the to_thread_name method.  */
@@ -3350,7 +3350,7 @@ windows_nat_target::get_ada_task_ptid (long lwp, long thread)
 const char *
 windows_nat_target::thread_name (struct thread_info *thr)
 {
-  return thread_rec (thr->ptid.tid (), 0)->name.get ();
+  return thread_rec (thr->ptid.lwp (), 0)->name.get ();
 }
 
 
@@ -3511,8 +3511,8 @@ windows_nat_target::thread_alive (ptid_t ptid)
 {
   int tid;
 
-  gdb_assert (ptid.tid () != 0);
-  tid = ptid.tid ();
+  gdb_assert (ptid.lwp () != 0);
+  tid = ptid.lwp ();
 
   return WaitForSingleObject (thread_rec (tid, FALSE)->h, 0) != WAIT_OBJECT_0;
 }
-- 
2.21.1

[PATCH v3 09/29] Share Windows thread-suspend and -resume code

[Tom Tromey]

This adds "suspend" and "resume" methods to windows_thread_info, and
changes gdb and gdbserver to share this code.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (thread_rec): Use windows_thread_info::suspend.
	(windows_continue): Use windows_continue::resume.
	* nat/windows-nat.h (struct windows_thread_info) <suspend,
	resume>: Declare new methods.
	* nat/windows-nat.c: New file.
	* configure.nat (NATDEPFILES): Add nat/windows-nat.o when needed.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (win32_require_context, suspend_one_thread): Use
	windows_thread_info::suspend.
	(continue_one_thread): Use windows_thread_info::resume.
	* configure.srv (srv_tgtobj): Add windows-nat.o when needed.
---
 gdb/ChangeLog           |  9 +++++++
 gdb/configure.nat       |  4 +--
 gdb/nat/windows-nat.c   | 60 +++++++++++++++++++++++++++++++++++++++++
 gdb/nat/windows-nat.h   |  6 +++++
 gdb/windows-nat.c       | 26 ++----------------
 gdbserver/ChangeLog     |  7 +++++
 gdbserver/configure.srv |  7 ++++-
 gdbserver/win32-low.cc  | 33 +++--------------------
 8 files changed, 95 insertions(+), 57 deletions(-)
 create mode 100644 gdb/nat/windows-nat.c

diff --git a/gdb/configure.nat b/gdb/configure.nat
index 83ffdb80486..6ea25834954 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -75,10 +75,10 @@ case ${gdb_host} in
 	NATDEPFILES='fork-child.o nat/fork-inferior.o inf-ptrace.o'
 	;;
     cygwin*)
-	NATDEPFILES='x86-nat.o nat/x86-dregs.o windows-nat.o'
+	NATDEPFILES='x86-nat.o nat/x86-dregs.o windows-nat.o nat/windows-nat.o'
 	;;
     mingw*)
-	NATDEPFILES='x86-nat.o nat/x86-dregs.o windows-nat.o'
+	NATDEPFILES='x86-nat.o nat/x86-dregs.o windows-nat.o nat/windows-nat.o'
 	;;
     aix)
 	NATDEPFILES='nat/fork-inferior.o fork-child.o inf-ptrace.o'
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
new file mode 100644
index 00000000000..a98ff421e6f
--- /dev/null
+++ b/gdb/nat/windows-nat.c
@@ -0,0 +1,60 @@
+/* Internal interfaces for the Windows code
+   Copyright (C) 1995-2019 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/>.  */
+
+#include "gdbsupport/common-defs.h"
+#include "nat/windows-nat.h"
+
+void
+windows_thread_info::suspend ()
+{
+  if (suspended != 0)
+    return;
+
+  if (SuspendThread (h) == (DWORD) -1)
+    {
+      DWORD err = GetLastError ();
+
+      /* We get Access Denied (5) when trying to suspend
+	 threads that Windows started on behalf of the
+	 debuggee, usually when those threads are just
+	 about to exit.
+	 We can get Invalid Handle (6) if the main thread
+	 has exited.  */
+      if (err != ERROR_INVALID_HANDLE && err != ERROR_ACCESS_DENIED)
+	warning (_("SuspendThread (tid=0x%x) failed. (winerr %u)"),
+		 (unsigned) tid, (unsigned) err);
+      suspended = -1;
+    }
+  else
+    suspended = 1;
+}
+
+void
+windows_thread_info::resume ()
+{
+  if (suspended > 0)
+    {
+      if (ResumeThread (h) == (DWORD) -1)
+	{
+	  DWORD err = GetLastError ();
+	  warning (_("warning: ResumeThread (tid=0x%x) failed. (winerr %u)"),
+		   (unsigned) tid, (unsigned) err);
+	}
+    }
+  suspended = 0;
+}
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 543de895e77..695f801c58f 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -34,6 +34,12 @@ struct windows_thread_info
 
   DISABLE_COPY_AND_ASSIGN (windows_thread_info);
 
+  /* Ensure that this thread has been suspended.  */
+  void suspend ();
+
+  /* Resume the thread if it has been suspended.  */
+  void resume ();
+
   /* The Win32 thread identifier.  */
   DWORD tid;
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 155682ba107..ce83da0ff9f 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -416,27 +416,7 @@ thread_rec (DWORD id, int get_context)
 	if (!th->suspended && get_context)
 	  {
 	    if (get_context > 0 && id != current_event.dwThreadId)
-	      {
-		if (SuspendThread (th->h) == (DWORD) -1)
-		  {
-		    DWORD err = GetLastError ();
-
-		    /* We get Access Denied (5) when trying to suspend
-		       threads that Windows started on behalf of the
-		       debuggee, usually when those threads are just
-		       about to exit.
-		       We can get Invalid Handle (6) if the main thread
-		       has exited.  */
-		    if (err != ERROR_INVALID_HANDLE
-			&& err != ERROR_ACCESS_DENIED)
-		      warning (_("SuspendThread (tid=0x%x) failed."
-				 " (winerr %u)"),
-			       (unsigned) id, (unsigned) err);
-		    th->suspended = -1;
-		  }
-		else
-		  th->suspended = 1;
-	      }
+	      th->suspend ();
 	    else if (get_context < 0)
 	      th->suspended = -1;
 	    th->reload_context = true;
@@ -1515,9 +1495,7 @@ windows_continue (DWORD continue_status, int id, int killed)
 		th->context.ContextFlags = 0;
 	      }
 	  }
-	if (th->suspended > 0)
-	  (void) ResumeThread (th->h);
-	th->suspended = 0;
+	th->resume ();
       }
 
   res = ContinueDebugEvent (current_event.dwProcessId,
diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv
index ecdd63a310a..7acf229fbef 100644
--- a/gdbserver/configure.srv
+++ b/gdbserver/configure.srv
@@ -74,7 +74,7 @@ case "${gdbserver_host}" in
 			srv_linux_thread_db=yes
 			;;
   arm*-*-mingw32ce*)	srv_regobj=reg-arm.o
-			srv_tgtobj="win32-low.o win32-arm-low.o"
+			srv_tgtobj="win32-low.o windows-nat.o win32-arm-low.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
 			# hostio_last_error implementation is in win32-low.c
 			srv_hostio_err_objs=""
@@ -99,6 +99,7 @@ case "${gdbserver_host}" in
   i[34567]86-*-cygwin*)	srv_regobj=""
 			srv_tgtobj="x86-low.o nat/x86-dregs.o win32-low.o"
 			srv_tgtobj="${srv_tgtobj} win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			;;
   i[34567]86-*-linux*)	srv_tgtobj="${srv_tgtobj} arch/i386.o"
@@ -126,6 +127,7 @@ case "${gdbserver_host}" in
 			srv_regobj=""
 			srv_tgtobj="x86-low.o nat/x86-dregs.o win32-low.o"
 			srv_tgtobj="${srv_tgtobj} win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
 			# hostio_last_error implementation is in win32-low.c
@@ -136,6 +138,7 @@ case "${gdbserver_host}" in
   i[34567]86-*-mingw*)	srv_regobj=""
 			srv_tgtobj="x86-low.o nat/x86-dregs.o win32-low.o"
 			srv_tgtobj="${srv_tgtobj} win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			srv_mingw=yes
 			;;
@@ -393,12 +396,14 @@ case "${gdbserver_host}" in
   x86_64-*-mingw*)	srv_regobj=""
 			srv_tgtobj="x86-low.o nat/x86-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
 			srv_mingw=yes
 			;;
   x86_64-*-cygwin*)	srv_regobj=""
 			srv_tgtobj="x86-low.o nat/x86-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} win32-low.o win32-i386-low.o"
+			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
 			;;
 
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 1284ed177cb..7cad640878f 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -171,18 +171,7 @@ win32_require_context (windows_thread_info *th)
 {
   if (th->context.ContextFlags == 0)
     {
-      if (!th->suspended)
-	{
-	  if (SuspendThread (th->h) == (DWORD) -1)
-	    {
-	      DWORD err = GetLastError ();
-	      OUTMSG (("warning: SuspendThread failed in thread_rec, "
-		       "(error %d): %s\n", (int) err, strwinerror (err)));
-	    }
-	  else
-	    th->suspended = 1;
-	}
-
+      th->suspend ();
       win32_get_thread_context (th);
     }
 }
@@ -435,13 +424,7 @@ continue_one_thread (thread_info *thread, int thread_id)
 	      th->context.ContextFlags = 0;
 	    }
 
-	  if (ResumeThread (th->h) == (DWORD) -1)
-	    {
-	      DWORD err = GetLastError ();
-	      OUTMSG (("warning: ResumeThread failed in continue_one_thread, "
-		       "(error %d): %s\n", (int) err, strwinerror (err)));
-	    }
-	  th->suspended = 0;
+	  th->resume ();
 	}
     }
 }
@@ -1348,17 +1331,7 @@ suspend_one_thread (thread_info *thread)
 {
   windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
 
-  if (!th->suspended)
-    {
-      if (SuspendThread (th->h) == (DWORD) -1)
-	{
-	  DWORD err = GetLastError ();
-	  OUTMSG (("warning: SuspendThread failed in suspend_one_thread, "
-		   "(error %d): %s\n", (int) err, strwinerror (err)));
-	}
-      else
-	th->suspended = 1;
-    }
+  th->suspend ();
 }
 
 static void
-- 
2.21.1

[PATCH v3 10/29] Change type of argument to windows-nat.c:thread_rec

[Tom Tromey]

windows-nat.c:thread_rec accepts an integer parameter whose
interpretation depends on whether it is less than, equal to, or
greater than zero.  I found this confusing at times, so this patch
replaces it with an enum instead.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (enum thread_disposition_type): New.
	(thread_rec): Replace "get_context" parameter with "disposition";
	change type.
	(windows_add_thread, windows_nat_target::fetch_registers)
	(windows_nat_target::store_registers, handle_exception)
	(windows_nat_target::resume, get_windows_debug_event)
	(windows_nat_target::get_tib_address)
	(windows_nat_target::thread_name)
	(windows_nat_target::thread_alive): Update.
---
 gdb/ChangeLog     | 12 +++++++++
 gdb/windows-nat.c | 63 ++++++++++++++++++++++++++++++++---------------
 2 files changed, 55 insertions(+), 20 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index ce83da0ff9f..cdc24ec7ebd 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -404,22 +404,44 @@ check (BOOL ok, const char *file, int line)
 		     (unsigned) GetLastError ());
 }
 
-/* Find a thread record given a thread id.  If GET_CONTEXT is not 0,
-   then also retrieve the context for this thread.  If GET_CONTEXT is
-   negative, then don't suspend the thread.  */
+/* Possible values to pass to 'thread_rec'.  */
+enum thread_disposition_type
+{
+  /* Do not invalidate the thread's context, and do not suspend the
+     thread.  */
+  DONT_INVALIDATE_CONTEXT,
+  /* Invalidate the context, but do not suspend the thread.  */
+  DONT_SUSPEND,
+  /* Invalidate the context and suspend the thread.  */
+  INVALIDATE_CONTEXT
+};
+
+/* Find a thread record given a thread id.  THREAD_DISPOSITION
+   controls whether the thread is suspended, and whether the context
+   is invalidated.  */
 static windows_thread_info *
-thread_rec (DWORD id, int get_context)
+thread_rec (DWORD id, enum thread_disposition_type disposition)
 {
   for (windows_thread_info *th : thread_list)
     if (th->tid == id)
       {
-	if (!th->suspended && get_context)
+	if (!th->suspended)
 	  {
-	    if (get_context > 0 && id != current_event.dwThreadId)
-	      th->suspend ();
-	    else if (get_context < 0)
-	      th->suspended = -1;
-	    th->reload_context = true;
+	    switch (disposition)
+	      {
+	      case DONT_INVALIDATE_CONTEXT:
+		/* Nothing.  */
+		break;
+	      case INVALIDATE_CONTEXT:
+		if (id != current_event.dwThreadId)
+		  th->suspend ();
+		th->reload_context = true;
+		break;
+	      case DONT_SUSPEND:
+		th->reload_context = true;
+		th->suspended = -1;
+		break;
+	      }
 	  }
 	return th;
       }
@@ -445,7 +467,7 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
 
   id = ptid.lwp ();
 
-  if ((th = thread_rec (id, FALSE)))
+  if ((th = thread_rec (id, DONT_INVALIDATE_CONTEXT)))
     return th;
 
   CORE_ADDR base = (CORE_ADDR) (uintptr_t) tlb;
@@ -617,7 +639,7 @@ void
 windows_nat_target::fetch_registers (struct regcache *regcache, int r)
 {
   DWORD tid = regcache->ptid ().lwp ();
-  windows_thread_info *th = thread_rec (tid, TRUE);
+  windows_thread_info *th = thread_rec (tid, INVALIDATE_CONTEXT);
 
   /* Check if TH exists.  Windows sometimes uses a non-existent
      thread id in its events.  */
@@ -713,7 +735,7 @@ void
 windows_nat_target::store_registers (struct regcache *regcache, int r)
 {
   DWORD tid = regcache->ptid ().lwp ();
-  windows_thread_info *th = thread_rec (tid, TRUE);
+  windows_thread_info *th = thread_rec (tid, INVALIDATE_CONTEXT);
 
   /* Check if TH exists.  Windows sometimes uses a non-existent
      thread id in its events.  */
@@ -1253,7 +1275,7 @@ handle_exception (struct target_waitstatus *ourstatus)
   ourstatus->kind = TARGET_WAITKIND_STOPPED;
 
   /* Record the context of the current thread.  */
-  thread_rec (current_event.dwThreadId, -1);
+  thread_rec (current_event.dwThreadId, DONT_SUSPEND);
 
   switch (code)
     {
@@ -1383,7 +1405,7 @@ handle_exception (struct target_waitstatus *ourstatus)
 	  if (named_thread_id == (DWORD) -1)
 	    named_thread_id = current_event.dwThreadId;
 
-	  named_thread = thread_rec (named_thread_id, 0);
+	  named_thread = thread_rec (named_thread_id, DONT_INVALIDATE_CONTEXT);
 	  if (named_thread != NULL)
 	    {
 	      int thread_name_len;
@@ -1588,7 +1610,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
 	       ptid.pid (), (unsigned) ptid.lwp (), step, sig));
 
   /* Get context for currently selected thread.  */
-  th = thread_rec (inferior_ptid.lwp (), FALSE);
+  th = thread_rec (inferior_ptid.lwp (), DONT_INVALIDATE_CONTEXT);
   if (th)
     {
 #ifdef __x86_64__
@@ -1888,7 +1910,7 @@ windows_nat_target::get_windows_debug_event (int pid,
       inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
       current_thread = th;
       if (!current_thread)
-	current_thread = thread_rec (thread_id, TRUE);
+	current_thread = thread_rec (thread_id, INVALIDATE_CONTEXT);
     }
 
 out:
@@ -3307,7 +3329,7 @@ windows_nat_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
 {
   windows_thread_info *th;
 
-  th = thread_rec (ptid.lwp (), 0);
+  th = thread_rec (ptid.lwp (), DONT_INVALIDATE_CONTEXT);
   if (th == NULL)
     return false;
 
@@ -3328,7 +3350,7 @@ windows_nat_target::get_ada_task_ptid (long lwp, long thread)
 const char *
 windows_nat_target::thread_name (struct thread_info *thr)
 {
-  return thread_rec (thr->ptid.lwp (), 0)->name.get ();
+  return thread_rec (thr->ptid.lwp (), DONT_INVALIDATE_CONTEXT)->name.get ();
 }
 
 
@@ -3492,7 +3514,8 @@ windows_nat_target::thread_alive (ptid_t ptid)
   gdb_assert (ptid.lwp () != 0);
   tid = ptid.lwp ();
 
-  return WaitForSingleObject (thread_rec (tid, FALSE)->h, 0) != WAIT_OBJECT_0;
+  return (WaitForSingleObject (thread_rec (tid, DONT_INVALIDATE_CONTEXT)->h, 0)
+	  != WAIT_OBJECT_0);
 }
 
 void _initialize_check_for_gdb_ini ();
-- 
2.21.1

[PATCH v3 11/29] Handle pending stops from the Windows kernel

[Tom Tromey]

PR gdb/22992 concerns an assertion failure in gdb when debugging a
certain inferior:

    int finish_step_over(execution_control_state*): Assertion `ecs->event_thread->control.trap_expected' failed.

Initially the investigation centered on the discovery that gdb was not
suspending other threads when attempting to single-step.  This
oversight is corrected in this patch: now, when stepping a thread, gdb
will call SuspendThread on all other threads.

However, the bug persisted even after this change.  In particular,
WaitForDebugEvent could see a stop for a thread that was ostensibly
suspended.  Our theory of what is happening here is that there are
actually simultaneous breakpoint hits, and the Windows kernel queues
the events, causing the second stop to be reported on a suspended
thread.

In Windows 10 or later gdb could use the DBG_REPLY_LATER flag to
ContinueDebugEvent to request that such events be re-reported later.
However, relying on that did not seem advisable, so this patch instead
arranges to queue such "pending" stops, and then to report them later,
once the step has completed.

In the PR, Pedro pointed out that it's best in this scenario to
implement the stopped_by_sw_breakpoint method, so this patch does this
as well.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	PR gdb/22992
	* windows-nat.c (current_event): Update comment.
	(last_wait_event, desired_stop_thread_id): New globals.
	(struct pending_stop): New.
	(pending_stops): New global.
	(windows_nat_target) <stopped_by_sw_breakpoint>
	<supports_stopped_by_sw_breakpoint>: New methods.
	(windows_fetch_one_register): Add assertions.  Adjust PC.
	(windows_continue): Handle pending stops.  Suspend other threads
	when stepping.  Use last_wait_event
	(wait_for_debug_event): New function.
	(get_windows_debug_event): Use wait_for_debug_event.  Handle
	pending stops.  Queue spurious stops.
	(windows_nat_target::wait): Set stopped_at_software_breakpoint.
	(windows_nat_target::kill): Use wait_for_debug_event.
	* nat/windows-nat.h (struct windows_thread_info)
	<stopped_at_software_breakpoint>: New field.
	* nat/windows-nat.c (windows_thread_info::resume): Clear
	stopped_at_software_breakpoint.
---
 gdb/ChangeLog         |  22 +++++
 gdb/nat/windows-nat.c |   2 +
 gdb/nat/windows-nat.h |   4 +
 gdb/windows-nat.c     | 200 +++++++++++++++++++++++++++++++++++++++---
 4 files changed, 215 insertions(+), 13 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index a98ff421e6f..767ed8c192f 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -49,6 +49,8 @@ windows_thread_info::resume ()
 {
   if (suspended > 0)
     {
+      stopped_at_software_breakpoint = false;
+
       if (ResumeThread (h) == (DWORD) -1)
 	{
 	  DWORD err = GetLastError ();
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 695f801c58f..224ae5c536c 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -77,6 +77,10 @@ struct windows_thread_info
      inferior thread.  */
   bool reload_context = false;
 
+  /* True if this thread is currently stopped at a software
+     breakpoint.  This is used to offset the PC when needed.  */
+  bool stopped_at_software_breakpoint = false;
+
   /* The name of the thread, allocated by xmalloc.  */
   gdb::unique_xmalloc_ptr<char> name;
 };
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index cdc24ec7ebd..e48fa57f74a 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -249,8 +249,17 @@ static std::vector<windows_thread_info *> thread_list;
 
 /* The process and thread handles for the above context.  */
 
-static DEBUG_EVENT current_event;	/* The current debug event from
-					   WaitForDebugEvent */
+/* The current debug event from WaitForDebugEvent or from a pending
+   stop.  */
+static DEBUG_EVENT current_event;
+
+/* The most recent event from WaitForDebugEvent.  Unlike
+   current_event, this is guaranteed never to come from a pending
+   stop.  This is important because only data from the most recent
+   event from WaitForDebugEvent can be used when calling
+   ContinueDebugEvent.  */
+static DEBUG_EVENT last_wait_event;
+
 static HANDLE current_process_handle;	/* Currently executing process */
 static windows_thread_info *current_thread;	/* Info on currently selected thread */
 static EXCEPTION_RECORD siginfo_er;	/* Contents of $_siginfo */
@@ -325,6 +334,37 @@ static const struct xlate_exception xlate[] =
 
 #endif /* 0 */
 
+/* The ID of the thread for which we anticipate a stop event.
+   Normally this is -1, meaning we'll accept an event in any
+   thread.  */
+static DWORD desired_stop_thread_id = -1;
+
+/* A single pending stop.  See "pending_stops" for more
+   information.  */
+struct pending_stop
+{
+  /* The thread id.  */
+  DWORD thread_id;
+
+  /* The target waitstatus we computed.  */
+  target_waitstatus status;
+
+  /* The event.  A few fields of this can be referenced after a stop,
+     and it seemed simplest to store the entire event.  */
+  DEBUG_EVENT event;
+};
+
+/* A vector of pending stops.  Sometimes, Windows will report a stop
+   on a thread that has been ostensibly suspended.  We believe what
+   happens here is that two threads hit a breakpoint simultaneously,
+   and the Windows kernel queues the stop events.  However, this can
+   result in the strange effect of trying to single step thread A --
+   leaving all other threads suspended -- and then seeing a stop in
+   thread B.  To handle this scenario, we queue all such "pending"
+   stops here, and then process them once the step has completed.  See
+   PR gdb/22992.  */
+static std::vector<pending_stop> pending_stops;
+
 struct windows_nat_target final : public x86_nat_target<inf_child_target>
 {
   void close () override;
@@ -343,6 +383,16 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
   void fetch_registers (struct regcache *, int) override;
   void store_registers (struct regcache *, int) override;
 
+  bool stopped_by_sw_breakpoint () override
+  {
+    return current_thread->stopped_at_software_breakpoint;
+  }
+
+  bool supports_stopped_by_sw_breakpoint () override
+  {
+    return true;
+  }
+
   enum target_xfer_status xfer_partial (enum target_object object,
 					const char *annex,
 					gdb_byte *readbuf,
@@ -613,6 +663,10 @@ windows_fetch_one_register (struct regcache *regcache,
   struct gdbarch *gdbarch = regcache->arch ();
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
+  gdb_assert (!gdbarch_read_pc_p (gdbarch));
+  gdb_assert (gdbarch_pc_regnum (gdbarch) >= 0);
+  gdb_assert (!gdbarch_write_pc_p (gdbarch));
+
   if (r == I387_FISEG_REGNUM (tdep))
     {
       long l = *((long *) context_offset) & 0xffff;
@@ -632,7 +686,29 @@ windows_fetch_one_register (struct regcache *regcache,
       regcache->raw_supply (r, (char *) &l);
     }
   else
-    regcache->raw_supply (r, context_offset);
+    {
+      if (th->stopped_at_software_breakpoint
+	  && r == gdbarch_pc_regnum (gdbarch))
+	{
+	  int size = register_size (gdbarch, r);
+	  if (size == 4)
+	    {
+	      uint32_t value;
+	      memcpy (&value, context_offset, size);
+	      value -= gdbarch_decr_pc_after_break (gdbarch);
+	      memcpy (context_offset, &value, size);
+	    }
+	  else
+	    {
+	      gdb_assert (size == 8);
+	      uint64_t value;
+	      memcpy (&value, context_offset, size);
+	      value -= gdbarch_decr_pc_after_break (gdbarch);
+	      memcpy (context_offset, &value, size);
+	    }
+	}
+      regcache->raw_supply (r, context_offset);
+    }
 }
 
 void
@@ -1450,16 +1526,36 @@ windows_continue (DWORD continue_status, int id, int killed)
 {
   BOOL res;
 
+  desired_stop_thread_id = id;
+
+  /* If there are pending stops, and we might plausibly hit one of
+     them, we don't want to actually continue the inferior -- we just
+     want to report the stop.  In this case, we just pretend to
+     continue.  See the comment by the definition of "pending_stops"
+     for details on why this is needed.  */
+  for (const auto &item : pending_stops)
+    {
+      if (desired_stop_thread_id == -1
+	  || desired_stop_thread_id == item.thread_id)
+	{
+	  DEBUG_EVENTS (("windows_continue - pending stop anticipated, "
+			 "desired=0x%x, item=0x%x\n",
+			 desired_stop_thread_id, item.thread_id));
+	  return TRUE;
+	}
+    }
+
   DEBUG_EVENTS (("ContinueDebugEvent (cpid=%d, ctid=0x%x, %s);\n",
-		  (unsigned) current_event.dwProcessId,
-		  (unsigned) current_event.dwThreadId,
+		  (unsigned) last_wait_event.dwProcessId,
+		  (unsigned) last_wait_event.dwThreadId,
 		  continue_status == DBG_CONTINUE ?
 		  "DBG_CONTINUE" : "DBG_EXCEPTION_NOT_HANDLED"));
 
   for (windows_thread_info *th : thread_list)
-    if ((id == -1 || id == (int) th->tid)
-	&& th->suspended)
+    if (id == -1 || id == (int) th->tid)
       {
+	if (!th->suspended)
+	  continue;
 #ifdef __x86_64__
 	if (wow64_process)
 	  {
@@ -1519,9 +1615,15 @@ windows_continue (DWORD continue_status, int id, int killed)
 	  }
 	th->resume ();
       }
+    else
+      {
+	/* When single-stepping a specific thread, other threads must
+	   be suspended.  */
+	th->suspend ();
+      }
 
-  res = ContinueDebugEvent (current_event.dwProcessId,
-			    current_event.dwThreadId,
+  res = ContinueDebugEvent (last_wait_event.dwProcessId,
+			    last_wait_event.dwThreadId,
 			    continue_status);
 
   if (!res)
@@ -1704,6 +1806,17 @@ ctrl_c_handler (DWORD event_type)
   return TRUE;
 }
 
+/* A wrapper for WaitForDebugEvent that sets "last_wait_event"
+   appropriately.  */
+static BOOL
+wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout)
+{
+  BOOL result = WaitForDebugEvent (event, timeout);
+  if (result)
+    last_wait_event = *event;
+  return result;
+}
+
 /* Get the next event from the child.  Returns a non-zero thread id if the event
    requires handling by WFI (or whatever).  */
 
@@ -1717,9 +1830,36 @@ windows_nat_target::get_windows_debug_event (int pid,
   static windows_thread_info dummy_thread_info (0, 0, 0);
   DWORD thread_id = 0;
 
+  /* If there is a relevant pending stop, report it now.  See the
+     comment by the definition of "pending_stops" for details on why
+     this is needed.  */
+  for (auto iter = pending_stops.begin ();
+       iter != pending_stops.end ();
+       ++iter)
+    {
+      if (desired_stop_thread_id == -1
+	  || desired_stop_thread_id == iter->thread_id)
+	{
+	  thread_id = iter->thread_id;
+	  *ourstatus = iter->status;
+	  current_event = iter->event;
+
+	  inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
+	  current_thread = thread_rec (thread_id, INVALIDATE_CONTEXT);
+	  current_thread->reload_context = 1;
+
+	  DEBUG_EVENTS (("get_windows_debug_event - "
+			 "pending stop found in 0x%x (desired=0x%x)\n",
+			 thread_id, desired_stop_thread_id));
+
+	  pending_stops.erase (iter);
+	  return thread_id;
+	}
+    }
+
   last_sig = GDB_SIGNAL_0;
 
-  if (!(debug_event = WaitForDebugEvent (&current_event, 1000)))
+  if (!(debug_event = wait_for_debug_event (&current_event, 1000)))
     goto out;
 
   event_count++;
@@ -1903,7 +2043,27 @@ windows_nat_target::get_windows_debug_event (int pid,
 
   if (!thread_id || saw_create != 1)
     {
-      CHECK (windows_continue (continue_status, -1, 0));
+      CHECK (windows_continue (continue_status, desired_stop_thread_id, 0));
+    }
+  else if (desired_stop_thread_id != -1 && desired_stop_thread_id != thread_id)
+    {
+      /* Pending stop.  See the comment by the definition of
+	 "pending_stops" for details on why this is needed.  */
+      DEBUG_EVENTS (("get_windows_debug_event - "
+		     "unexpected stop in 0x%x (expecting 0x%x)\n",
+		     thread_id, desired_stop_thread_id));
+
+      if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
+	  && (current_event.u.Exception.ExceptionRecord.ExceptionCode
+	      == EXCEPTION_BREAKPOINT)
+	  && windows_initialization_done)
+	{
+	  th = thread_rec (thread_id, INVALIDATE_CONTEXT);
+	  th->stopped_at_software_breakpoint = true;
+	}
+      pending_stops.push_back ({thread_id, *ourstatus, current_event});
+      thread_id = 0;
+      CHECK (windows_continue (continue_status, desired_stop_thread_id, 0));
     }
   else
     {
@@ -1965,7 +2125,21 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
       SetConsoleCtrlHandler (&ctrl_c_handler, FALSE);
 
       if (retval)
-	return ptid_t (current_event.dwProcessId, retval, 0);
+	{
+	  ptid_t result = ptid_t (current_event.dwProcessId, retval, 0);
+
+	  if (current_thread != nullptr)
+	    {
+	      current_thread->stopped_at_software_breakpoint = false;
+	      if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
+		  && (current_event.u.Exception.ExceptionRecord.ExceptionCode
+		      == EXCEPTION_BREAKPOINT)
+		  && windows_initialization_done)
+		current_thread->stopped_at_software_breakpoint = true;
+	    }
+
+	  return result;
+	}
       else
 	{
 	  int detach = 0;
@@ -3174,7 +3348,7 @@ windows_nat_target::kill ()
     {
       if (!windows_continue (DBG_CONTINUE, -1, 1))
 	break;
-      if (!WaitForDebugEvent (&current_event, INFINITE))
+      if (!wait_for_debug_event (&current_event, INFINITE))
 	break;
       if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT)
 	break;
-- 
2.21.1

[PATCH v3 12/29] Call CloseHandle from ~windows_thread_info

[Tom Tromey]

Add a destructor to windows_thread_info that calls CloseHandle.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* nat/windows-nat.h (struct windows_thread_info): Declare
	destructor.
	* nat/windows-nat.c (~windows_thread_info): New.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (delete_thread_info): Don't call CloseHandle.
---
 gdb/ChangeLog          | 6 ++++++
 gdb/nat/windows-nat.c  | 5 +++++
 gdb/nat/windows-nat.h  | 2 ++
 gdbserver/ChangeLog    | 4 ++++
 gdbserver/win32-low.cc | 1 -
 5 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 767ed8c192f..ca3e3087944 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -19,6 +19,11 @@
 #include "gdbsupport/common-defs.h"
 #include "nat/windows-nat.h"
 
+windows_thread_info::~windows_thread_info ()
+{
+  CloseHandle (h);
+}
+
 void
 windows_thread_info::suspend ()
 {
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 224ae5c536c..ccdf207140e 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -32,6 +32,8 @@ struct windows_thread_info
   {
   }
 
+  ~windows_thread_info ();
+
   DISABLE_COPY_AND_ASSIGN (windows_thread_info);
 
   /* Ensure that this thread has been suspended.  */
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 7cad640878f..c642d47764c 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -218,7 +218,6 @@ delete_thread_info (thread_info *thread)
   windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
 
   remove_thread (thread);
-  CloseHandle (th->h);
   delete th;
 }
 
-- 
2.21.1

[PATCH v3 13/29] Wrap shared windows-nat code in windows_nat namespace

[Tom Tromey]

This wraps the shared windows-nat code in a windows_nat namespace.
This helps avoid name clashes.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c: Add "using namespace".
	* nat/windows-nat.h: Wrap contents in windows_nat namespace.
	* nat/windows-nat.c: Wrap contents in windows_nat namespace.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.h (struct win32_target_ops): Use qualified names where
	needed.
	* win32-i386-low.c: Add "using namespace".
	* win32-low.c: Add "using namespace".
	* win32-arm-low.c: Add "using namespace".
---
 gdb/ChangeLog               |  6 ++++++
 gdb/nat/windows-nat.c       |  5 +++++
 gdb/nat/windows-nat.h       |  5 +++++
 gdb/windows-nat.c           |  2 ++
 gdbserver/ChangeLog         |  8 ++++++++
 gdbserver/win32-arm-low.cc  |  2 ++
 gdbserver/win32-i386-low.cc |  2 ++
 gdbserver/win32-low.cc      |  2 ++
 gdbserver/win32-low.h       | 16 +++++++++-------
 9 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index ca3e3087944..6c0218cd9d8 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -19,6 +19,9 @@
 #include "gdbsupport/common-defs.h"
 #include "nat/windows-nat.h"
 
+namespace windows_nat
+{
+
 windows_thread_info::~windows_thread_info ()
 {
   CloseHandle (h);
@@ -65,3 +68,5 @@ windows_thread_info::resume ()
     }
   suspended = 0;
 }
+
+}
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index ccdf207140e..df283646753 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -21,6 +21,9 @@
 
 #include <windows.h>
 
+namespace windows_nat
+{
+
 /* Thread information structure used to track extra information about
    each thread.  */
 struct windows_thread_info
@@ -87,4 +90,6 @@ struct windows_thread_info
   gdb::unique_xmalloc_ptr<char> name;
 };
 
+}
+
 #endif
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index e48fa57f74a..b05a27c7cac 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -75,6 +75,8 @@
 #define STATUS_WX86_BREAKPOINT 0x4000001F
 #define STATUS_WX86_SINGLE_STEP 0x4000001E
 
+using namespace windows_nat;
+
 #define AdjustTokenPrivileges		dyn_AdjustTokenPrivileges
 #define DebugActiveProcessStop		dyn_DebugActiveProcessStop
 #define DebugBreakProcess		dyn_DebugBreakProcess
diff --git a/gdbserver/win32-arm-low.cc b/gdbserver/win32-arm-low.cc
index c50c5dfdbf2..78b7fd09ec3 100644
--- a/gdbserver/win32-arm-low.cc
+++ b/gdbserver/win32-arm-low.cc
@@ -18,6 +18,8 @@
 #include "server.h"
 #include "win32-low.h"
 
+using namespace windows_nat;
+
 #ifndef CONTEXT_FLOATING_POINT
 #define CONTEXT_FLOATING_POINT 0
 #endif
diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
index 1b78cf98b33..1c14bc70362 100644
--- a/gdbserver/win32-i386-low.cc
+++ b/gdbserver/win32-i386-low.cc
@@ -26,6 +26,8 @@
 #include "tdesc.h"
 #include "x86-tdesc.h"
 
+using namespace windows_nat;
+
 #ifndef CONTEXT_EXTENDED_REGISTERS
 #define CONTEXT_EXTENDED_REGISTERS 0
 #endif
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index c642d47764c..8f8b6cedd28 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -36,6 +36,8 @@
 #include "gdbsupport/common-inferior.h"
 #include "gdbsupport/gdb_wait.h"
 
+using namespace windows_nat;
+
 #ifndef USE_WIN32API
 #include <sys/cygwin.h>
 #endif
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index 28ac2b082d9..917f7275622 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -40,23 +40,25 @@ struct win32_target_ops
   void (*initial_stuff) (void);
 
   /* Fetch the context from the inferior.  */
-  void (*get_thread_context) (windows_thread_info *th);
+  void (*get_thread_context) (windows_nat::windows_thread_info *th);
 
   /* Called just before resuming the thread.  */
-  void (*prepare_to_resume) (windows_thread_info *th);
+  void (*prepare_to_resume) (windows_nat::windows_thread_info *th);
 
   /* Called when a thread was added.  */
-  void (*thread_added) (windows_thread_info *th);
+  void (*thread_added) (windows_nat::windows_thread_info *th);
 
   /* Fetch register from gdbserver regcache data.  */
   void (*fetch_inferior_register) (struct regcache *regcache,
-				   windows_thread_info *th, int r);
+				   windows_nat::windows_thread_info *th,
+				   int r);
 
   /* Store a new register value into the thread context of TH.  */
   void (*store_inferior_register) (struct regcache *regcache,
-				   windows_thread_info *th, int r);
+				   windows_nat::windows_thread_info *th,
+				   int r);
 
-  void (*single_step) (windows_thread_info *th);
+  void (*single_step) (windows_nat::windows_thread_info *th);
 
   const unsigned char *breakpoint;
   int breakpoint_len;
@@ -143,7 +145,7 @@ class win32_process_target : public process_stratum_target
 };
 
 /* Retrieve the context for this thread, if not already retrieved.  */
-extern void win32_require_context (windows_thread_info *th);
+extern void win32_require_context (windows_nat::windows_thread_info *th);
 
 /* Map the Windows error number in ERROR to a locale-dependent error
    message string and return a pointer to it.  Typically, the values
-- 
2.21.1

[PATCH v3 14/29] Share thread_rec between gdb and gdbserver

[Tom Tromey]

This changes gdb and gdbserver to use the same calling convention for
the "thread_rec" helper function.  Fully merging these is difficult
due to differences in how threads are managed by the enclosing
applications; but sharing a declaration makes it possible for future
shared code to call this method.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (enum thread_disposition_type): Move to
	nat/windows-nat.h.
	(windows_nat::thread_rec): Rename from thread_rec.  No longer
	static.
	(windows_add_thread, windows_nat_target::fetch_registers)
	(windows_nat_target::store_registers, handle_exception)
	(windows_nat_target::resume, get_windows_debug_event)
	(windows_nat_target::get_tib_address)
	(windows_nat_target::thread_name)
	(windows_nat_target::thread_alive): Update.
	* nat/windows-nat.h (enum thread_disposition_type): Move from
	windows-nat.c.
	(thread_rec): Declare.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (windows_nat::thread_rec): Rename from thread_rec.
	No longer static.  Change parameters.
	(child_add_thread, child_fetch_inferior_registers)
	(child_store_inferior_registers, win32_resume)
	(win32_get_tib_address): Update.
---
 gdb/ChangeLog          | 16 +++++++++++
 gdb/nat/windows-nat.h  | 21 +++++++++++++++
 gdb/windows-nat.c      | 61 +++++++++++++++---------------------------
 gdbserver/ChangeLog    |  8 ++++++
 gdbserver/win32-low.cc | 22 ++++++++-------
 5 files changed, 79 insertions(+), 49 deletions(-)

diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index df283646753..e63ef753c92 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -90,6 +90,27 @@ struct windows_thread_info
   gdb::unique_xmalloc_ptr<char> name;
 };
 
+
+/* Possible values to pass to 'thread_rec'.  */
+enum thread_disposition_type
+{
+  /* Do not invalidate the thread's context, and do not suspend the
+     thread.  */
+  DONT_INVALIDATE_CONTEXT,
+  /* Invalidate the context, but do not suspend the thread.  */
+  DONT_SUSPEND,
+  /* Invalidate the context and suspend the thread.  */
+  INVALIDATE_CONTEXT
+};
+
+/* Find a thread record given a thread id.  THREAD_DISPOSITION
+   controls whether the thread is suspended, and whether the context
+   is invalidated.
+
+   This function must be supplied by the embedding application.  */
+extern windows_thread_info *thread_rec (ptid_t ptid,
+					thread_disposition_type disposition);
+
 }
 
 #endif
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index b05a27c7cac..1192b9abac4 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -456,26 +456,13 @@ check (BOOL ok, const char *file, int line)
 		     (unsigned) GetLastError ());
 }
 
-/* Possible values to pass to 'thread_rec'.  */
-enum thread_disposition_type
-{
-  /* Do not invalidate the thread's context, and do not suspend the
-     thread.  */
-  DONT_INVALIDATE_CONTEXT,
-  /* Invalidate the context, but do not suspend the thread.  */
-  DONT_SUSPEND,
-  /* Invalidate the context and suspend the thread.  */
-  INVALIDATE_CONTEXT
-};
+/* See nat/windows-nat.h.  */
 
-/* Find a thread record given a thread id.  THREAD_DISPOSITION
-   controls whether the thread is suspended, and whether the context
-   is invalidated.  */
-static windows_thread_info *
-thread_rec (DWORD id, enum thread_disposition_type disposition)
+windows_thread_info *
+windows_nat::thread_rec (ptid_t ptid, thread_disposition_type disposition)
 {
   for (windows_thread_info *th : thread_list)
-    if (th->tid == id)
+    if (th->tid == ptid.lwp ())
       {
 	if (!th->suspended)
 	  {
@@ -485,7 +472,7 @@ thread_rec (DWORD id, enum thread_disposition_type disposition)
 		/* Nothing.  */
 		break;
 	      case INVALIDATE_CONTEXT:
-		if (id != current_event.dwThreadId)
+		if (ptid.lwp () != current_event.dwThreadId)
 		  th->suspend ();
 		th->reload_context = true;
 		break;
@@ -513,13 +500,10 @@ static windows_thread_info *
 windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
 {
   windows_thread_info *th;
-  DWORD id;
 
   gdb_assert (ptid.lwp () != 0);
 
-  id = ptid.lwp ();
-
-  if ((th = thread_rec (id, DONT_INVALIDATE_CONTEXT)))
+  if ((th = thread_rec (ptid, DONT_INVALIDATE_CONTEXT)))
     return th;
 
   CORE_ADDR base = (CORE_ADDR) (uintptr_t) tlb;
@@ -529,7 +513,7 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
   if (wow64_process)
     base += 0x2000;
 #endif
-  th = new windows_thread_info (id, h, base);
+  th = new windows_thread_info (ptid.lwp (), h, base);
   thread_list.push_back (th);
 
   /* Add this new thread to the list of threads.
@@ -716,8 +700,7 @@ windows_fetch_one_register (struct regcache *regcache,
 void
 windows_nat_target::fetch_registers (struct regcache *regcache, int r)
 {
-  DWORD tid = regcache->ptid ().lwp ();
-  windows_thread_info *th = thread_rec (tid, INVALIDATE_CONTEXT);
+  windows_thread_info *th = thread_rec (regcache->ptid (), INVALIDATE_CONTEXT);
 
   /* Check if TH exists.  Windows sometimes uses a non-existent
      thread id in its events.  */
@@ -812,8 +795,7 @@ windows_store_one_register (const struct regcache *regcache,
 void
 windows_nat_target::store_registers (struct regcache *regcache, int r)
 {
-  DWORD tid = regcache->ptid ().lwp ();
-  windows_thread_info *th = thread_rec (tid, INVALIDATE_CONTEXT);
+  windows_thread_info *th = thread_rec (regcache->ptid (), INVALIDATE_CONTEXT);
 
   /* Check if TH exists.  Windows sometimes uses a non-existent
      thread id in its events.  */
@@ -1353,7 +1335,8 @@ handle_exception (struct target_waitstatus *ourstatus)
   ourstatus->kind = TARGET_WAITKIND_STOPPED;
 
   /* Record the context of the current thread.  */
-  thread_rec (current_event.dwThreadId, DONT_SUSPEND);
+  thread_rec (ptid_t (current_event.dwProcessId, current_event.dwThreadId, 0),
+	      DONT_SUSPEND);
 
   switch (code)
     {
@@ -1483,7 +1466,9 @@ handle_exception (struct target_waitstatus *ourstatus)
 	  if (named_thread_id == (DWORD) -1)
 	    named_thread_id = current_event.dwThreadId;
 
-	  named_thread = thread_rec (named_thread_id, DONT_INVALIDATE_CONTEXT);
+	  named_thread = thread_rec (ptid_t (current_event.dwProcessId,
+					     named_thread_id, 0),
+				     DONT_INVALIDATE_CONTEXT);
 	  if (named_thread != NULL)
 	    {
 	      int thread_name_len;
@@ -1714,7 +1699,7 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
 	       ptid.pid (), (unsigned) ptid.lwp (), step, sig));
 
   /* Get context for currently selected thread.  */
-  th = thread_rec (inferior_ptid.lwp (), DONT_INVALIDATE_CONTEXT);
+  th = thread_rec (inferior_ptid, DONT_INVALIDATE_CONTEXT);
   if (th)
     {
 #ifdef __x86_64__
@@ -1847,7 +1832,7 @@ windows_nat_target::get_windows_debug_event (int pid,
 	  current_event = iter->event;
 
 	  inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
-	  current_thread = thread_rec (thread_id, INVALIDATE_CONTEXT);
+	  current_thread = thread_rec (inferior_ptid, INVALIDATE_CONTEXT);
 	  current_thread->reload_context = 1;
 
 	  DEBUG_EVENTS (("get_windows_debug_event - "
@@ -2060,7 +2045,8 @@ windows_nat_target::get_windows_debug_event (int pid,
 	      == EXCEPTION_BREAKPOINT)
 	  && windows_initialization_done)
 	{
-	  th = thread_rec (thread_id, INVALIDATE_CONTEXT);
+	  ptid_t ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
+	  th = thread_rec (ptid, INVALIDATE_CONTEXT);
 	  th->stopped_at_software_breakpoint = true;
 	}
       pending_stops.push_back ({thread_id, *ourstatus, current_event});
@@ -2072,7 +2058,7 @@ windows_nat_target::get_windows_debug_event (int pid,
       inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
       current_thread = th;
       if (!current_thread)
-	current_thread = thread_rec (thread_id, INVALIDATE_CONTEXT);
+	current_thread = thread_rec (inferior_ptid, INVALIDATE_CONTEXT);
     }
 
 out:
@@ -3505,7 +3491,7 @@ windows_nat_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
 {
   windows_thread_info *th;
 
-  th = thread_rec (ptid.lwp (), DONT_INVALIDATE_CONTEXT);
+  th = thread_rec (ptid, DONT_INVALIDATE_CONTEXT);
   if (th == NULL)
     return false;
 
@@ -3526,7 +3512,7 @@ windows_nat_target::get_ada_task_ptid (long lwp, long thread)
 const char *
 windows_nat_target::thread_name (struct thread_info *thr)
 {
-  return thread_rec (thr->ptid.lwp (), DONT_INVALIDATE_CONTEXT)->name.get ();
+  return thread_rec (thr->ptid, DONT_INVALIDATE_CONTEXT)->name.get ();
 }
 
 
@@ -3685,12 +3671,9 @@ cygwin_get_dr7 (void)
 bool
 windows_nat_target::thread_alive (ptid_t ptid)
 {
-  int tid;
-
   gdb_assert (ptid.lwp () != 0);
-  tid = ptid.lwp ();
 
-  return (WaitForSingleObject (thread_rec (tid, DONT_INVALIDATE_CONTEXT)->h, 0)
+  return (WaitForSingleObject (thread_rec (ptid, DONT_INVALIDATE_CONTEXT)->h, 0)
 	  != WAIT_OBJECT_0);
 }
 
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 8f8b6cedd28..1e86b3b5926 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -178,17 +178,17 @@ win32_require_context (windows_thread_info *th)
     }
 }
 
-/* Find a thread record given a thread id.  If GET_CONTEXT is set then
-   also retrieve the context for this thread.  */
-static windows_thread_info *
-thread_rec (ptid_t ptid, int get_context)
+/* See nat/windows-nat.h.  */
+
+windows_thread_info *
+windows_nat::thread_rec (ptid_t ptid, thread_disposition_type disposition)
 {
   thread_info *thread = find_thread_ptid (ptid);
   if (thread == NULL)
     return NULL;
 
   windows_thread_info *th = (windows_thread_info *) thread_target_data (thread);
-  if (get_context)
+  if (disposition != DONT_INVALIDATE_CONTEXT)
     win32_require_context (th);
   return th;
 }
@@ -200,7 +200,7 @@ child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
   windows_thread_info *th;
   ptid_t ptid = ptid_t (pid, tid, 0);
 
-  if ((th = thread_rec (ptid, FALSE)))
+  if ((th = thread_rec (ptid, DONT_INVALIDATE_CONTEXT)))
     return th;
 
   th = new windows_thread_info (tid, h, (CORE_ADDR) (uintptr_t) tlb);
@@ -454,7 +454,8 @@ static void
 child_fetch_inferior_registers (struct regcache *regcache, int r)
 {
   int regno;
-  windows_thread_info *th = thread_rec (current_thread_ptid (), TRUE);
+  windows_thread_info *th = thread_rec (current_thread_ptid (),
+					INVALIDATE_CONTEXT);
   if (r == -1 || r > NUM_REGS)
     child_fetch_inferior_registers (regcache, NUM_REGS);
   else
@@ -468,7 +469,8 @@ static void
 child_store_inferior_registers (struct regcache *regcache, int r)
 {
   int regno;
-  windows_thread_info *th = thread_rec (current_thread_ptid (), TRUE);
+  windows_thread_info *th = thread_rec (current_thread_ptid (),
+					INVALIDATE_CONTEXT);
   if (r == -1 || r == 0 || r > NUM_REGS)
     child_store_inferior_registers (regcache, NUM_REGS);
   else
@@ -937,7 +939,7 @@ win32_process_target::resume (thread_resume *resume_info, size_t n)
 
   /* Get context for the currently selected thread.  */
   ptid = debug_event_ptid (&current_event);
-  th = thread_rec (ptid, FALSE);
+  th = thread_rec (ptid, DONT_INVALIDATE_CONTEXT);
   if (th)
     {
       win32_prepare_to_resume (th);
@@ -1807,7 +1809,7 @@ int
 win32_process_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
 {
   windows_thread_info *th;
-  th = thread_rec (ptid, 0);
+  th = thread_rec (ptid, DONT_INVALIDATE_CONTEXT);
   if (th == NULL)
     return 0;
   if (addr != NULL)
-- 
2.21.1

[PATCH v3 15/29] Share get_image_name between gdb and gdbserver

[Tom Tromey]

This moves get_image_name to nat/windows-nat.c so that it can be
shared between gdb and gdbserver.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (get_image_name): Move to nat/windows-nat.c.
	(handle_load_dll): Update.
	* nat/windows-nat.c (get_image_name): Move from windows-nat.c.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (get_image_name): Remove.
	(handle_load_dll): Update.
---
 gdb/ChangeLog          |  6 +++++
 gdb/nat/windows-nat.c  | 57 ++++++++++++++++++++++++++++++++++++++++++
 gdb/nat/windows-nat.h  |  7 ++++++
 gdb/windows-nat.c      | 52 +-------------------------------------
 gdbserver/ChangeLog    |  5 ++++
 gdbserver/win32-low.cc | 51 +------------------------------------
 6 files changed, 77 insertions(+), 101 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 6c0218cd9d8..8217a985320 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -69,4 +69,61 @@ windows_thread_info::resume ()
   suspended = 0;
 }
 
+const char *
+get_image_name (HANDLE h, void *address, int unicode)
+{
+#ifdef __CYGWIN__
+  static char buf[MAX_PATH];
+#else
+  static char buf[(2 * MAX_PATH) + 1];
+#endif
+  DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
+  char *address_ptr;
+  int len = 0;
+  char b[2];
+  SIZE_T done;
+
+  /* Attempt to read the name of the dll that was detected.
+     This is documented to work only when actively debugging
+     a program.  It will not work for attached processes.  */
+  if (address == NULL)
+    return NULL;
+
+#ifdef _WIN32_WCE
+  /* Windows CE reports the address of the image name,
+     instead of an address of a pointer into the image name.  */
+  address_ptr = address;
+#else
+  /* See if we could read the address of a string, and that the
+     address isn't null.  */
+  if (!ReadProcessMemory (h, address,  &address_ptr,
+			  sizeof (address_ptr), &done)
+      || done != sizeof (address_ptr)
+      || !address_ptr)
+    return NULL;
+#endif
+
+  /* Find the length of the string.  */
+  while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
+	 && (b[0] != 0 || b[size - 1] != 0) && done == size)
+    continue;
+
+  if (!unicode)
+    ReadProcessMemory (h, address_ptr, buf, len, &done);
+  else
+    {
+      WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
+      ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
+			 &done);
+#ifdef __CYGWIN__
+      wcstombs (buf, unicode_address, MAX_PATH);
+#else
+      WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, sizeof buf,
+			   0, 0);
+#endif
+    }
+
+  return buf;
+}
+
 }
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index e63ef753c92..4176ed7f660 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -111,6 +111,13 @@ enum thread_disposition_type
 extern windows_thread_info *thread_rec (ptid_t ptid,
 					thread_disposition_type disposition);
 
+/* Return the name of the DLL referenced by H at ADDRESS.  UNICODE
+   determines what sort of string is read from the inferior.  Returns
+   the name of the DLL, or NULL on error.  If a name is returned, it
+   is stored in a static buffer which is valid until the next call to
+   get_image_name.  */
+extern const char *get_image_name (HANDLE h, void *address, int unicode);
+
 }
 
 #endif
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 1192b9abac4..c0b1b8521db 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -921,56 +921,6 @@ windows_make_so (const char *name, LPVOID load_addr)
   return so;
 }
 
-static char *
-get_image_name (HANDLE h, void *address, int unicode)
-{
-#ifdef __CYGWIN__
-  static char buf[__PMAX];
-#else
-  static char buf[(2 * __PMAX) + 1];
-#endif
-  DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
-  char *address_ptr;
-  int len = 0;
-  char b[2];
-  SIZE_T done;
-
-  /* Attempt to read the name of the dll that was detected.
-     This is documented to work only when actively debugging
-     a program.  It will not work for attached processes.  */
-  if (address == NULL)
-    return NULL;
-
-  /* See if we could read the address of a string, and that the
-     address isn't null.  */
-  if (!ReadProcessMemory (h, address,  &address_ptr,
-			  sizeof (address_ptr), &done)
-      || done != sizeof (address_ptr) || !address_ptr)
-    return NULL;
-
-  /* Find the length of the string.  */
-  while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
-	 && (b[0] != 0 || b[size - 1] != 0) && done == size)
-    continue;
-
-  if (!unicode)
-    ReadProcessMemory (h, address_ptr, buf, len, &done);
-  else
-    {
-      WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
-      ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
-			 &done);
-#ifdef __CYGWIN__
-      wcstombs (buf, unicode_address, __PMAX);
-#else
-      WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, sizeof buf,
-			   0, 0);
-#endif
-    }
-
-  return buf;
-}
-
 /* Handle a DLL load event, and return 1.
 
    This function assumes that this event did not occur during inferior
@@ -982,7 +932,7 @@ static void
 handle_load_dll ()
 {
   LOAD_DLL_DEBUG_INFO *event = &current_event.u.LoadDll;
-  char *dll_name;
+  const char *dll_name;
 
   /* Try getting the DLL name via the lpImageName field of the event.
      Note that Microsoft documents this fields as strictly optional,
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 1e86b3b5926..810896e87ca 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -1031,55 +1031,6 @@ win32_add_one_solib (const char *name, CORE_ADDR load_addr)
   loaded_dll (buf2, load_addr);
 }
 
-static char *
-get_image_name (HANDLE h, void *address, int unicode)
-{
-  static char buf[(2 * MAX_PATH) + 1];
-  DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
-  char *address_ptr;
-  int len = 0;
-  char b[2];
-  SIZE_T done;
-
-  /* Attempt to read the name of the dll that was detected.
-     This is documented to work only when actively debugging
-     a program.  It will not work for attached processes. */
-  if (address == NULL)
-    return NULL;
-
-#ifdef _WIN32_WCE
-  /* Windows CE reports the address of the image name,
-     instead of an address of a pointer into the image name.  */
-  address_ptr = address;
-#else
-  /* See if we could read the address of a string, and that the
-     address isn't null. */
-  if (!ReadProcessMemory (h, address,  &address_ptr,
-			  sizeof (address_ptr), &done)
-      || done != sizeof (address_ptr)
-      || !address_ptr)
-    return NULL;
-#endif
-
-  /* Find the length of the string */
-  while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
-	 && (b[0] != 0 || b[size - 1] != 0) && done == size)
-    continue;
-
-  if (!unicode)
-    ReadProcessMemory (h, address_ptr, buf, len, &done);
-  else
-    {
-      WCHAR *unicode_address = XALLOCAVEC (WCHAR, len);
-      ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
-			 &done);
-
-      WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0, 0);
-    }
-
-  return buf;
-}
-
 typedef BOOL (WINAPI *winapi_EnumProcessModules) (HANDLE, HMODULE *,
 						  DWORD, LPDWORD);
 typedef BOOL (WINAPI *winapi_GetModuleInformation) (HANDLE, HMODULE,
@@ -1188,7 +1139,7 @@ static void
 handle_load_dll (void)
 {
   LOAD_DLL_DEBUG_INFO *event = &current_event.u.LoadDll;
-  char *dll_name;
+  const char *dll_name;
 
   dll_name = get_image_name (current_process_handle,
 			     event->lpImageName, event->fUnicode);
-- 
2.21.1

[PATCH v3 16/29] Share some Windows-related globals

[Tom Tromey]

This moves some Windows-related globals into nat/windows-nat.c,
sharing them between gdb and gdbserver.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (current_process_handle, current_process_id)
	(main_thread_id, last_sig, current_event, last_wait_event)
	(current_windows_thread, desired_stop_thread_id, pending_stops)
	(struct pending_stop, siginfo_er): Move to nat/windows-nat.c.
	(display_selectors, fake_create_process)
	(get_windows_debug_event): Update.
	* nat/windows-nat.h (current_process_handle, current_process_id)
	(main_thread_id, last_sig, current_event, last_wait_event)
	(current_windows_thread, desired_stop_thread_id, pending_stops)
	(struct pending_stop, siginfo_er): Move from windows-nat.c.
	* nat/windows-nat.c (current_process_handle, current_process_id)
	(main_thread_id, last_sig, current_event, last_wait_event)
	(current_windows_thread, desired_stop_thread_id, pending_stops)
	(siginfo_er): New globals.  Move from windows-nat.c.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (current_process_handle, current_process_id)
	(main_thread_id, last_sig, current_event, siginfo_er): Move to
	nat/windows-nat.c.
---
 gdb/ChangeLog          |  17 ++++++
 gdb/nat/windows-nat.c  |  13 ++++-
 gdb/nat/windows-nat.h  |  57 +++++++++++++++++++
 gdb/windows-nat.c      | 125 +++++++++++++----------------------------
 gdbserver/ChangeLog    |   6 ++
 gdbserver/win32-low.cc |   8 ---
 6 files changed, 130 insertions(+), 96 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 8217a985320..80a1583b884 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -1,5 +1,5 @@
 /* Internal interfaces for the Windows code
-   Copyright (C) 1995-2019 Free Software Foundation, Inc.
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -22,6 +22,17 @@
 namespace windows_nat
 {
 
+HANDLE current_process_handle;
+DWORD current_process_id;
+DWORD main_thread_id;
+enum gdb_signal last_sig = GDB_SIGNAL_0;
+DEBUG_EVENT current_event;
+DEBUG_EVENT last_wait_event;
+windows_thread_info *current_windows_thread;
+DWORD desired_stop_thread_id = -1;
+std::vector<pending_stop> pending_stops;
+EXCEPTION_RECORD siginfo_er;
+
 windows_thread_info::~windows_thread_info ()
 {
   CloseHandle (h);
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 4176ed7f660..501147b2c90 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -20,6 +20,9 @@
 #define NAT_WINDOWS_NAT_H
 
 #include <windows.h>
+#include <vector>
+
+#include "target/waitstatus.h"
 
 namespace windows_nat
 {
@@ -111,6 +114,60 @@ enum thread_disposition_type
 extern windows_thread_info *thread_rec (ptid_t ptid,
 					thread_disposition_type disposition);
 
+/* Currently executing process */
+extern HANDLE current_process_handle;
+extern DWORD current_process_id;
+extern DWORD main_thread_id;
+extern enum gdb_signal last_sig;
+
+/* The current debug event from WaitForDebugEvent or from a pending
+   stop.  */
+extern DEBUG_EVENT current_event;
+
+/* The most recent event from WaitForDebugEvent.  Unlike
+   current_event, this is guaranteed never to come from a pending
+   stop.  This is important because only data from the most recent
+   event from WaitForDebugEvent can be used when calling
+   ContinueDebugEvent.  */
+extern DEBUG_EVENT last_wait_event;
+
+/* Info on currently selected thread */
+extern windows_thread_info *current_windows_thread;
+
+/* The ID of the thread for which we anticipate a stop event.
+   Normally this is -1, meaning we'll accept an event in any
+   thread.  */
+extern DWORD desired_stop_thread_id;
+
+/* A single pending stop.  See "pending_stops" for more
+   information.  */
+struct pending_stop
+{
+  /* The thread id.  */
+  DWORD thread_id;
+
+  /* The target waitstatus we computed.  */
+  target_waitstatus status;
+
+  /* The event.  A few fields of this can be referenced after a stop,
+     and it seemed simplest to store the entire event.  */
+  DEBUG_EVENT event;
+};
+
+/* A vector of pending stops.  Sometimes, Windows will report a stop
+   on a thread that has been ostensibly suspended.  We believe what
+   happens here is that two threads hit a breakpoint simultaneously,
+   and the Windows kernel queues the stop events.  However, this can
+   result in the strange effect of trying to single step thread A --
+   leaving all other threads suspended -- and then seeing a stop in
+   thread B.  To handle this scenario, we queue all such "pending"
+   stops here, and then process them once the step has completed.  See
+   PR gdb/22992.  */
+extern std::vector<pending_stop> pending_stops;
+
+/* Contents of $_siginfo */
+extern EXCEPTION_RECORD siginfo_er;
+
 /* Return the name of the DLL referenced by H at ADDRESS.  UNICODE
    determines what sort of string is read from the inferior.  Returns
    the name of the DLL, or NULL on error.  If a name is returned, it
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index c0b1b8521db..74b852ca603 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -244,28 +244,8 @@ static CORE_ADDR cygwin_get_dr (int i);
 static unsigned long cygwin_get_dr6 (void);
 static unsigned long cygwin_get_dr7 (void);
 
-static enum gdb_signal last_sig = GDB_SIGNAL_0;
-/* Set if a signal was received from the debugged process.  */
-
 static std::vector<windows_thread_info *> thread_list;
 
-/* The process and thread handles for the above context.  */
-
-/* The current debug event from WaitForDebugEvent or from a pending
-   stop.  */
-static DEBUG_EVENT current_event;
-
-/* The most recent event from WaitForDebugEvent.  Unlike
-   current_event, this is guaranteed never to come from a pending
-   stop.  This is important because only data from the most recent
-   event from WaitForDebugEvent can be used when calling
-   ContinueDebugEvent.  */
-static DEBUG_EVENT last_wait_event;
-
-static HANDLE current_process_handle;	/* Currently executing process */
-static windows_thread_info *current_thread;	/* Info on currently selected thread */
-static EXCEPTION_RECORD siginfo_er;	/* Contents of $_siginfo */
-
 /* Counts of things.  */
 static int exception_count = 0;
 static int event_count = 0;
@@ -336,37 +316,6 @@ static const struct xlate_exception xlate[] =
 
 #endif /* 0 */
 
-/* The ID of the thread for which we anticipate a stop event.
-   Normally this is -1, meaning we'll accept an event in any
-   thread.  */
-static DWORD desired_stop_thread_id = -1;
-
-/* A single pending stop.  See "pending_stops" for more
-   information.  */
-struct pending_stop
-{
-  /* The thread id.  */
-  DWORD thread_id;
-
-  /* The target waitstatus we computed.  */
-  target_waitstatus status;
-
-  /* The event.  A few fields of this can be referenced after a stop,
-     and it seemed simplest to store the entire event.  */
-  DEBUG_EVENT event;
-};
-
-/* A vector of pending stops.  Sometimes, Windows will report a stop
-   on a thread that has been ostensibly suspended.  We believe what
-   happens here is that two threads hit a breakpoint simultaneously,
-   and the Windows kernel queues the stop events.  However, this can
-   result in the strange effect of trying to single step thread A --
-   leaving all other threads suspended -- and then seeing a stop in
-   thread B.  To handle this scenario, we queue all such "pending"
-   stops here, and then process them once the step has completed.  See
-   PR gdb/22992.  */
-static std::vector<pending_stop> pending_stops;
-
 struct windows_nat_target final : public x86_nat_target<inf_child_target>
 {
   void close () override;
@@ -387,7 +336,7 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
 
   bool stopped_by_sw_breakpoint () override
   {
-    return current_thread->stopped_at_software_breakpoint;
+    return current_windows_thread->stopped_at_software_breakpoint;
   }
 
   bool supports_stopped_by_sw_breakpoint () override
@@ -1207,7 +1156,7 @@ display_selector (HANDLE thread, DWORD sel)
 static void
 display_selectors (const char * args, int from_tty)
 {
-  if (!current_thread)
+  if (!current_windows_thread)
     {
       puts_filtered ("Impossible to display selectors now.\n");
       return;
@@ -1218,45 +1167,45 @@ display_selectors (const char * args, int from_tty)
       if (wow64_process)
 	{
 	  puts_filtered ("Selector $cs\n");
-	  display_selector (current_thread->h,
-			    current_thread->wow64_context.SegCs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->wow64_context.SegCs);
 	  puts_filtered ("Selector $ds\n");
-	  display_selector (current_thread->h,
-			    current_thread->wow64_context.SegDs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->wow64_context.SegDs);
 	  puts_filtered ("Selector $es\n");
-	  display_selector (current_thread->h,
-			    current_thread->wow64_context.SegEs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->wow64_context.SegEs);
 	  puts_filtered ("Selector $ss\n");
-	  display_selector (current_thread->h,
-			    current_thread->wow64_context.SegSs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->wow64_context.SegSs);
 	  puts_filtered ("Selector $fs\n");
-	  display_selector (current_thread->h,
-			    current_thread->wow64_context.SegFs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->wow64_context.SegFs);
 	  puts_filtered ("Selector $gs\n");
-	  display_selector (current_thread->h,
-			    current_thread->wow64_context.SegGs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->wow64_context.SegGs);
 	}
       else
 #endif
 	{
 	  puts_filtered ("Selector $cs\n");
-	  display_selector (current_thread->h,
-			    current_thread->context.SegCs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->context.SegCs);
 	  puts_filtered ("Selector $ds\n");
-	  display_selector (current_thread->h,
-			    current_thread->context.SegDs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->context.SegDs);
 	  puts_filtered ("Selector $es\n");
-	  display_selector (current_thread->h,
-			    current_thread->context.SegEs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->context.SegEs);
 	  puts_filtered ("Selector $ss\n");
-	  display_selector (current_thread->h,
-			    current_thread->context.SegSs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->context.SegSs);
 	  puts_filtered ("Selector $fs\n");
-	  display_selector (current_thread->h,
-			    current_thread->context.SegFs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->context.SegFs);
 	  puts_filtered ("Selector $gs\n");
-	  display_selector (current_thread->h,
-			    current_thread->context.SegGs);
+	  display_selector (current_windows_thread->h,
+			    current_windows_thread->context.SegGs);
 	}
     }
   else
@@ -1264,7 +1213,7 @@ display_selectors (const char * args, int from_tty)
       int sel;
       sel = parse_and_eval_long (args);
       printf_filtered ("Selector \"%s\"\n",args);
-      display_selector (current_thread->h, sel);
+      display_selector (current_windows_thread->h, sel);
     }
 }
 
@@ -1587,7 +1536,7 @@ fake_create_process (void)
        (unsigned) GetLastError ());
       /*  We can not debug anything in that case.  */
     }
-  current_thread
+  current_windows_thread
     = windows_add_thread (ptid_t (current_event.dwProcessId,
 				  current_event.dwThreadId, 0),
 			  current_event.u.CreateThread.hThread,
@@ -1782,8 +1731,9 @@ windows_nat_target::get_windows_debug_event (int pid,
 	  current_event = iter->event;
 
 	  inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
-	  current_thread = thread_rec (inferior_ptid, INVALIDATE_CONTEXT);
-	  current_thread->reload_context = 1;
+	  current_windows_thread = thread_rec (inferior_ptid,
+					       INVALIDATE_CONTEXT);
+	  current_windows_thread->reload_context = 1;
 
 	  DEBUG_EVENTS (("get_windows_debug_event - "
 			 "pending stop found in 0x%x (desired=0x%x)\n",
@@ -2006,9 +1956,10 @@ windows_nat_target::get_windows_debug_event (int pid,
   else
     {
       inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
-      current_thread = th;
-      if (!current_thread)
-	current_thread = thread_rec (inferior_ptid, INVALIDATE_CONTEXT);
+      current_windows_thread = th;
+      if (!current_windows_thread)
+	current_windows_thread = thread_rec (inferior_ptid,
+					     INVALIDATE_CONTEXT);
     }
 
 out:
@@ -2066,14 +2017,14 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	{
 	  ptid_t result = ptid_t (current_event.dwProcessId, retval, 0);
 
-	  if (current_thread != nullptr)
+	  if (current_windows_thread != nullptr)
 	    {
-	      current_thread->stopped_at_software_breakpoint = false;
+	      current_windows_thread->stopped_at_software_breakpoint = false;
 	      if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
 		  && (current_event.u.Exception.ExceptionRecord.ExceptionCode
 		      == EXCEPTION_BREAKPOINT)
 		  && windows_initialization_done)
-		current_thread->stopped_at_software_breakpoint = true;
+		current_windows_thread->stopped_at_software_breakpoint = true;
 	    }
 
 	  return result;
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 810896e87ca..7060b6d1527 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -74,14 +74,6 @@ int using_threads = 1;
 
 /* Globals.  */
 static int attaching = 0;
-static HANDLE current_process_handle = NULL;
-static DWORD current_process_id = 0;
-static DWORD main_thread_id = 0;
-static EXCEPTION_RECORD siginfo_er;	/* Contents of $_siginfo */
-static enum gdb_signal last_sig = GDB_SIGNAL_0;
-
-/* The current debug event from WaitForDebugEvent.  */
-static DEBUG_EVENT current_event;
 
 /* A status that hasn't been reported to the core yet, and so
    win32_wait should return it next, instead of fetching the next
-- 
2.21.1

[PATCH v3 17/29] Normalize handle_output_debug_string API

[Tom Tromey]

This changes gdbserver's implementation of handle_output_debug_string
to have the same calling convention as that of gdb.  This allows for
sharing some more code in a subsequent patch.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (windows_nat::handle_output_debug_string):
	Rename.  No longer static.
	* nat/windows-nat.h (handle_output_debug_string): Declare.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (handle_output_debug_string): Add parameter.  Change
	return type.
	(win32_kill, get_child_debug_event): Update.
---
 gdb/ChangeLog          |  6 ++++++
 gdb/nat/windows-nat.h  | 11 +++++++++++
 gdb/windows-nat.c      |  9 ++++-----
 gdbserver/ChangeLog    |  6 ++++++
 gdbserver/win32-low.cc | 21 ++++++++++++---------
 5 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 501147b2c90..f438befbc94 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -114,6 +114,17 @@ enum thread_disposition_type
 extern windows_thread_info *thread_rec (ptid_t ptid,
 					thread_disposition_type disposition);
 
+
+/* Handle OUTPUT_DEBUG_STRING_EVENT from child process.  Updates
+   OURSTATUS and returns the thread id if this represents a thread
+   change (this is specific to Cygwin), otherwise 0.
+
+   Cygwin prepends its messages with a "cygwin:".  Interpret this as
+   a Cygwin signal.  Otherwise just print the string as a warning.
+
+   This function must be supplied by the embedding application.  */
+extern int handle_output_debug_string (struct target_waitstatus *ourstatus);
+
 /* Currently executing process */
 extern HANDLE current_process_handle;
 extern DWORD current_process_id;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 74b852ca603..370e7bc1ae5 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1004,11 +1004,10 @@ signal_event_command (const char *args, int from_tty)
   CloseHandle ((HANDLE) event_id);
 }
 
-/* Handle DEBUG_STRING output from child process.
-   Cygwin prepends its messages with a "cygwin:".  Interpret this as
-   a Cygwin signal.  Otherwise just print the string as a warning.  */
-static int
-handle_output_debug_string (struct target_waitstatus *ourstatus)
+/* See nat/windows-nat.h.  */
+
+int
+windows_nat::handle_output_debug_string (struct target_waitstatus *ourstatus)
 {
   gdb::unique_xmalloc_ptr<char> s;
   int retval = 0;
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 7060b6d1527..2130366747c 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -733,9 +733,10 @@ win32_process_target::attach (unsigned long pid)
 	 (int) err, strwinerror (err));
 }
 
-/* Handle OUTPUT_DEBUG_STRING_EVENT from child process.  */
-static void
-handle_output_debug_string (void)
+/* See nat/windows-nat.h.  */
+
+int
+windows_nat::handle_output_debug_string (struct target_waitstatus *ourstatus)
 {
 #define READ_BUFFER_LEN 1024
   CORE_ADDR addr;
@@ -743,7 +744,7 @@ handle_output_debug_string (void)
   DWORD nbytes = current_event.u.DebugString.nDebugStringLength;
 
   if (nbytes == 0)
-    return;
+    return 0;
 
   if (nbytes > READ_BUFFER_LEN)
     nbytes = READ_BUFFER_LEN;
@@ -756,13 +757,13 @@ handle_output_debug_string (void)
 	 in Unicode.  */
       WCHAR buffer[(READ_BUFFER_LEN + 1) / sizeof (WCHAR)] = { 0 };
       if (read_inferior_memory (addr, (unsigned char *) buffer, nbytes) != 0)
-	return;
+	return 0;
       wcstombs (s, buffer, (nbytes + 1) / sizeof (WCHAR));
     }
   else
     {
       if (read_inferior_memory (addr, (unsigned char *) s, nbytes) != 0)
-	return;
+	return 0;
     }
 
   if (!startswith (s, "cYg"))
@@ -770,12 +771,14 @@ handle_output_debug_string (void)
       if (!server_waiting)
 	{
 	  OUTMSG2(("%s", s));
-	  return;
+	  return 0;
 	}
 
       monitor_output (s);
     }
 #undef READ_BUFFER_LEN
+
+  return 0;
 }
 
 static void
@@ -804,7 +807,7 @@ win32_process_target::kill (process_info *process)
       if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT)
 	break;
       else if (current_event.dwDebugEventCode == OUTPUT_DEBUG_STRING_EVENT)
-	handle_output_debug_string ();
+	handle_output_debug_string (nullptr);
     }
 
   win32_clear_inferiors ();
@@ -1504,7 +1507,7 @@ get_child_debug_event (struct target_waitstatus *ourstatus)
 		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
-      handle_output_debug_string ();
+      handle_output_debug_string (nullptr);
       break;
 
     default:
-- 
2.21.1

[PATCH v3 18/29] Fix up complaints.h for namespace use

[Tom Tromey]

If 'complaint' is used in a namespace context, it will fail because
'stop_whining' is only declared at the top level.  This patch fixes
this problem in a simple way, by moving the declaration of
'stop_whining' out of the macro and to the top-level.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* complaints.h (stop_whining): Declare at top-level.
	(complaint): Don't declare stop_whining.
---
 gdb/ChangeLog    | 5 +++++
 gdb/complaints.h | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gdb/complaints.h b/gdb/complaints.h
index b3bb4068e1b..6ad056d257e 100644
--- a/gdb/complaints.h
+++ b/gdb/complaints.h
@@ -25,6 +25,10 @@
 extern void complaint_internal (const char *fmt, ...)
   ATTRIBUTE_PRINTF (1, 2);
 
+/* This controls whether complaints are emitted.  */
+
+extern int stop_whining;
+
 /* Register a complaint.  This is a macro around complaint_internal to
    avoid computing complaint's arguments when complaints are disabled.
    Running FMT via gettext [i.e., _(FMT)] can be quite expensive, for
@@ -32,8 +36,6 @@ extern void complaint_internal (const char *fmt, ...)
 #define complaint(FMT, ...)					\
   do								\
     {								\
-      extern int stop_whining;					\
-								\
       if (stop_whining > 0)					\
 	complaint_internal (FMT, ##__VA_ARGS__);		\
     }								\
-- 
2.21.1

[PATCH v3 19/29] Share handle_load_dll and handle_unload_dll declarations

[Tom Tromey]

This changes nat/windows-nat.h to declare handle_load_dll and
handle_unload_dll.  The embedding application is required to implement
these -- while the actual code was difficult to share due to some
other differences between the two programs, sharing the declaration
lets a subsequent patch share more code that uses these as callbacks.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (windows_nat::handle_load_dll)
	(windows_nat::handle_unload_dll): Rename.  No longer static.
	* nat/windows-nat.h (handle_load_dll, handle_unload_dll):
	Declare.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (windows_nat::handle_load_dll): Rename from
	handle_load_dll.  No longer static.
	(windows_nat::handle_unload_dll): Rename from handle_unload_dll.
	No longer static.
---
 gdb/ChangeLog          |  7 +++++++
 gdb/nat/windows-nat.h  | 19 +++++++++++++++++++
 gdb/windows-nat.c      | 23 ++++++-----------------
 gdbserver/ChangeLog    |  7 +++++++
 gdbserver/win32-low.cc | 22 ++++++----------------
 5 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index f438befbc94..2b2fd116269 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -125,6 +125,25 @@ extern windows_thread_info *thread_rec (ptid_t ptid,
    This function must be supplied by the embedding application.  */
 extern int handle_output_debug_string (struct target_waitstatus *ourstatus);
 
+/* Handle a DLL load event.
+
+   This function assumes that the current event did not occur during
+   inferior initialization.
+
+   This function must be supplied by the embedding application.  */
+
+extern void handle_load_dll ();
+
+/* Handle a DLL unload event.
+
+   This function assumes that this event did not occur during inferior
+   initialization.
+
+   This function must be supplied by the embedding application.  */
+
+extern void handle_unload_dll ();
+
+
 /* Currently executing process */
 extern HANDLE current_process_handle;
 extern DWORD current_process_id;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 370e7bc1ae5..7a68f2d355b 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -870,15 +870,10 @@ windows_make_so (const char *name, LPVOID load_addr)
   return so;
 }
 
-/* Handle a DLL load event, and return 1.
-
-   This function assumes that this event did not occur during inferior
-   initialization, where their event info may be incomplete (see
-   do_initial_windows_stuff and windows_add_all_dlls for more info
-   on how we handle DLL loading during that phase).  */
+/* See nat/windows-nat.h.  */
 
-static void
-handle_load_dll ()
+void
+windows_nat::handle_load_dll ()
 {
   LOAD_DLL_DEBUG_INFO *event = &current_event.u.LoadDll;
   const char *dll_name;
@@ -911,16 +906,10 @@ windows_free_so (struct so_list *so)
   xfree (so);
 }
 
-/* Handle a DLL unload event.
-   Return 1 if successful, or zero otherwise.
-
-   This function assumes that this event did not occur during inferior
-   initialization, where their event info may be incomplete (see
-   do_initial_windows_stuff and windows_add_all_dlls for more info
-   on how we handle DLL loading during that phase).  */
+/* See nat/windows-nat.h.  */
 
-static void
-handle_unload_dll ()
+void
+windows_nat::handle_unload_dll ()
 {
   LPVOID lpBaseOfDll = current_event.u.UnloadDll.lpBaseOfDll;
   struct so_list *so;
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 2130366747c..73d4a6a2d8a 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -1123,15 +1123,10 @@ typedef HANDLE (WINAPI *winapi_CreateToolhelp32Snapshot) (DWORD, DWORD);
 typedef BOOL (WINAPI *winapi_Module32First) (HANDLE, LPMODULEENTRY32);
 typedef BOOL (WINAPI *winapi_Module32Next) (HANDLE, LPMODULEENTRY32);
 
-/* Handle a DLL load event.
-
-   This function assumes that this event did not occur during inferior
-   initialization, where their event info may be incomplete (see
-   do_initial_child_stuff and win32_add_all_dlls for more info on
-   how we handle DLL loading during that phase).  */
+/* See nat/windows-nat.h.  */
 
-static void
-handle_load_dll (void)
+void
+windows_nat::handle_load_dll ()
 {
   LOAD_DLL_DEBUG_INFO *event = &current_event.u.LoadDll;
   const char *dll_name;
@@ -1144,15 +1139,10 @@ handle_load_dll (void)
   win32_add_one_solib (dll_name, (CORE_ADDR) (uintptr_t) event->lpBaseOfDll);
 }
 
-/* Handle a DLL unload event.
-
-   This function assumes that this event did not occur during inferior
-   initialization, where their event info may be incomplete (see
-   do_initial_child_stuff and win32_add_one_solib for more info
-   on how we handle DLL loading during that phase).  */
+/* See nat/windows-nat.h.  */
 
-static void
-handle_unload_dll (void)
+void
+windows_nat::handle_unload_dll ()
 {
   CORE_ADDR load_addr =
 	  (CORE_ADDR) (uintptr_t) current_event.u.UnloadDll.lpBaseOfDll;
-- 
2.21.1

[PATCH v3 20/29] Remove some globals from windows-nat.c

[Tom Tromey]

windows-nat.c has a few "count" globals that don't seem to be used.
Possibly they were used for debugging at some point, but they no
longer seem useful to me.  Because they get in the way of some code
sharing, this patch removes them.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (exception_count, event_count): Remove.
	(handle_exception, get_windows_debug_event)
	(do_initial_windows_stuff): Update.
---
 gdb/ChangeLog     | 6 ++++++
 gdb/windows-nat.c | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 7a68f2d355b..3dc31ebd452 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -247,8 +247,6 @@ static unsigned long cygwin_get_dr7 (void);
 static std::vector<windows_thread_info *> thread_list;
 
 /* Counts of things.  */
-static int exception_count = 0;
-static int event_count = 0;
 static int saw_create;
 static int open_process_used = 0;
 #ifdef __x86_64__
@@ -1386,7 +1384,6 @@ handle_exception (struct target_waitstatus *ourstatus)
       ourstatus->value.sig = GDB_SIGNAL_UNKNOWN;
       break;
     }
-  exception_count++;
   last_sig = ourstatus->value.sig;
   return result;
 }
@@ -1737,7 +1734,6 @@ windows_nat_target::get_windows_debug_event (int pid,
   if (!(debug_event = wait_for_debug_event (&current_event, 1000)))
     goto out;
 
-  event_count++;
   continue_status = DBG_CONTINUE;
 
   event_code = current_event.dwDebugEventCode;
@@ -2111,8 +2107,6 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
   struct inferior *inf;
 
   last_sig = GDB_SIGNAL_0;
-  event_count = 0;
-  exception_count = 0;
   open_process_used = 0;
   debug_registers_changed = 0;
   debug_registers_used = 0;
-- 
2.21.1

[PATCH v3 21/29] Share handle_exception

[Tom Tromey]

Both gdb and gdbserver have a "handle_exception" function, the bulk of
which is shared between the two implementations.  This patch arranges
for the entire thing to be moved into nat/windows-nat.c, with the
differences handled by callbacks.  This patch introduces one more
callback to make this possible.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (MS_VC_EXCEPTION): Move to nat/windows-nat.c.
	(handle_exception_result): Move to nat/windows-nat.h.
	(DEBUG_EXCEPTION_SIMPLE): Remove.
	(windows_nat::handle_ms_vc_exception): New function.
	(handle_exception): Move to nat/windows-nat.c.
	(get_windows_debug_event): Update.
	(STATUS_WX86_BREAKPOINT, STATUS_WX86_SINGLE_STEP): Move to
	nat/windows-nat.c.
	* nat/windows-nat.h (handle_ms_vc_exception): Declare.
	(handle_exception_result): Move from windows-nat.c.
	(handle_exception): Declare.
	* nat/windows-nat.c (MS_VC_EXCEPTION, handle_exception)
	(STATUS_WX86_SINGLE_STEP, STATUS_WX86_BREAKPOINT): Move from
	windows-nat.c.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (handle_exception): Remove.
	(windows_nat::handle_ms_vc_exception): New function.
	(get_child_debug_event): Add "continue_status" parameter.
	Update.
	(win32_wait): Update.
---
 gdb/ChangeLog          |  17 ++++
 gdb/nat/windows-nat.c  | 175 +++++++++++++++++++++++++++++++++
 gdb/nat/windows-nat.h  |  20 ++++
 gdb/windows-nat.c      | 218 ++++++-----------------------------------
 gdbserver/ChangeLog    |   8 ++
 gdbserver/win32-low.cc | 132 ++++---------------------
 6 files changed, 266 insertions(+), 304 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 80a1583b884..6bbf41c7b13 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -18,6 +18,10 @@
 
 #include "gdbsupport/common-defs.h"
 #include "nat/windows-nat.h"
+#include "gdbsupport/common-debug.h"
+
+#define STATUS_WX86_BREAKPOINT 0x4000001F
+#define STATUS_WX86_SINGLE_STEP 0x4000001E
 
 namespace windows_nat
 {
@@ -137,4 +141,175 @@ get_image_name (HANDLE h, void *address, int unicode)
   return buf;
 }
 
+/* The exception thrown by a program to tell the debugger the name of
+   a thread.  The exception record contains an ID of a thread and a
+   name to give it.  This exception has no documented name, but MSDN
+   dubs it "MS_VC_EXCEPTION" in one code example.  */
+#define MS_VC_EXCEPTION 0x406d1388
+
+handle_exception_result
+handle_exception (struct target_waitstatus *ourstatus, bool debug_exceptions)
+{
+#define DEBUG_EXCEPTION_SIMPLE(x)       if (debug_exceptions) \
+  debug_printf ("gdb: Target exception %s at %s\n", x, \
+    host_address_to_string (\
+      current_event.u.Exception.ExceptionRecord.ExceptionAddress))
+
+  EXCEPTION_RECORD *rec = &current_event.u.Exception.ExceptionRecord;
+  DWORD code = rec->ExceptionCode;
+  handle_exception_result result = HANDLE_EXCEPTION_HANDLED;
+
+  memcpy (&siginfo_er, rec, sizeof siginfo_er);
+
+  ourstatus->kind = TARGET_WAITKIND_STOPPED;
+
+  /* Record the context of the current thread.  */
+  thread_rec (ptid_t (current_event.dwProcessId, current_event.dwThreadId, 0),
+	      DONT_SUSPEND);
+
+  switch (code)
+    {
+    case EXCEPTION_ACCESS_VIOLATION:
+      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_ACCESS_VIOLATION");
+      ourstatus->value.sig = GDB_SIGNAL_SEGV;
+#ifdef __CYGWIN__
+      {
+	/* See if the access violation happened within the cygwin DLL
+	   itself.  Cygwin uses a kind of exception handling to deal
+	   with passed-in invalid addresses.  gdb should not treat
+	   these as real SEGVs since they will be silently handled by
+	   cygwin.  A real SEGV will (theoretically) be caught by
+	   cygwin later in the process and will be sent as a
+	   cygwin-specific-signal.  So, ignore SEGVs if they show up
+	   within the text segment of the DLL itself.  */
+	const char *fn;
+	CORE_ADDR addr = (CORE_ADDR) (uintptr_t) rec->ExceptionAddress;
+
+	if ((!cygwin_exceptions && (addr >= cygwin_load_start
+				    && addr < cygwin_load_end))
+	    || (find_pc_partial_function (addr, &fn, NULL, NULL)
+		&& startswith (fn, "KERNEL32!IsBad")))
+	  return HANDLE_EXCEPTION_UNHANDLED;
+      }
+#endif
+      break;
+    case STATUS_STACK_OVERFLOW:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_STACK_OVERFLOW");
+      ourstatus->value.sig = GDB_SIGNAL_SEGV;
+      break;
+    case STATUS_FLOAT_DENORMAL_OPERAND:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_DENORMAL_OPERAND");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case EXCEPTION_ARRAY_BOUNDS_EXCEEDED:
+      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_ARRAY_BOUNDS_EXCEEDED");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case STATUS_FLOAT_INEXACT_RESULT:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_INEXACT_RESULT");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case STATUS_FLOAT_INVALID_OPERATION:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_INVALID_OPERATION");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case STATUS_FLOAT_OVERFLOW:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_OVERFLOW");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case STATUS_FLOAT_STACK_CHECK:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_STACK_CHECK");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case STATUS_FLOAT_UNDERFLOW:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_UNDERFLOW");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case STATUS_FLOAT_DIVIDE_BY_ZERO:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_DIVIDE_BY_ZERO");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case STATUS_INTEGER_DIVIDE_BY_ZERO:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_INTEGER_DIVIDE_BY_ZERO");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case STATUS_INTEGER_OVERFLOW:
+      DEBUG_EXCEPTION_SIMPLE ("STATUS_INTEGER_OVERFLOW");
+      ourstatus->value.sig = GDB_SIGNAL_FPE;
+      break;
+    case EXCEPTION_BREAKPOINT:
+#ifdef __x86_64__
+      if (ignore_first_breakpoint)
+	{
+	  /* For WOW64 processes, there are always 2 breakpoint exceptions
+	     on startup, first a BREAKPOINT for the 64bit ntdll.dll,
+	     then a WX86_BREAKPOINT for the 32bit ntdll.dll.
+	     Here we only care about the WX86_BREAKPOINT's.  */
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+	  ignore_first_breakpoint = false;
+	}
+#endif
+      /* FALLTHROUGH */
+    case STATUS_WX86_BREAKPOINT:
+      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_BREAKPOINT");
+      ourstatus->value.sig = GDB_SIGNAL_TRAP;
+#ifdef _WIN32_WCE
+      /* Remove the initial breakpoint.  */
+      check_breakpoints ((CORE_ADDR) (long) current_event
+			 .u.Exception.ExceptionRecord.ExceptionAddress);
+#endif
+      break;
+    case DBG_CONTROL_C:
+      DEBUG_EXCEPTION_SIMPLE ("DBG_CONTROL_C");
+      ourstatus->value.sig = GDB_SIGNAL_INT;
+      break;
+    case DBG_CONTROL_BREAK:
+      DEBUG_EXCEPTION_SIMPLE ("DBG_CONTROL_BREAK");
+      ourstatus->value.sig = GDB_SIGNAL_INT;
+      break;
+    case EXCEPTION_SINGLE_STEP:
+    case STATUS_WX86_SINGLE_STEP:
+      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_SINGLE_STEP");
+      ourstatus->value.sig = GDB_SIGNAL_TRAP;
+      break;
+    case EXCEPTION_ILLEGAL_INSTRUCTION:
+      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_ILLEGAL_INSTRUCTION");
+      ourstatus->value.sig = GDB_SIGNAL_ILL;
+      break;
+    case EXCEPTION_PRIV_INSTRUCTION:
+      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_PRIV_INSTRUCTION");
+      ourstatus->value.sig = GDB_SIGNAL_ILL;
+      break;
+    case EXCEPTION_NONCONTINUABLE_EXCEPTION:
+      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_NONCONTINUABLE_EXCEPTION");
+      ourstatus->value.sig = GDB_SIGNAL_ILL;
+      break;
+    case MS_VC_EXCEPTION:
+      DEBUG_EXCEPTION_SIMPLE ("MS_VC_EXCEPTION");
+      if (handle_ms_vc_exception (rec))
+	{
+	  ourstatus->value.sig = GDB_SIGNAL_TRAP;
+	  result = HANDLE_EXCEPTION_IGNORED;
+	  break;
+	}
+	/* treat improperly formed exception as unknown */
+	/* FALLTHROUGH */
+    default:
+      /* Treat unhandled first chance exceptions specially.  */
+      if (current_event.u.Exception.dwFirstChance)
+	return HANDLE_EXCEPTION_UNHANDLED;
+      debug_printf ("gdb: unknown target exception 0x%08x at %s\n",
+	(unsigned) current_event.u.Exception.ExceptionRecord.ExceptionCode,
+	host_address_to_string (
+	  current_event.u.Exception.ExceptionRecord.ExceptionAddress));
+      ourstatus->value.sig = GDB_SIGNAL_UNKNOWN;
+      break;
+    }
+
+  last_sig = ourstatus->value.sig;
+  return result;
+
+#undef DEBUG_EXCEPTION_SIMPLE
+}
+
 }
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 2b2fd116269..a4e0b39fcab 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -143,6 +143,16 @@ extern void handle_load_dll ();
 
 extern void handle_unload_dll ();
 
+/* Handle MS_VC_EXCEPTION when processing a stop.  MS_VC_EXCEPTION is
+   somewhat undocumented but is used to tell the debugger the name of
+   a thread.
+
+   Return true if the exception was handled; return false otherwise.
+
+   This function must be supplied by the embedding application.  */
+
+extern bool handle_ms_vc_exception (const EXCEPTION_RECORD *rec);
+
 
 /* Currently executing process */
 extern HANDLE current_process_handle;
@@ -205,6 +215,16 @@ extern EXCEPTION_RECORD siginfo_er;
    get_image_name.  */
 extern const char *get_image_name (HANDLE h, void *address, int unicode);
 
+typedef enum
+{
+  HANDLE_EXCEPTION_UNHANDLED = 0,
+  HANDLE_EXCEPTION_HANDLED,
+  HANDLE_EXCEPTION_IGNORED
+} handle_exception_result;
+
+extern handle_exception_result handle_exception
+  (struct target_waitstatus *ourstatus, bool debug_exceptions);
+
 }
 
 #endif
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3dc31ebd452..a9656a8c91b 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -72,9 +72,6 @@
 #include "gdbsupport/gdb_wait.h"
 #include "nat/windows-nat.h"
 
-#define STATUS_WX86_BREAKPOINT 0x4000001F
-#define STATUS_WX86_SINGLE_STEP 0x4000001E
-
 using namespace windows_nat;
 
 #define AdjustTokenPrivileges		dyn_AdjustTokenPrivileges
@@ -213,19 +210,6 @@ static int debug_registers_used;
 static int windows_initialization_done;
 #define DR6_CLEAR_VALUE 0xffff0ff0
 
-/* The exception thrown by a program to tell the debugger the name of
-   a thread.  The exception record contains an ID of a thread and a
-   name to give it.  This exception has no documented name, but MSDN
-   dubs it "MS_VC_EXCEPTION" in one code example.  */
-#define MS_VC_EXCEPTION 0x406d1388
-
-typedef enum
-{
-  HANDLE_EXCEPTION_UNHANDLED = 0,
-  HANDLE_EXCEPTION_HANDLED,
-  HANDLE_EXCEPTION_IGNORED
-} handle_exception_result;
-
 /* The string sent by cygwin when it processes a signal.
    FIXME: This should be in a cygwin include file.  */
 #ifndef _CYGWIN_SIGNAL_STRING
@@ -1203,189 +1187,45 @@ display_selectors (const char * args, int from_tty)
     }
 }
 
-#define DEBUG_EXCEPTION_SIMPLE(x)       if (debug_exceptions) \
-  printf_unfiltered ("gdb: Target exception %s at %s\n", x, \
-    host_address_to_string (\
-      current_event.u.Exception.ExceptionRecord.ExceptionAddress))
+/* See nat/windows-nat.h.  */
 
-static handle_exception_result
-handle_exception (struct target_waitstatus *ourstatus)
+bool
+windows_nat::handle_ms_vc_exception (const EXCEPTION_RECORD *rec)
 {
-  EXCEPTION_RECORD *rec = &current_event.u.Exception.ExceptionRecord;
-  DWORD code = rec->ExceptionCode;
-  handle_exception_result result = HANDLE_EXCEPTION_HANDLED;
-
-  memcpy (&siginfo_er, rec, sizeof siginfo_er);
-
-  ourstatus->kind = TARGET_WAITKIND_STOPPED;
-
-  /* Record the context of the current thread.  */
-  thread_rec (ptid_t (current_event.dwProcessId, current_event.dwThreadId, 0),
-	      DONT_SUSPEND);
-
-  switch (code)
+  if (rec->NumberParameters >= 3
+      && (rec->ExceptionInformation[0] & 0xffffffff) == 0x1000)
     {
-    case EXCEPTION_ACCESS_VIOLATION:
-      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_ACCESS_VIOLATION");
-      ourstatus->value.sig = GDB_SIGNAL_SEGV;
-#ifdef __CYGWIN__
-      {
-	/* See if the access violation happened within the cygwin DLL
-	   itself.  Cygwin uses a kind of exception handling to deal
-	   with passed-in invalid addresses.  gdb should not treat
-	   these as real SEGVs since they will be silently handled by
-	   cygwin.  A real SEGV will (theoretically) be caught by
-	   cygwin later in the process and will be sent as a
-	   cygwin-specific-signal.  So, ignore SEGVs if they show up
-	   within the text segment of the DLL itself.  */
-	const char *fn;
-	CORE_ADDR addr = (CORE_ADDR) (uintptr_t) rec->ExceptionAddress;
-
-	if ((!cygwin_exceptions && (addr >= cygwin_load_start
-				    && addr < cygwin_load_end))
-	    || (find_pc_partial_function (addr, &fn, NULL, NULL)
-		&& startswith (fn, "KERNEL32!IsBad")))
-	  return HANDLE_EXCEPTION_UNHANDLED;
-      }
-#endif
-      break;
-    case STATUS_STACK_OVERFLOW:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_STACK_OVERFLOW");
-      ourstatus->value.sig = GDB_SIGNAL_SEGV;
-      break;
-    case STATUS_FLOAT_DENORMAL_OPERAND:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_DENORMAL_OPERAND");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case EXCEPTION_ARRAY_BOUNDS_EXCEEDED:
-      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_ARRAY_BOUNDS_EXCEEDED");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_INEXACT_RESULT:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_INEXACT_RESULT");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_INVALID_OPERATION:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_INVALID_OPERATION");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_OVERFLOW:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_OVERFLOW");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_STACK_CHECK:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_STACK_CHECK");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_UNDERFLOW:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_UNDERFLOW");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_DIVIDE_BY_ZERO:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_FLOAT_DIVIDE_BY_ZERO");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_INTEGER_DIVIDE_BY_ZERO:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_INTEGER_DIVIDE_BY_ZERO");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_INTEGER_OVERFLOW:
-      DEBUG_EXCEPTION_SIMPLE ("STATUS_INTEGER_OVERFLOW");
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case EXCEPTION_BREAKPOINT:
-#ifdef __x86_64__
-      if (ignore_first_breakpoint)
-	{
-	  /* For WOW64 processes, there are always 2 breakpoint exceptions
-	     on startup, first a BREAKPOINT for the 64bit ntdll.dll,
-	     then a WX86_BREAKPOINT for the 32bit ntdll.dll.
-	     Here we only care about the WX86_BREAKPOINT's.  */
-	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
-	  ignore_first_breakpoint = false;
-	}
-#endif
-      /* FALLTHROUGH */
-    case STATUS_WX86_BREAKPOINT:
-      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_BREAKPOINT");
-      ourstatus->value.sig = GDB_SIGNAL_TRAP;
-      break;
-    case DBG_CONTROL_C:
-      DEBUG_EXCEPTION_SIMPLE ("DBG_CONTROL_C");
-      ourstatus->value.sig = GDB_SIGNAL_INT;
-      break;
-    case DBG_CONTROL_BREAK:
-      DEBUG_EXCEPTION_SIMPLE ("DBG_CONTROL_BREAK");
-      ourstatus->value.sig = GDB_SIGNAL_INT;
-      break;
-    case EXCEPTION_SINGLE_STEP:
-    case STATUS_WX86_SINGLE_STEP:
-      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_SINGLE_STEP");
-      ourstatus->value.sig = GDB_SIGNAL_TRAP;
-      break;
-    case EXCEPTION_ILLEGAL_INSTRUCTION:
-      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_ILLEGAL_INSTRUCTION");
-      ourstatus->value.sig = GDB_SIGNAL_ILL;
-      break;
-    case EXCEPTION_PRIV_INSTRUCTION:
-      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_PRIV_INSTRUCTION");
-      ourstatus->value.sig = GDB_SIGNAL_ILL;
-      break;
-    case EXCEPTION_NONCONTINUABLE_EXCEPTION:
-      DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_NONCONTINUABLE_EXCEPTION");
-      ourstatus->value.sig = GDB_SIGNAL_ILL;
-      break;
-    case MS_VC_EXCEPTION:
-      if (rec->NumberParameters >= 3
-	  && (rec->ExceptionInformation[0] & 0xffffffff) == 0x1000)
-	{
-	  DWORD named_thread_id;
-	  windows_thread_info *named_thread;
-	  CORE_ADDR thread_name_target;
+      DWORD named_thread_id;
+      windows_thread_info *named_thread;
+      CORE_ADDR thread_name_target;
 
-	  DEBUG_EXCEPTION_SIMPLE ("MS_VC_EXCEPTION");
+      thread_name_target = rec->ExceptionInformation[1];
+      named_thread_id = (DWORD) (0xffffffff & rec->ExceptionInformation[2]);
 
-	  thread_name_target = rec->ExceptionInformation[1];
-	  named_thread_id = (DWORD) (0xffffffff & rec->ExceptionInformation[2]);
+      if (named_thread_id == (DWORD) -1)
+	named_thread_id = current_event.dwThreadId;
 
-	  if (named_thread_id == (DWORD) -1)
-	    named_thread_id = current_event.dwThreadId;
+      named_thread = thread_rec (ptid_t (current_event.dwProcessId,
+					 named_thread_id, 0),
+				 DONT_INVALIDATE_CONTEXT);
+      if (named_thread != NULL)
+	{
+	  int thread_name_len;
+	  gdb::unique_xmalloc_ptr<char> thread_name;
 
-	  named_thread = thread_rec (ptid_t (current_event.dwProcessId,
-					     named_thread_id, 0),
-				     DONT_INVALIDATE_CONTEXT);
-	  if (named_thread != NULL)
+	  thread_name_len = target_read_string (thread_name_target,
+						&thread_name, 1025, NULL);
+	  if (thread_name_len > 0)
 	    {
-	      int thread_name_len;
-	      gdb::unique_xmalloc_ptr<char> thread_name;
-
-	      thread_name_len = target_read_string (thread_name_target,
-						    &thread_name, 1025, NULL);
-	      if (thread_name_len > 0)
-		{
-		  thread_name.get ()[thread_name_len - 1] = '\0';
-		  named_thread->name = std::move (thread_name);
-		}
+	      thread_name.get ()[thread_name_len - 1] = '\0';
+	      named_thread->name = std::move (thread_name);
 	    }
-	  ourstatus->value.sig = GDB_SIGNAL_TRAP;
-	  result = HANDLE_EXCEPTION_IGNORED;
-	  break;
 	}
-	/* treat improperly formed exception as unknown */
-	/* FALLTHROUGH */
-    default:
-      /* Treat unhandled first chance exceptions specially.  */
-      if (current_event.u.Exception.dwFirstChance)
-	return HANDLE_EXCEPTION_UNHANDLED;
-      printf_unfiltered ("gdb: unknown target exception 0x%08x at %s\n",
-	(unsigned) current_event.u.Exception.ExceptionRecord.ExceptionCode,
-	host_address_to_string (
-	  current_event.u.Exception.ExceptionRecord.ExceptionAddress));
-      ourstatus->value.sig = GDB_SIGNAL_UNKNOWN;
-      break;
+
+      return true;
     }
-  last_sig = ourstatus->value.sig;
-  return result;
+
+  return false;
 }
 
 /* Resume thread specified by ID, or all artificially suspended
@@ -1876,7 +1716,7 @@ windows_nat_target::get_windows_debug_event (int pid,
 		     "EXCEPTION_DEBUG_EVENT"));
       if (saw_create != 1)
 	break;
-      switch (handle_exception (ourstatus))
+      switch (handle_exception (ourstatus, debug_exceptions))
 	{
 	case HANDLE_EXCEPTION_UNHANDLED:
 	default:
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 73d4a6a2d8a..7018083746b 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -1154,117 +1154,6 @@ windows_nat::handle_unload_dll ()
   unloaded_dll (NULL, load_addr);
 }
 
-static void
-handle_exception (struct target_waitstatus *ourstatus)
-{
-  DWORD code = current_event.u.Exception.ExceptionRecord.ExceptionCode;
-
-  memcpy (&siginfo_er, &current_event.u.Exception.ExceptionRecord,
-	  sizeof siginfo_er);
-
-  ourstatus->kind = TARGET_WAITKIND_STOPPED;
-
-  switch (code)
-    {
-    case EXCEPTION_ACCESS_VIOLATION:
-      OUTMSG2 (("EXCEPTION_ACCESS_VIOLATION"));
-      ourstatus->value.sig = GDB_SIGNAL_SEGV;
-      break;
-    case STATUS_STACK_OVERFLOW:
-      OUTMSG2 (("STATUS_STACK_OVERFLOW"));
-      ourstatus->value.sig = GDB_SIGNAL_SEGV;
-      break;
-    case STATUS_FLOAT_DENORMAL_OPERAND:
-      OUTMSG2 (("STATUS_FLOAT_DENORMAL_OPERAND"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case EXCEPTION_ARRAY_BOUNDS_EXCEEDED:
-      OUTMSG2 (("EXCEPTION_ARRAY_BOUNDS_EXCEEDED"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_INEXACT_RESULT:
-      OUTMSG2 (("STATUS_FLOAT_INEXACT_RESULT"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_INVALID_OPERATION:
-      OUTMSG2 (("STATUS_FLOAT_INVALID_OPERATION"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_OVERFLOW:
-      OUTMSG2 (("STATUS_FLOAT_OVERFLOW"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_STACK_CHECK:
-      OUTMSG2 (("STATUS_FLOAT_STACK_CHECK"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_UNDERFLOW:
-      OUTMSG2 (("STATUS_FLOAT_UNDERFLOW"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_FLOAT_DIVIDE_BY_ZERO:
-      OUTMSG2 (("STATUS_FLOAT_DIVIDE_BY_ZERO"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_INTEGER_DIVIDE_BY_ZERO:
-      OUTMSG2 (("STATUS_INTEGER_DIVIDE_BY_ZERO"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case STATUS_INTEGER_OVERFLOW:
-      OUTMSG2 (("STATUS_INTEGER_OVERFLOW"));
-      ourstatus->value.sig = GDB_SIGNAL_FPE;
-      break;
-    case EXCEPTION_BREAKPOINT:
-      OUTMSG2 (("EXCEPTION_BREAKPOINT"));
-      ourstatus->value.sig = GDB_SIGNAL_TRAP;
-#ifdef _WIN32_WCE
-      /* Remove the initial breakpoint.  */
-      check_breakpoints ((CORE_ADDR) (long) current_event
-			 .u.Exception.ExceptionRecord.ExceptionAddress);
-#endif
-      break;
-    case DBG_CONTROL_C:
-      OUTMSG2 (("DBG_CONTROL_C"));
-      ourstatus->value.sig = GDB_SIGNAL_INT;
-      break;
-    case DBG_CONTROL_BREAK:
-      OUTMSG2 (("DBG_CONTROL_BREAK"));
-      ourstatus->value.sig = GDB_SIGNAL_INT;
-      break;
-    case EXCEPTION_SINGLE_STEP:
-      OUTMSG2 (("EXCEPTION_SINGLE_STEP"));
-      ourstatus->value.sig = GDB_SIGNAL_TRAP;
-      break;
-    case EXCEPTION_ILLEGAL_INSTRUCTION:
-      OUTMSG2 (("EXCEPTION_ILLEGAL_INSTRUCTION"));
-      ourstatus->value.sig = GDB_SIGNAL_ILL;
-      break;
-    case EXCEPTION_PRIV_INSTRUCTION:
-      OUTMSG2 (("EXCEPTION_PRIV_INSTRUCTION"));
-      ourstatus->value.sig = GDB_SIGNAL_ILL;
-      break;
-    case EXCEPTION_NONCONTINUABLE_EXCEPTION:
-      OUTMSG2 (("EXCEPTION_NONCONTINUABLE_EXCEPTION"));
-      ourstatus->value.sig = GDB_SIGNAL_ILL;
-      break;
-    default:
-      if (current_event.u.Exception.dwFirstChance)
-	{
-	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
-	  return;
-	}
-      OUTMSG2 (("gdbserver: unknown target exception 0x%08x at 0x%s",
-	    (unsigned) current_event.u.Exception.ExceptionRecord.ExceptionCode,
-	    phex_nz ((uintptr_t) current_event.u.Exception.ExceptionRecord.
-	    ExceptionAddress, sizeof (uintptr_t))));
-      ourstatus->value.sig = GDB_SIGNAL_UNKNOWN;
-      break;
-    }
-  OUTMSG2 (("\n"));
-  last_sig = ourstatus->value.sig;
-}
-
-
 static void
 suspend_one_thread (thread_info *thread)
 {
@@ -1297,15 +1186,25 @@ auto_delete_breakpoint (CORE_ADDR stop_pc)
 }
 #endif
 
+/* See nat/windows-nat.h.  */
+
+bool
+windows_nat::handle_ms_vc_exception (const EXCEPTION_RECORD *rec)
+{
+  return false;
+}
+
 /* Get the next event from the child.  */
 
 static int
-get_child_debug_event (struct target_waitstatus *ourstatus)
+get_child_debug_event (DWORD *continue_status,
+		       struct target_waitstatus *ourstatus)
 {
   ptid_t ptid;
 
   last_sig = GDB_SIGNAL_0;
   ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+  *continue_status = DBG_CONTINUE;
 
   /* Check if GDB sent us an interrupt request.  */
   check_remote_input_interrupt_request ();
@@ -1488,7 +1387,9 @@ get_child_debug_event (struct target_waitstatus *ourstatus)
 		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
-      handle_exception (ourstatus);
+      if (handle_exception (ourstatus, debug_threads)
+	  == HANDLE_EXCEPTION_UNHANDLED)
+	*continue_status = DBG_EXCEPTION_NOT_HANDLED;
       break;
 
     case OUTPUT_DEBUG_STRING_EVENT:
@@ -1536,7 +1437,8 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
 
   while (1)
     {
-      if (!get_child_debug_event (ourstatus))
+      DWORD continue_status;
+      if (!get_child_debug_event (&continue_status, ourstatus))
 	continue;
 
       switch (ourstatus->kind)
@@ -1560,7 +1462,7 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
 	  /* fall-through */
 	case TARGET_WAITKIND_SPURIOUS:
 	  /* do nothing, just continue */
-	  child_continue (DBG_CONTINUE, -1);
+	  child_continue (continue_status, -1);
 	  break;
 	}
     }
-- 
2.21.1

Re: [PATCH v3 21/29] Share handle_exception

[Simon Marchi]

On 2020-03-13 3:08 p.m., Tom Tromey wrote:
> Both gdb and gdbserver have a "handle_exception" function, the bulk of
> which is shared between the two implementations.  This patch arranges
> for the entire thing to be moved into nat/windows-nat.c, with the
> differences handled by callbacks.  This patch introduces one more
> callback to make this possible.
> 
> gdb/ChangeLog
> 2020-03-13  Tom Tromey  <tromey@adacore.com>
> 
> 	* windows-nat.c (MS_VC_EXCEPTION): Move to nat/windows-nat.c.
> 	(handle_exception_result): Move to nat/windows-nat.h.
> 	(DEBUG_EXCEPTION_SIMPLE): Remove.
> 	(windows_nat::handle_ms_vc_exception): New function.
> 	(handle_exception): Move to nat/windows-nat.c.
> 	(get_windows_debug_event): Update.
> 	(STATUS_WX86_BREAKPOINT, STATUS_WX86_SINGLE_STEP): Move to
> 	nat/windows-nat.c.
> 	* nat/windows-nat.h (handle_ms_vc_exception): Declare.
> 	(handle_exception_result): Move from windows-nat.c.
> 	(handle_exception): Declare.
> 	* nat/windows-nat.c (MS_VC_EXCEPTION, handle_exception)
> 	(STATUS_WX86_SINGLE_STEP, STATUS_WX86_BREAKPOINT): Move from
> 	windows-nat.c.
This patch introduced some GDB-specific calls in nat/windows-nat.c.  That
file is built by gdbserver, so it breaks that build.  This is when building
on Cygwin:

  CXX    nat/windows-nat.o
/home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/windows-nat.c: In function ‘windows_nat::handle_exception_result windows_nat::handle_exception(target_waitstatus*, bool)’:
/home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/windows-nat.c:200:8: error: ‘cygwin_exceptions’ was not declared in this scope
  200 |  if ((!cygwin_exceptions && (addr >= cygwin_load_start
      |        ^~~~~~~~~~~~~~~~~
/home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/windows-nat.c:200:38: error: ‘cygwin_load_start’ was not declared in this scope
  200 |  if ((!cygwin_exceptions && (addr >= cygwin_load_start
      |                                      ^~~~~~~~~~~~~~~~~
/home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/windows-nat.c:201:19: error: ‘cygwin_load_end’ was not declared in this scope
  201 |         && addr < cygwin_load_end))
      |                   ^~~~~~~~~~~~~~~
/home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/windows-nat.c:202:10: error: ‘find_pc_partial_function’ was not declared in this scope
  202 |      || (find_pc_partial_function (addr, &fn, NULL, NULL)
      |          ^~~~~~~~~~~~~~~~~~~~~~~~

Simon

Re: [PATCH v3 21/29] Share handle_exception

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> This patch introduced some GDB-specific calls in nat/windows-nat.c.  That
Simon> file is built by gdbserver, so it breaks that build.  This is when building
Simon> on Cygwin:

For reasons I don't really understand, I can't seem to do a Cygwin
build.  (And, I have little control over the Windows machines I do have
access to, so I can't really fix the problem, whatever it is.)

So, could you try this patch?

Tom

commit f4258cb31d8f39bb3779f31f55bab3de55556497
Author: Tom Tromey <tromey@adacore.com>
Date:   Wed Apr 15 10:53:04 2020 -0600

    Fix Cygwin gdb build
    
    Simon pointed out that the windows-nat sharing series broke the Cygwin
    build.  This patch fixes the problem, by moving some globals into
    nat/windows-nat.c.
    
    gdb/ChangeLog
    2020-04-15  Tom Tromey  <tromey@adacore.com>
    
            * windows-nat.c (cygwin_exceptions, cygwin_load_start)
            (cygwin_load_end): Move to nat/windows-nat.c.
            * nat/windows-nat.h (cygwin_exceptions, cygwin_load_start)
            (cygwin_load_end): Move definitions from windows-nat.c.
            * nat/windows-nat.c (cygwin_exceptions, cygwin_load_start)
            (cygwin_load_end): Declare.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 425c4459922..c82d8e7ff40 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2020-04-15  Tom Tromey  <tromey@adacore.com>
+
+	* windows-nat.c (cygwin_exceptions, cygwin_load_start)
+	(cygwin_load_end): Move to nat/windows-nat.c.
+	* nat/windows-nat.h (cygwin_exceptions, cygwin_load_start)
+	(cygwin_load_end): Move definitions from windows-nat.c.
+	* nat/windows-nat.c (cygwin_exceptions, cygwin_load_start)
+	(cygwin_load_end): Declare.
+
 2016-01-20  Jon Turney  <jon.turney@dronecode.org.uk>
 
 	* windows-nat.c (windows_make_so): Warn rather than stopping with
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index cd7c1d177c6..a5e367496e8 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -45,6 +45,18 @@ EXCEPTION_RECORD siginfo_er;
 bool ignore_first_breakpoint = false;
 #endif
 
+#ifdef __CYGWIN__
+
+/* When true, break when an exception is detected in the Cygwin DLL
+   itself.  */
+bool cygwin_exceptions = false;
+
+/* The starting and ending address of the cygwin1.dll text segment.  */
+CORE_ADDR cygwin_load_start;
+CORE_ADDR cygwin_load_end;
+
+#endif /* __CYGWIN__ */
+
 /* Note that 'debug_events' must be locally defined in the relevant
    functions.  */
 #define DEBUG_EVENTS(x)	if (debug_events) debug_printf x
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index aea1519672d..0c103cb867f 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -210,6 +210,18 @@ extern EXCEPTION_RECORD siginfo_er;
 extern bool ignore_first_breakpoint;
 #endif
 
+#ifdef __CYGWIN__
+
+/* When true, break when an exception is detected in the Cygwin DLL
+   itself.  */
+extern bool cygwin_exceptions;
+
+/* The starting and ending address of the cygwin1.dll text segment.  */
+extern CORE_ADDR cygwin_load_start;
+extern CORE_ADDR cygwin_load_end;
+
+#endif /* __CYGWIN__ */
+
 /* Return the name of the DLL referenced by H at ADDRESS.  UNICODE
    determines what sort of string is read from the inferior.  Returns
    the name of the DLL, or NULL on error.  If a name is returned, it
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 613153bfac6..a4e15ee7a32 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -162,9 +162,6 @@ static Wow64GetThreadSelectorEntry_ftype *Wow64GetThreadSelectorEntry;
 # define bad_GetModuleFileNameEx bad_GetModuleFileNameExA
 #else
 # define __PMAX	PATH_MAX
-/* The starting and ending address of the cygwin1.dll text segment.  */
-  static CORE_ADDR cygwin_load_start;
-  static CORE_ADDR cygwin_load_end;
 #   define __USEWIDE
     typedef wchar_t cygwin_buf_t;
     typedef DWORD WINAPI (GetModuleFileNameEx_ftype) (HANDLE, HMODULE,
@@ -239,9 +236,6 @@ static bool wow64_process = false;
 
 /* User options.  */
 static bool new_console = false;
-#ifdef __CYGWIN__
-static bool cygwin_exceptions = false;
-#endif
 static bool new_group = true;
 static bool debug_exec = false;		/* show execution */
 static bool debug_events = false;	/* show events from kernel */

Re: [PATCH v3 21/29] Share handle_exception

[Simon Marchi]

On 2020-04-15 12:54 p.m., Tom Tromey wrote:
> Simon> This patch introduced some GDB-specific calls in nat/windows-nat.c.  That
> Simon> file is built by gdbserver, so it breaks that build.  This is when building
> Simon> on Cygwin:
> 
> For reasons I don't really understand, I can't seem to do a Cygwin
> build.  (And, I have little control over the Windows machines I do have
> access to, so I can't really fix the problem, whatever it is.)
> 
> So, could you try this patch?
> 
> Tom

It helps, but there's still:

  CXX    nat/windows-nat.o
/home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/windows-nat.c: In function ‘windows_nat::handle_exception_result windows_nat::handle_exception(target_waitstatus*, bool)’:
/home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/windows-nat.c:214:10: error: ‘find_pc_partial_function’ was not declared in this scope
  214 |      || (find_pc_partial_function (addr, &fn, NULL, NULL)
      |          ^~~~~~~~~~~~~~~~~~~~~~~~


But even if that did build, I don't think it's right.  That code will be used by gdbserver, which
will never initialize cygwin_load_start and cygwin_load_end.

If we look at gdbserver's version of the function before this change, it didn't have that part,
so perhaps it should be factored out into a callback, which will reside in gdb/windows-nat.c.

Simon

Re: [PATCH v3 21/29] Share handle_exception

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> It helps, but there's still:

Simon>   CXX    nat/windows-nat.o
Simon> /home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/windows-nat.c: In function ‘windows_nat::handle_exception_result windows_nat::handle_exception(target_waitstatus*, bool)’:
Simon> /home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/windows-nat.c:214:10: error: ‘find_pc_partial_function’ was not declared in this scope
Simon>   214 |      || (find_pc_partial_function (addr, &fn, NULL, NULL)
Simon>       |          ^~~~~~~~~~~~~~~~~~~~~~~~

Yeah, I realized this quite a while after sending it.

Simon> If we look at gdbserver's version of the function before this change, it didn't have that part,
Simon> so perhaps it should be factored out into a callback, which will reside in gdb/windows-nat.c.

Yep, please try this.

Tom

commit 3cb8f05b1e897a617e499edc54c9fc5cf456634c
Author: Tom Tromey <tromey@adacore.com>
Date:   Wed Apr 15 13:11:23 2020 -0600

    Fix Cygwin gdb build
    
    Simon pointed out that the windows-nat sharing series broke the Cygwin
    build.  This patch fixes the problem, by moving the Cygwin-specific
    code to a new handler function.  This approach is taken because this
    code calls find_pc_partial_function, which isn't available in
    gdbserver.
    
    gdb/ChangeLog
    2020-04-15  Tom Tromey  <tromey@adacore.com>
    
            * windows-nat.c (windows_nat::handle_access_violation): New
            function.
            * nat/windows-nat.h (handle_access_violation): Declare.
            * nat/windows-nat.c (handle_exception): Move Cygwin code to
            windows-nat.c.  Call handle_access_violation.
    
    gdbserver/ChangeLog
    2020-04-15  Tom Tromey  <tromey@adacore.com>
    
            * win32-low.cc (windows_nat::handle_access_violation): New
            function.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b0a535a624f..f6a15d1482c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2020-04-15  Tom Tromey  <tromey@adacore.com>
+
+	* windows-nat.c (windows_nat::handle_access_violation): New
+	function.
+	* nat/windows-nat.h (handle_access_violation): Declare.
+	* nat/windows-nat.c (handle_exception): Move Cygwin code to
+	windows-nat.c.  Call handle_access_violation.
+
 2020-04-15  Tom Tromey  <tromey@adacore.com>
 
 	* windows-nat.c (DEBUG_EXEC, DEBUG_EVENTS, DEBUG_MEM)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index cd7c1d177c6..8c2092a51d7 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -184,26 +184,8 @@ handle_exception (struct target_waitstatus *ourstatus, bool debug_exceptions)
     case EXCEPTION_ACCESS_VIOLATION:
       DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_ACCESS_VIOLATION");
       ourstatus->value.sig = GDB_SIGNAL_SEGV;
-#ifdef __CYGWIN__
-      {
-	/* See if the access violation happened within the cygwin DLL
-	   itself.  Cygwin uses a kind of exception handling to deal
-	   with passed-in invalid addresses.  gdb should not treat
-	   these as real SEGVs since they will be silently handled by
-	   cygwin.  A real SEGV will (theoretically) be caught by
-	   cygwin later in the process and will be sent as a
-	   cygwin-specific-signal.  So, ignore SEGVs if they show up
-	   within the text segment of the DLL itself.  */
-	const char *fn;
-	CORE_ADDR addr = (CORE_ADDR) (uintptr_t) rec->ExceptionAddress;
-
-	if ((!cygwin_exceptions && (addr >= cygwin_load_start
-				    && addr < cygwin_load_end))
-	    || (find_pc_partial_function (addr, &fn, NULL, NULL)
-		&& startswith (fn, "KERNEL32!IsBad")))
-	  return HANDLE_EXCEPTION_UNHANDLED;
-      }
-#endif
+      if (handle_access_violation (rec))
+	return HANDLE_EXCEPTION_UNHANDLED;
       break;
     case STATUS_STACK_OVERFLOW:
       DEBUG_EXCEPTION_SIMPLE ("STATUS_STACK_OVERFLOW");
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index aea1519672d..8d0fa9bd216 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -157,6 +157,13 @@ extern void handle_unload_dll ();
 
 extern bool handle_ms_vc_exception (const EXCEPTION_RECORD *rec);
 
+/* When EXCEPTION_ACCESS_VIOLATION is processed, we give the embedding
+   application a chance to change it to be considered "unhandled".
+   This function must be supplied by the embedding application.  If it
+   returns true, then the exception is "unhandled".  */
+
+extern bool handle_access_violation (const EXCEPTION_RECORD *rec);
+
 
 /* Currently executing process */
 extern HANDLE current_process_handle;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index e82e58ec1f2..b857f82eb89 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1230,6 +1230,31 @@ windows_nat::handle_ms_vc_exception (const EXCEPTION_RECORD *rec)
   return false;
 }
 
+/* See nat/windows-nat.h.  */
+
+bool
+windows_nat::handle_access_violation (const EXCEPTION_RECORD *rec)
+{
+#ifdef __CYGWIN__
+  /* See if the access violation happened within the cygwin DLL
+     itself.  Cygwin uses a kind of exception handling to deal with
+     passed-in invalid addresses.  gdb should not treat these as real
+     SEGVs since they will be silently handled by cygwin.  A real SEGV
+     will (theoretically) be caught by cygwin later in the process and
+     will be sent as a cygwin-specific-signal.  So, ignore SEGVs if
+     they show up within the text segment of the DLL itself.  */
+  const char *fn;
+  CORE_ADDR addr = (CORE_ADDR) (uintptr_t) rec->ExceptionAddress;
+
+  if ((!cygwin_exceptions && (addr >= cygwin_load_start
+			      && addr < cygwin_load_end))
+      || (find_pc_partial_function (addr, &fn, NULL, NULL)
+	  && startswith (fn, "KERNEL32!IsBad")))
+    return true;
+#endif
+  return false;
+}
+
 /* Resume thread specified by ID, or all artificially suspended
    threads, if we are continuing execution.  KILLED non-zero means we
    have killed the inferior, so we should ignore weird errors due to
diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index 2b381455ed7..d22b6b2daef 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,8 @@
+2020-04-15  Tom Tromey  <tromey@adacore.com>
+
+	* win32-low.cc (windows_nat::handle_access_violation): New
+	function.
+
 2020-04-13  Tom Tromey  <tom@tromey.com>
 
 	* server.h (gdb_fildes_t): Remove typedef.
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index e1226b4b0db..41348dd77d4 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -1198,6 +1198,14 @@ windows_nat::handle_ms_vc_exception (const EXCEPTION_RECORD *rec)
   return false;
 }
 
+/* See nat/windows-nat.h.  */
+
+bool
+windows_nat::handle_access_violation (const EXCEPTION_RECORD *rec)
+{
+  return false;
+}
+
 /* A helper function that will, if needed, set
    'stopped_at_software_breakpoint' on the thread and adjust the
    PC.  */

Re: [PATCH v3 21/29] Share handle_exception

[Simon Marchi]

On 2020-04-15 3:13 p.m., Tom Tromey wrote:
> Yeah, I realized this quite a while after sending it.
> 
> Simon> If we look at gdbserver's version of the function before this change, it didn't have that part,
> Simon> so perhaps it should be factored out into a callback, which will reside in gdb/windows-nat.c.
> 
> Yep, please try this.
> 
> Tom

Thanks, that looks good.  There's a tiny format string error left, I'll push a patch for it.

Simon

[PATCH v3 22/29] Share some inferior-related Windows code

[Tom Tromey]

This adds a couple of functions to nat/windows-nat.c and changes gdb
and gdbserver to use them.  One function checks the list of pending
stops for a match (not yet used by gdbserver, but will be in a
subsequent patch); and the other is a wrapper for ContinueDebugEvent
that always uses the last "real" stop event.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (windows_continue): Use matching_pending_stop and
	continue_last_debug_event.
	* nat/windows-nat.h (matching_pending_stop)
	(continue_last_debug_event): Declare.
	* nat/windows-nat.c (DEBUG_EVENTS): New define.
	(matching_pending_stop, continue_last_debug_event): New
	functions.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (child_continue): Call continue_last_debug_event.
---
 gdb/ChangeLog          | 10 +++++++++
 gdb/nat/windows-nat.c  | 46 ++++++++++++++++++++++++++++++++++++++++++
 gdb/nat/windows-nat.h  | 13 ++++++++++++
 gdb/windows-nat.c      | 28 +++----------------------
 gdbserver/ChangeLog    |  4 ++++
 gdbserver/win32-low.cc |  7 +------
 6 files changed, 77 insertions(+), 31 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 6bbf41c7b13..2c2454b6f6f 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -37,6 +37,10 @@ DWORD desired_stop_thread_id = -1;
 std::vector<pending_stop> pending_stops;
 EXCEPTION_RECORD siginfo_er;
 
+/* Note that 'debug_events' must be locally defined in the relevant
+   functions.  */
+#define DEBUG_EVENTS(x)	if (debug_events) debug_printf x
+
 windows_thread_info::~windows_thread_info ()
 {
   CloseHandle (h);
@@ -312,4 +316,46 @@ handle_exception (struct target_waitstatus *ourstatus, bool debug_exceptions)
 #undef DEBUG_EXCEPTION_SIMPLE
 }
 
+/* See nat/windows-nat.h.  */
+
+bool
+matching_pending_stop (bool debug_events)
+{
+  /* If there are pending stops, and we might plausibly hit one of
+     them, we don't want to actually continue the inferior -- we just
+     want to report the stop.  In this case, we just pretend to
+     continue.  See the comment by the definition of "pending_stops"
+     for details on why this is needed.  */
+  for (const auto &item : pending_stops)
+    {
+      if (desired_stop_thread_id == -1
+	  || desired_stop_thread_id == item.thread_id)
+	{
+	  DEBUG_EVENTS (("windows_continue - pending stop anticipated, "
+			 "desired=0x%x, item=0x%x\n",
+			 desired_stop_thread_id, item.thread_id));
+	  return true;
+	}
+    }
+
+  return false;
+}
+
+/* See nat/windows-nat.h.  */
+
+BOOL
+continue_last_debug_event (DWORD continue_status, bool debug_events)
+{
+  DEBUG_EVENTS (("ContinueDebugEvent (cpid=%d, ctid=0x%x, %s);\n",
+		  (unsigned) last_wait_event.dwProcessId,
+		  (unsigned) last_wait_event.dwThreadId,
+		  continue_status == DBG_CONTINUE ?
+		  "DBG_CONTINUE" : "DBG_EXCEPTION_NOT_HANDLED"));
+
+  return ContinueDebugEvent (last_wait_event.dwProcessId,
+			     last_wait_event.dwThreadId,
+			     continue_status);
+}
+
+
 }
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index a4e0b39fcab..0e9316577ba 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -225,6 +225,19 @@ typedef enum
 extern handle_exception_result handle_exception
   (struct target_waitstatus *ourstatus, bool debug_exceptions);
 
+/* Return true if there is a pending stop matching
+   desired_stop_thread_id.  If DEBUG_EVENTS is true, logging will be
+   enabled.  */
+
+extern bool matching_pending_stop (bool debug_events);
+
+/* A simple wrapper for ContinueDebugEvent that continues the last
+   waited-for event.  If DEBUG_EVENTS is true, logging will be
+   enabled.  */
+
+extern BOOL continue_last_debug_event (DWORD continue_status,
+				       bool debug_events);
+
 }
 
 #endif
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a9656a8c91b..67655dc46e4 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1239,28 +1239,8 @@ windows_continue (DWORD continue_status, int id, int killed)
 
   desired_stop_thread_id = id;
 
-  /* If there are pending stops, and we might plausibly hit one of
-     them, we don't want to actually continue the inferior -- we just
-     want to report the stop.  In this case, we just pretend to
-     continue.  See the comment by the definition of "pending_stops"
-     for details on why this is needed.  */
-  for (const auto &item : pending_stops)
-    {
-      if (desired_stop_thread_id == -1
-	  || desired_stop_thread_id == item.thread_id)
-	{
-	  DEBUG_EVENTS (("windows_continue - pending stop anticipated, "
-			 "desired=0x%x, item=0x%x\n",
-			 desired_stop_thread_id, item.thread_id));
-	  return TRUE;
-	}
-    }
-
-  DEBUG_EVENTS (("ContinueDebugEvent (cpid=%d, ctid=0x%x, %s);\n",
-		  (unsigned) last_wait_event.dwProcessId,
-		  (unsigned) last_wait_event.dwThreadId,
-		  continue_status == DBG_CONTINUE ?
-		  "DBG_CONTINUE" : "DBG_EXCEPTION_NOT_HANDLED"));
+  if (matching_pending_stop (debug_events))
+    return TRUE;
 
   for (windows_thread_info *th : thread_list)
     if (id == -1 || id == (int) th->tid)
@@ -1333,9 +1313,7 @@ windows_continue (DWORD continue_status, int id, int killed)
 	th->suspend ();
       }
 
-  res = ContinueDebugEvent (last_wait_event.dwProcessId,
-			    last_wait_event.dwThreadId,
-			    continue_status);
+  res = continue_last_debug_event (continue_status, debug_events);
 
   if (!res)
     error (_("Failed to resume program execution"
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 7018083746b..33f64700153 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -433,12 +433,7 @@ child_continue (DWORD continue_status, int thread_id)
     });
   faked_breakpoint = 0;
 
-  if (!ContinueDebugEvent (current_event.dwProcessId,
-			   current_event.dwThreadId,
-			   continue_status))
-    return FALSE;
-
-  return TRUE;
+  return continue_last_debug_event (continue_status, debug_threads);
 }
 
 /* Fetch register(s) from the current thread context.  */
-- 
2.21.1

[PATCH v3 23/29] Introduce fetch_pending_stop

[Tom Tromey]

This introduces a new "fetch_pending_stop" function and changes gdb to
use it.  This function removes the first matching pending stop from
the list of such stops.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (get_windows_debug_event): Use
	fetch_pending_stop.
	* nat/windows-nat.h (fetch_pending_stop): Declare.
	* nat/windows-nat.c (fetch_pending_stop): New function.
---
 gdb/ChangeLog         |  7 +++++++
 gdb/nat/windows-nat.c | 28 ++++++++++++++++++++++++++++
 gdb/nat/windows-nat.h |  7 +++++++
 gdb/windows-nat.c     | 29 +++++++++--------------------
 4 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 2c2454b6f6f..823471eb4da 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -343,6 +343,34 @@ matching_pending_stop (bool debug_events)
 
 /* See nat/windows-nat.h.  */
 
+gdb::optional<pending_stop>
+fetch_pending_stop (bool debug_events)
+{
+  gdb::optional<pending_stop> result;
+  for (auto iter = pending_stops.begin ();
+       iter != pending_stops.end ();
+       ++iter)
+    {
+      if (desired_stop_thread_id == -1
+	  || desired_stop_thread_id == iter->thread_id)
+	{
+	  result = *iter;
+	  current_event = iter->event;
+
+	  DEBUG_EVENTS (("get_windows_debug_event - "
+			 "pending stop found in 0x%x (desired=0x%x)\n",
+			 iter->thread_id, desired_stop_thread_id));
+
+	  pending_stops.erase (iter);
+	  break;
+	}
+    }
+
+  return result;
+}
+
+/* See nat/windows-nat.h.  */
+
 BOOL
 continue_last_debug_event (DWORD continue_status, bool debug_events)
 {
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 0e9316577ba..68b72d4bbd2 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -22,6 +22,7 @@
 #include <windows.h>
 #include <vector>
 
+#include "gdbsupport/gdb_optional.h"
 #include "target/waitstatus.h"
 
 namespace windows_nat
@@ -231,6 +232,12 @@ extern handle_exception_result handle_exception
 
 extern bool matching_pending_stop (bool debug_events);
 
+/* See if a pending stop matches DESIRED_STOP_THREAD_ID.  If so,
+   remove it from the list of pending stops, set 'current_event', and
+   return it.  Otherwise, return an empty optional.  */
+
+extern gdb::optional<pending_stop> fetch_pending_stop (bool debug_events);
+
 /* A simple wrapper for ContinueDebugEvent that continues the last
    waited-for event.  If DEBUG_EVENTS is true, logging will be
    enabled.  */
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 67655dc46e4..9a1dd49a42b 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1522,29 +1522,18 @@ windows_nat_target::get_windows_debug_event (int pid,
   /* If there is a relevant pending stop, report it now.  See the
      comment by the definition of "pending_stops" for details on why
      this is needed.  */
-  for (auto iter = pending_stops.begin ();
-       iter != pending_stops.end ();
-       ++iter)
+  gdb::optional<pending_stop> stop = fetch_pending_stop (debug_events);
+  if (stop.has_value ())
     {
-      if (desired_stop_thread_id == -1
-	  || desired_stop_thread_id == iter->thread_id)
-	{
-	  thread_id = iter->thread_id;
-	  *ourstatus = iter->status;
-	  current_event = iter->event;
-
-	  inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
-	  current_windows_thread = thread_rec (inferior_ptid,
-					       INVALIDATE_CONTEXT);
-	  current_windows_thread->reload_context = 1;
+      thread_id = stop->thread_id;
+      *ourstatus = stop->status;
 
-	  DEBUG_EVENTS (("get_windows_debug_event - "
-			 "pending stop found in 0x%x (desired=0x%x)\n",
-			 thread_id, desired_stop_thread_id));
+      inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
+      current_windows_thread = thread_rec (inferior_ptid,
+					   INVALIDATE_CONTEXT);
+      current_windows_thread->reload_context = 1;
 
-	  pending_stops.erase (iter);
-	  return thread_id;
-	}
+      return thread_id;
     }
 
   last_sig = GDB_SIGNAL_0;
-- 
2.21.1

[PATCH v3 24/29] Move wait_for_debug_event to nat/windows-nat.c

[Tom Tromey]

This moves the wait_for_debug_event helper function to
nat/windows-nat.c, and changes gdbserver to use it.
wait_for_debug_event is a wrapper for WaitForDebugEvent that also sets
last_wait_event when appropriate.  This is needed to properly handle
queued stops.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (wait_for_debug_event): Move to
	nat/windows-nat.c.
	* nat/windows-nat.h (wait_for_debug_event): Declare.
	* nat/windows-nat.c (wait_for_debug_event): Move from
	windows-nat.c.  No longer static.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.c (win32_kill, get_child_debug_event): Use
	wait_for_debug_event.
---
 gdb/ChangeLog          |  8 ++++++++
 gdb/nat/windows-nat.c  | 10 ++++++++++
 gdb/nat/windows-nat.h  |  5 +++++
 gdb/windows-nat.c      | 11 -----------
 gdbserver/ChangeLog    |  5 +++++
 gdbserver/win32-low.cc |  6 +++---
 6 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 823471eb4da..bb28e9b13c7 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -385,5 +385,15 @@ continue_last_debug_event (DWORD continue_status, bool debug_events)
 			     continue_status);
 }
 
+/* See nat/windows-nat.h.  */
+
+BOOL
+wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout)
+{
+  BOOL result = WaitForDebugEvent (event, timeout);
+  if (result)
+    last_wait_event = *event;
+  return result;
+}
 
 }
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 68b72d4bbd2..846fa67f407 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -245,6 +245,11 @@ extern gdb::optional<pending_stop> fetch_pending_stop (bool debug_events);
 extern BOOL continue_last_debug_event (DWORD continue_status,
 				       bool debug_events);
 
+/* A simple wrapper for WaitForDebugEvent that also sets
+   'last_wait_event' on success.  */
+
+extern BOOL wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout);
+
 }
 
 #endif
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 9a1dd49a42b..23bc9b1714f 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1495,17 +1495,6 @@ ctrl_c_handler (DWORD event_type)
   return TRUE;
 }
 
-/* A wrapper for WaitForDebugEvent that sets "last_wait_event"
-   appropriately.  */
-static BOOL
-wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout)
-{
-  BOOL result = WaitForDebugEvent (event, timeout);
-  if (result)
-    last_wait_event = *event;
-  return result;
-}
-
 /* Get the next event from the child.  Returns a non-zero thread id if the event
    requires handling by WFI (or whatever).  */
 
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 33f64700153..d151505e9f8 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -797,7 +797,7 @@ win32_process_target::kill (process_info *process)
     {
       if (!child_continue (DBG_CONTINUE, -1))
 	break;
-      if (!WaitForDebugEvent (&current_event, INFINITE))
+      if (!wait_for_debug_event (&current_event, INFINITE))
 	break;
       if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT)
 	break;
@@ -1231,7 +1231,7 @@ get_child_debug_event (DWORD *continue_status,
 	 happen is the user will see a spurious breakpoint.  */
 
       current_event.dwDebugEventCode = 0;
-      if (!WaitForDebugEvent (&current_event, 0))
+      if (!wait_for_debug_event (&current_event, 0))
 	{
 	  OUTMSG2(("no attach events left\n"));
 	  fake_breakpoint_event ();
@@ -1246,7 +1246,7 @@ get_child_debug_event (DWORD *continue_status,
       /* Keep the wait time low enough for comfortable remote
 	 interruption, but high enough so gdbserver doesn't become a
 	 bottleneck.  */
-      if (!WaitForDebugEvent (&current_event, 250))
+      if (!wait_for_debug_event (&current_event, 250))
         {
 	  DWORD e  = GetLastError();
 
-- 
2.21.1

[PATCH v3 25/29] Make last_wait_event static

[Tom Tromey]

Now that last_wait_event is entirely handled in nat/windows-nat.c, it
can be made static.

gdb/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* nat/windows-nat.h (last_wait_event): Don't declare.
	(wait_for_debug_event): Update comment.
	* nat/windows-nat.c (last_wait_event): Now static.
---
 gdb/ChangeLog         | 6 ++++++
 gdb/nat/windows-nat.c | 9 ++++++++-
 gdb/nat/windows-nat.h | 9 +--------
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index bb28e9b13c7..94e7f572c0a 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -31,7 +31,14 @@ DWORD current_process_id;
 DWORD main_thread_id;
 enum gdb_signal last_sig = GDB_SIGNAL_0;
 DEBUG_EVENT current_event;
-DEBUG_EVENT last_wait_event;
+
+/* The most recent event from WaitForDebugEvent.  Unlike
+   current_event, this is guaranteed never to come from a pending
+   stop.  This is important because only data from the most recent
+   event from WaitForDebugEvent can be used when calling
+   ContinueDebugEvent.  */
+static DEBUG_EVENT last_wait_event;
+
 windows_thread_info *current_windows_thread;
 DWORD desired_stop_thread_id = -1;
 std::vector<pending_stop> pending_stops;
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 846fa67f407..0597120c218 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -165,13 +165,6 @@ extern enum gdb_signal last_sig;
    stop.  */
 extern DEBUG_EVENT current_event;
 
-/* The most recent event from WaitForDebugEvent.  Unlike
-   current_event, this is guaranteed never to come from a pending
-   stop.  This is important because only data from the most recent
-   event from WaitForDebugEvent can be used when calling
-   ContinueDebugEvent.  */
-extern DEBUG_EVENT last_wait_event;
-
 /* Info on currently selected thread */
 extern windows_thread_info *current_windows_thread;
 
@@ -245,7 +238,7 @@ extern gdb::optional<pending_stop> fetch_pending_stop (bool debug_events);
 extern BOOL continue_last_debug_event (DWORD continue_status,
 				       bool debug_events);
 
-/* A simple wrapper for WaitForDebugEvent that also sets
+/* A simple wrapper for WaitForDebugEvent that also sets the internal
    'last_wait_event' on success.  */
 
 extern BOOL wait_for_debug_event (DEBUG_EVENT *event, DWORD timeout);
-- 
2.21.1

[PATCH v3 26/29] Add read_pc / write_pc support to win32-low

[Tom Tromey]

This changes win32-low.c to implement the read_pc and write_pc
methods.  A subsequent patch will need these.

Note that I have no way to test, or even compile, the win32-arm-low.c
change.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.h (win32_process_target::read_pc)
	(win32_process_target::write_pc): Declare.
	* win32-low.c (win32_process_target::read_pc)
	(win32_process_target::write_pc): New methods.
	* win32-i386-low.c (i386_win32_get_pc, i386_win32_set_pc): New
	functions.
	(the_low_target): Update.
	* win32-arm-low.c (arm_win32_get_pc, arm_win32_set_pc): New
	functions.
	(the_low_target): Update.
---
 gdbserver/ChangeLog         | 13 +++++++++++
 gdbserver/win32-arm-low.cc  | 23 +++++++++++++++++++
 gdbserver/win32-i386-low.cc | 46 +++++++++++++++++++++++++++++++++++++
 gdbserver/win32-low.cc      | 12 ++++++++++
 gdbserver/win32-low.h       |  9 ++++++++
 5 files changed, 103 insertions(+)

diff --git a/gdbserver/win32-arm-low.cc b/gdbserver/win32-arm-low.cc
index 78b7fd09ec3..77200112df1 100644
--- a/gdbserver/win32-arm-low.cc
+++ b/gdbserver/win32-arm-low.cc
@@ -115,6 +115,27 @@ arm_arch_setup (void)
 static const unsigned long arm_wince_breakpoint = 0xe6000010;
 #define arm_wince_breakpoint_len 4
 
+/* Implement win32_target_ops "get_pc" method.  */
+
+static CORE_ADDR
+arm_win32_get_pc (struct regcache *regcache)
+{
+  uint32_t pc;
+
+  collect_register_by_name (regcache, "pc", &pc);
+  return (CORE_ADDR) pc;
+}
+
+/* Implement win32_target_ops "set_pc" method.  */
+
+static void
+arm_win32_set_pc (struct regcache *regcache, CORE_ADDR pc)
+{
+  uint32_t newpc = pc;
+
+  supply_register_by_name (regcache, "pc", &newpc);
+}
+
 struct win32_target_ops the_low_target = {
   arm_arch_setup,
   sizeof (mappings) / sizeof (mappings[0]),
@@ -127,6 +148,8 @@ struct win32_target_ops the_low_target = {
   NULL, /* single_step */
   (const unsigned char *) &arm_wince_breakpoint,
   arm_wince_breakpoint_len,
+  arm_win32_get_pc,
+  arm_win32_set_pc,
   /* Watchpoint related functions.  See target.h for comments.  */
   NULL, /* supports_z_point_type */
   NULL, /* insert_point */
diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
index 1c14bc70362..eac15b5694a 100644
--- a/gdbserver/win32-i386-low.cc
+++ b/gdbserver/win32-i386-low.cc
@@ -450,6 +450,50 @@ i386_arch_setup (void)
   win32_tdesc = tdesc;
 }
 
+/* Implement win32_target_ops "get_pc" method.  */
+
+static CORE_ADDR
+i386_win32_get_pc (struct regcache *regcache)
+{
+  bool use_64bit = register_size (regcache->tdesc, 0) == 8;
+
+  if (use_64bit)
+    {
+      uint64_t pc;
+
+      collect_register_by_name (regcache, "rip", &pc);
+      return (CORE_ADDR) pc;
+    }
+  else
+    {
+      uint32_t pc;
+
+      collect_register_by_name (regcache, "eip", &pc);
+      return (CORE_ADDR) pc;
+    }
+}
+
+/* Implement win32_target_ops "set_pc" method.  */
+
+static void
+i386_win32_set_pc (struct regcache *regcache, CORE_ADDR pc)
+{
+  bool use_64bit = register_size (regcache->tdesc, 0) == 8;
+
+  if (use_64bit)
+    {
+      uint64_t newpc = pc;
+
+      supply_register_by_name (regcache, "rip", &newpc);
+    }
+  else
+    {
+      uint32_t newpc = pc;
+
+      supply_register_by_name (regcache, "eip", &newpc);
+    }
+}
+
 struct win32_target_ops the_low_target = {
   i386_arch_setup,
   sizeof (mappings) / sizeof (mappings[0]),
@@ -462,6 +506,8 @@ struct win32_target_ops the_low_target = {
   i386_single_step,
   &i386_win32_breakpoint,
   i386_win32_breakpoint_len,
+  i386_win32_get_pc,
+  i386_win32_set_pc,
   i386_supports_z_point_type,
   i386_insert_point,
   i386_remove_point,
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index d151505e9f8..131eacb13c4 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -1659,6 +1659,18 @@ win32_process_target::sw_breakpoint_from_kind (int kind, int *size)
   return the_low_target.breakpoint;
 }
 
+CORE_ADDR
+win32_process_target::read_pc (struct regcache *regcache)
+{
+  return (*the_low_target.get_pc) (regcache);
+}
+
+void
+win32_process_target::write_pc (struct regcache *regcache, CORE_ADDR pc)
+{
+  return (*the_low_target.set_pc) (regcache, pc);
+}
+
 /* The win32 target ops object.  */
 
 static win32_process_target the_win32_target;
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index 917f7275622..56ff8a9baf2 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -63,6 +63,11 @@ struct win32_target_ops
   const unsigned char *breakpoint;
   int breakpoint_len;
 
+  /* Get the PC register from REGCACHE.  */
+  CORE_ADDR (*get_pc) (struct regcache *regcache);
+  /* Set the PC register in REGCACHE.  */
+  void (*set_pc) (struct regcache *regcache, CORE_ADDR newpc);
+
   /* Breakpoint/Watchpoint related functions.  See target.h for comments.  */
   int (*supports_z_point_type) (char z_type);
   int (*insert_point) (enum raw_bkpt_type type, CORE_ADDR addr,
@@ -142,6 +147,10 @@ class win32_process_target : public process_stratum_target
   int get_tib_address (ptid_t ptid, CORE_ADDR *addr) override;
 
   const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
+
+  CORE_ADDR read_pc (regcache *regcache) override;
+
+  void write_pc (regcache *regcache, CORE_ADDR pc) override;
 };
 
 /* Retrieve the context for this thread, if not already retrieved.  */
-- 
2.21.1

[PATCH v3 27/29] Introduce win32_target_ops::decr_pc_after_break

[Tom Tromey]

This adds a decr_pc_after_break member to win32_target_ops and updates
the two Windows targets to set it.

Note that I can't test the win32-arm-low.c change.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.h (struct win32_target_ops) <decr_pc_after_break>: New
	field.
	* win32-i386-low.c (the_low_target): Update.
	* win32-arm-low.c (the_low_target): Update.
---
 gdbserver/ChangeLog         | 7 +++++++
 gdbserver/win32-arm-low.cc  | 1 +
 gdbserver/win32-i386-low.cc | 1 +
 gdbserver/win32-low.h       | 4 ++++
 4 files changed, 13 insertions(+)

diff --git a/gdbserver/win32-arm-low.cc b/gdbserver/win32-arm-low.cc
index 77200112df1..238ee4b05be 100644
--- a/gdbserver/win32-arm-low.cc
+++ b/gdbserver/win32-arm-low.cc
@@ -148,6 +148,7 @@ struct win32_target_ops the_low_target = {
   NULL, /* single_step */
   (const unsigned char *) &arm_wince_breakpoint,
   arm_wince_breakpoint_len,
+  0,
   arm_win32_get_pc,
   arm_win32_set_pc,
   /* Watchpoint related functions.  See target.h for comments.  */
diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
index eac15b5694a..48893af33b2 100644
--- a/gdbserver/win32-i386-low.cc
+++ b/gdbserver/win32-i386-low.cc
@@ -506,6 +506,7 @@ struct win32_target_ops the_low_target = {
   i386_single_step,
   &i386_win32_breakpoint,
   i386_win32_breakpoint_len,
+  1,
   i386_win32_get_pc,
   i386_win32_set_pc,
   i386_supports_z_point_type,
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index 56ff8a9baf2..d2b39a46fd9 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -63,6 +63,10 @@ struct win32_target_ops
   const unsigned char *breakpoint;
   int breakpoint_len;
 
+  /* Amount by which to decrement the PC after a breakpoint is
+     hit.  */
+  int decr_pc_after_break;
+
   /* Get the PC register from REGCACHE.  */
   CORE_ADDR (*get_pc) (struct regcache *regcache);
   /* Set the PC register in REGCACHE.  */
-- 
2.21.1

[PATCH v3 28/29] Implement stopped_by_sw_breakpoint for Windows gdbserver

[Tom Tromey]

This changes the Windows gdbserver port to implement the
stopped_by_sw_breakpoint target method.  This is needed to support
pending stops.

This is a separate patch now, because Pedro suggested splitting it out
for simpler bisecting, in the case that it introduces a bug.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	* win32-low.h  (win32_process_target::stopped_by_sw_breakpoint)
	(win32_process_target::supports_stopped_by_sw_breakpoint):
	Declare.
	* win32-low.c (win32_supports_z_point_type): Always handle
	Z_PACKET_SW_BP.
	(win32_insert_point): Call insert_memory_breakpoint when needed.
	(win32_remove_point): Call remove_memory_breakpoint when needed.
	(win32_process_target::stopped_by_sw_breakpoint)
	(win32_process_target::supports_stopped_by_sw_breakpoint): New
	methods.
	(win32_target_ops): Update.
	(maybe_adjust_pc): New function.
	(win32_wait): Call maybe_adjust_pc.
---
 gdbserver/ChangeLog    | 16 ++++++++++
 gdbserver/win32-low.cc | 67 ++++++++++++++++++++++++++++++++++--------
 gdbserver/win32-low.h  |  4 +++
 3 files changed, 75 insertions(+), 12 deletions(-)

diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 131eacb13c4..4312bb3ab7c 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -236,15 +236,18 @@ child_delete_thread (DWORD pid, DWORD tid)
 bool
 win32_process_target::supports_z_point_type (char z_type)
 {
-  return (the_low_target.supports_z_point_type != NULL
-	  && the_low_target.supports_z_point_type (z_type));
+  return (z_type == Z_PACKET_SW_BP
+	  || (the_low_target.supports_z_point_type != NULL
+	      && the_low_target.supports_z_point_type (z_type)));
 }
 
 int
 win32_process_target::insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
 				    int size, raw_breakpoint *bp)
 {
-  if (the_low_target.insert_point != NULL)
+  if (type == raw_bkpt_type_sw)
+    return insert_memory_breakpoint (bp);
+  else if (the_low_target.insert_point != NULL)
     return the_low_target.insert_point (type, addr, size, bp);
   else
     /* Unsupported (see target.h).  */
@@ -255,7 +258,9 @@ int
 win32_process_target::remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
 				    int size, raw_breakpoint *bp)
 {
-  if (the_low_target.remove_point != NULL)
+  if (type == raw_bkpt_type_sw)
+    return remove_memory_breakpoint (bp);
+  else if (the_low_target.remove_point != NULL)
     return the_low_target.remove_point (type, addr, size, bp);
   else
     /* Unsupported (see target.h).  */
@@ -1189,6 +1194,32 @@ windows_nat::handle_ms_vc_exception (const EXCEPTION_RECORD *rec)
   return false;
 }
 
+/* A helper function that will, if needed, set
+   'stopped_at_software_breakpoint' on the thread and adjust the
+   PC.  */
+
+static void
+maybe_adjust_pc ()
+{
+  struct regcache *regcache = get_thread_regcache (current_thread, 1);
+  child_fetch_inferior_registers (regcache, -1);
+
+  windows_thread_info *th = thread_rec (current_thread_ptid (),
+					DONT_INVALIDATE_CONTEXT);
+  th->stopped_at_software_breakpoint = false;
+
+  if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
+      && (current_event.u.Exception.ExceptionRecord.ExceptionCode
+	  == EXCEPTION_BREAKPOINT)
+      && child_initialization_done)
+    {
+      th->stopped_at_software_breakpoint = true;
+      CORE_ADDR pc = regcache_read_pc (regcache);
+      CORE_ADDR sw_breakpoint_pc = pc - the_low_target.decr_pc_after_break;
+      regcache_write_pc (regcache, sw_breakpoint_pc);
+    }
+}
+
 /* Get the next event from the child.  */
 
 static int
@@ -1417,8 +1448,6 @@ ptid_t
 win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
 			    int options)
 {
-  struct regcache *regcache;
-
   if (cached_status.kind != TARGET_WAITKIND_IGNORE)
     {
       /* The core always does a wait after creating the inferior, and
@@ -1446,12 +1475,12 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
 	case TARGET_WAITKIND_STOPPED:
 	case TARGET_WAITKIND_SIGNALLED:
 	case TARGET_WAITKIND_LOADED:
-	  OUTMSG2 (("Child Stopped with signal = %d \n",
-		    ourstatus->value.sig));
-
-	  regcache = get_thread_regcache (current_thread, 1);
-	  child_fetch_inferior_registers (regcache, -1);
-	  return debug_event_ptid (&current_event);
+	  {
+	    OUTMSG2 (("Child Stopped with signal = %d \n",
+		      ourstatus->value.sig));
+	    maybe_adjust_pc ();
+	    return debug_event_ptid (&current_event);
+	  }
 	default:
 	  OUTMSG (("Ignoring unknown internal event, %d\n", ourstatus->kind));
 	  /* fall-through */
@@ -1659,6 +1688,20 @@ win32_process_target::sw_breakpoint_from_kind (int kind, int *size)
   return the_low_target.breakpoint;
 }
 
+bool
+win32_process_target::stopped_by_sw_breakpoint ()
+{
+  windows_thread_info *th = thread_rec (current_thread_ptid (),
+					DONT_INVALIDATE_CONTEXT);
+  return th == nullptr ? false : th->stopped_at_software_breakpoint;
+}
+
+bool
+win32_process_target::supports_stopped_by_sw_breakpoint ()
+{
+  return true;
+}
+
 CORE_ADDR
 win32_process_target::read_pc (struct regcache *regcache)
 {
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index d2b39a46fd9..b3fa392dd31 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -155,6 +155,10 @@ class win32_process_target : public process_stratum_target
   CORE_ADDR read_pc (regcache *regcache) override;
 
   void write_pc (regcache *regcache, CORE_ADDR pc) override;
+
+  bool stopped_by_sw_breakpoint () override;
+
+  bool supports_stopped_by_sw_breakpoint () override;
 };
 
 /* Retrieve the context for this thread, if not already retrieved.  */
-- 
2.21.1

[PATCH v3 29/29] Add pending stop support to gdbserver's Windows port

[Tom Tromey]

This changes gdbserver to also handle pending stops, the same way that
gdb does.  This is PR gdb/22992.

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <tromey@adacore.com>

	PR gdb/22992
	* win32-low.c (child_continue): Call matching_pending_stop.
	(get_child_debug_event): Call fetch_pending_stop.  Push pending
	stop when needed.
---
 gdbserver/ChangeLog    |  7 +++++++
 gdbserver/win32-low.cc | 34 +++++++++++++++++++++++++++++++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 4312bb3ab7c..e1226b4b0db 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -430,6 +430,10 @@ continue_one_thread (thread_info *thread, int thread_id)
 static BOOL
 child_continue (DWORD continue_status, int thread_id)
 {
+  desired_stop_thread_id = thread_id;
+  if (matching_pending_stop (debug_threads))
+    return TRUE;
+
   /* The inferior will only continue after the ContinueDebugEvent
      call.  */
   for_each_thread ([&] (thread_info *thread)
@@ -1274,6 +1278,16 @@ get_child_debug_event (DWORD *continue_status,
   else
 #endif
     {
+      gdb::optional<pending_stop> stop = fetch_pending_stop (debug_threads);
+      if (stop.has_value ())
+	{
+	  *ourstatus = stop->status;
+	  current_event = stop->event;
+	  ptid = debug_event_ptid (&current_event);
+	  current_thread = find_thread_ptid (ptid);
+	  return 1;
+	}
+
       /* Keep the wait time low enough for comfortable remote
 	 interruption, but high enough so gdbserver doesn't become a
 	 bottleneck.  */
@@ -1377,7 +1391,7 @@ get_child_debug_event (DWORD *continue_status,
 	    ourstatus->value.sig = gdb_signal_from_host (exit_signal);
 	  }
       }
-      child_continue (DBG_CONTINUE, -1);
+      child_continue (DBG_CONTINUE, desired_stop_thread_id);
       CloseHandle (current_process_handle);
       current_process_handle = NULL;
       break;
@@ -1437,7 +1451,21 @@ get_child_debug_event (DWORD *continue_status,
     }
 
   ptid = debug_event_ptid (&current_event);
-  current_thread = find_thread_ptid (ptid);
+
+  if (desired_stop_thread_id != -1 && desired_stop_thread_id != ptid.lwp ())
+    {
+      /* Pending stop.  See the comment by the definition of
+	 "pending_stops" for details on why this is needed.  */
+      OUTMSG2 (("get_windows_debug_event - "
+		"unexpected stop in 0x%x (expecting 0x%x)\n",
+		ptid.lwp (), desired_stop_thread_id));
+      maybe_adjust_pc ();
+      pending_stops.push_back ({(DWORD) ptid.lwp (), *ourstatus, current_event});
+      ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+    }
+  else
+    current_thread = find_thread_ptid (ptid);
+
   return 1;
 }
 
@@ -1486,7 +1514,7 @@ win32_process_target::wait (ptid_t ptid, target_waitstatus *ourstatus,
 	  /* fall-through */
 	case TARGET_WAITKIND_SPURIOUS:
 	  /* do nothing, just continue */
-	  child_continue (continue_status, -1);
+	  child_continue (continue_status, desired_stop_thread_id);
 	  break;
 	}
     }
-- 
2.21.1

[pushed] gdbserver: fix format string warning in win32-low.cc (was: Re: [PATCH v3 29/29] Add pending stop support to gdbserver's Windows port)

[Simon Marchi]

On 2020-03-13 3:08 p.m., Tom Tromey wrote:
> This changes gdbserver to also handle pending stops, the same way that
> gdb does.  This is PR gdb/22992.
> 
> gdbserver/ChangeLog
> 2020-03-13  Tom Tromey  <tromey@adacore.com>
> 
> 	PR gdb/22992
> 	* win32-low.c (child_continue): Call matching_pending_stop.
> 	(get_child_debug_event): Call fetch_pending_stop.  Push pending
> 	stop when needed.
> ---
>  gdbserver/ChangeLog    |  7 +++++++
>  gdbserver/win32-low.cc | 34 +++++++++++++++++++++++++++++++---
>  2 files changed, 38 insertions(+), 3 deletions(-)

I pushed the following patch to fix a compilation error on Cygwin following this patch.

From e2275c6ee8caa98d6526422743a97d5dd5ac040d Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Wed, 15 Apr 2020 21:09:17 -0400
Subject: [PATCH] gdbserver: fix format string warning in win32-low.cc
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When compiling on Cygwin, we get:

      CXX    win32-low.o
    /home/smarchi/src/binutils-gdb/gdbserver/win32-low.cc: In function ‘int get_child_debug_event(DWORD*, target_waitstatus*)’:
    /home/smarchi/src/binutils-gdb/gdbserver/win32-low.cc:1459:17: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 2 has type ‘long int’ [-Werror=format=]
     1459 |       OUTMSG2 (("get_windows_debug_event - "
          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
     1460 |   "unexpected stop in 0x%x (expecting 0x%x)\n",
          |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     1461 |   ptid.lwp (), desired_stop_thread_id));
          |   ~~~~~~~~~~~
          |            |
          |            long int
    /home/smarchi/src/binutils-gdb/gdbserver/win32-low.cc:52:11: note: in definition of macro ‘OUTMSG2’
       52 |    printf X;    \
          |           ^
    /home/smarchi/src/binutils-gdb/gdbserver/win32-low.cc:1460:26: note: format string is defined here
     1460 |   "unexpected stop in 0x%x (expecting 0x%x)\n",
          |                         ~^
          |                          |
          |                          unsigned int
          |                         %lx

`ptid.lwp ()` is a `long` value, so it indeed needs the `l` size modifier.

gdbserver/ChangeLog:

	* win32-low.cc (get_child_debug_event): Fix format string warning.
---
 gdbserver/ChangeLog    | 4 ++++
 gdbserver/win32-low.cc | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index 2b381455ed79..2abe0f1268c2 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,7 @@
+2020-04-15  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* win32-low.cc (get_child_debug_event): Fix format string warning.
+
 2020-04-13  Tom Tromey  <tom@tromey.com>

 	* server.h (gdb_fildes_t): Remove typedef.
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index e1226b4b0db0..75305a4cfabb 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -1457,7 +1457,7 @@ get_child_debug_event (DWORD *continue_status,
       /* Pending stop.  See the comment by the definition of
 	 "pending_stops" for details on why this is needed.  */
       OUTMSG2 (("get_windows_debug_event - "
-		"unexpected stop in 0x%x (expecting 0x%x)\n",
+		"unexpected stop in 0x%lx (expecting 0x%x)\n",
 		ptid.lwp (), desired_stop_thread_id));
       maybe_adjust_pc ();
       pending_stops.push_back ({(DWORD) ptid.lwp (), *ourstatus, current_event});
-- 
2.26.0

Re: [PATCH v3 00/29] Windows code sharing + bug fix

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> This is v3 of the series to share a lot of the Windows code between
Tom> gdb and gdbserver.  It also fixes a bug that a customer reported; in
Tom> fact this fix was the origin of this series.  See patch #11 for
Tom> details on the bug.

Tom> I think this addresses all the review comments.  It's somewhat hard to
Tom> be sure since they were done in gerrit, and extracting comments from
Tom> that is a pain.  Also I think I had sent v2 before I updated my
Tom> scripts to preserve the gerrit change ID, so it is in gerrit twice.
Tom> Anyway, the reviews happened around Nov 2019, so you can find some in
Tom> the mailing list archives.

Tom> I tested this largely by hand.  I also ran it through the AdaCore test
Tom> suite, where it did ok.  (There were some issues, but I think they are
Tom> largely unrelated; some things like gdb printing paths that the test
Tom> suite didn't really recognize.)

Tom> I've also updated this to handle the WOW64 stuff, but only in a
Tom> minimal way -- I didn't try to make this work in gdbserver.

I'm checking this in now.
Let me know if there are any issues.

Tom

Re: [PATCH v3 00/29] Windows code sharing + bug fix

[Hannes Domani]

 Am Mittwoch, 8. April 2020, 22:33:18 MESZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben:

> >>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
>
> Tom> This is v3 of the series to share a lot of the Windows code between
> Tom> gdb and gdbserver.  It also fixes a bug that a customer reported; in
> Tom> fact this fix was the origin of this series.  See patch #11 for
> Tom> details on the bug.
>
> Tom> I think this addresses all the review comments.  It's somewhat hard to
> Tom> be sure since they were done in gerrit, and extracting comments from
> Tom> that is a pain.  Also I think I had sent v2 before I updated my
> Tom> scripts to preserve the gerrit change ID, so it is in gerrit twice.
> Tom> Anyway, the reviews happened around Nov 2019, so you can find some in
> Tom> the mailing list archives.
>
> Tom> I tested this largely by hand.  I also ran it through the AdaCore test
> Tom> suite, where it did ok.  (There were some issues, but I think they are
> Tom> largely unrelated; some things like gdb printing paths that the test
> Tom> suite didn't really recognize.)
>
> Tom> I've also updated this to handle the WOW64 stuff, but only in a
> Tom> minimal way -- I didn't try to make this work in gdbserver.
>
> I'm checking this in now.
> Let me know if there are any issues.

I seems I kinda missed to actually use Wow64SuspendThread when I implemented
the WOW64 stuff.
I never noticed any problems, and SuspendThread actually seems to work fine
for WOW64 processes, but that might just be luck (or depend on the Windows
version).

You moved the SuspendThread stuff now into nat/windows-nat.c, so that
complicates the situation a bit (but it's also the reason I noticed).

Maybe I should just try to implement the WOW64 stuff for gdbserver as well, and
then try to move more shared stuff into nat/windows-nat.c?


Hannes

Re: [PATCH v3 00/29] Windows code sharing + bug fix

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes> I seems I kinda missed to actually use Wow64SuspendThread when I implemented
Hannes> the WOW64 stuff.

Could you double-check to make sure I didn't mess it up when rebasing
this series over your patches?

Hannes> Maybe I should just try to implement the WOW64 stuff for gdbserver as well, and
Hannes> then try to move more shared stuff into nat/windows-nat.c?

It would be nice to try to keep the code more in sync.
They still aren't identical but they do share quite a bit more now.
So, unless it's unusually hard, I would say yes.

Tom

Re: [PATCH v3 00/29] Windows code sharing + bug fix

[Jon Turney]

On 09/04/2020 03:49, Tom Tromey wrote:
>>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Hannes> I seems I kinda missed to actually use Wow64SuspendThread when I implemented
> Hannes> the WOW64 stuff.
> 
> Could you double-check to make sure I didn't mess it up when rebasing
> this series over your patches?
> 
> Hannes> Maybe I should just try to implement the WOW64 stuff for gdbserver as well, and
> Hannes> then try to move more shared stuff into nat/windows-nat.c?
> 
> It would be nice to try to keep the code more in sync.
> They still aren't identical but they do share quite a bit more now.
> So, unless it's unusually hard, I would say yes.

I wonder if SuspendThread() is actually needed at all, since it doesn't 
make a huge amount of sense for the debugee to still be running when 
WaitForDebugEvent() returns.

Re: [PATCH v3 00/29] Windows code sharing + bug fix

[Hannes Domani]

 Am Donnerstag, 9. April 2020, 16:57:38 MESZ hat Jon Turney <jon.turney@dronecode.org.uk> Folgendes geschrieben:

> On 09/04/2020 03:49, Tom Tromey wrote:
>
> >>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
> >
> > Hannes> I seems I kinda missed to actually use Wow64SuspendThread when I implemented
> > Hannes> the WOW64 stuff.
> >
> > Could you double-check to make sure I didn't mess it up when rebasing
> > this series over your patches?
> >
> > Hannes> Maybe I should just try to implement the WOW64 stuff for gdbserver as well, and
> > Hannes> then try to move more shared stuff into nat/windows-nat.c?
> >
> > It would be nice to try to keep the code more in sync.
> > They still aren't identical but they do share quite a bit more now.
> > So, unless it's unusually hard, I would say yes.
>
>
> I wonder if SuspendThread() is actually needed at all, since it doesn't
> make a huge amount of sense for the debugee to still be running when
> WaitForDebugEvent() returns.

I took me quite a while to figure out a situation where SuspendThread() was
actually called.

And I can get there if I start the target process in a new console, and then
press ctrl-c in there so I get the DBG_CONTROL_C exception.
And then do something with threads, either "thread 1" or "info threads".

The backtrace then looks like this:
Thread 1 hit Breakpoint 1, 0x0000000077182a60 in SuspendThread () from C:\Windows\system32\kernel32.dll
(gdb) bt
#0  0x0000000077182a60 in SuspendThread () from C:\Windows\system32\kernel32.dll
#1  0x00000000005b9d32 in windows_nat::windows_thread_info::suspend (this=this@entry=0x11c79100) at C:/src/repos/binutils-gdb.git/gdb/nat/windows-nat.c:63
#2  0x00000000006c7e09 in windows_nat::thread_rec (ptid=..., disposition=disposition@entry=windows_nat::INVALIDATE_CONTEXT)
    at C:/src/repos/binutils-gdb.git/gdb/windows-nat.c:406
#3  0x00000000006c7e5e in windows_nat_target::fetch_registers (this=<optimized out>, regcache=0x11d053f0, r=8)
    at C:/src/repos/binutils-gdb.git/gdb/regcache.h:383
#4  0x000000000066ab3c in target_fetch_registers (regcache=regcache@entry=0x11d053f0, regno=regno@entry=8) at C:/src/repos/binutils-gdb.git/gdb/target.h:1326
#5  0x00000000005ec6ca in regcache::raw_update (regnum=8, this=0x11d053f0) at C:/src/repos/binutils-gdb.git/gdb/regcache.c:499
#6  regcache::raw_update (this=0x11d053f0, regnum=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/regcache.c:488
#7  0x00000000005ed51e in readable_regcache::raw_read (this=0x11d053f0, regnum=8, buf=0x11d33e70 "") at C:/src/repos/binutils-gdb.git/gdb/regcache.c:513
#8  0x00000000005ee1bf in readable_regcache::cooked_read_value (this=0x11d053f0, regnum=8) at C:/src/repos/binutils-gdb.git/gdb/regcache.c:663
#9  0x000000000061af87 in sentinel_frame_prev_register (this_frame=<optimized out>, this_prologue_cache=<optimized out>, regnum=<optimized out>)
    at C:/src/repos/binutils-gdb.git/gdb/sentinel-frame.c:53
#10 0x0000000000527546 in frame_unwind_register_value (next_frame=next_frame@entry=0x11837cf0, regnum=regnum@entry=8)
    at C:/src/repos/binutils-gdb.git/gdb/frame.c:1229
#11 0x000000000052780d in frame_register_unwind (next_frame=next_frame@entry=0x11837cf0, regnum=regnum@entry=8, optimizedp=optimizedp@entry=0xfd5f4b8,
    unavailablep=unavailablep@entry=0xfd5f4bc, lvalp=lvalp@entry=0xfd5f4c4, addrp=addrp@entry=0xfd5f4c8, realnump=realnump@entry=0xfd5f4c0,
    bufferp=bufferp@entry=0xfd5f508 "@˜+\017") at C:/src/repos/binutils-gdb.git/gdb/frame.c:1132
#12 0x0000000000527b99 in frame_unwind_register (next_frame=next_frame@entry=0x11837cf0, regnum=8, buf=buf@entry=0xfd5f508 "@˜+\017")
    at C:/src/repos/binutils-gdb.git/gdb/frame.c:1188
#13 0x000000000054d9d7 in i386_unwind_pc (gdbarch=0x118ae400, next_frame=0x11837cf0) at C:/src/repos/binutils-gdb.git/gdb/i386-tdep.c:1971
#14 0x0000000000526eb0 in frame_unwind_pc (this_frame=0x11837cf0) at C:/src/repos/binutils-gdb.git/gdb/frame.c:928
#15 0x0000000000527010 in get_frame_pc (frame=0x11837dc0) at C:/src/repos/binutils-gdb.git/gdb/frame.c:2399
#16 get_frame_address_in_block (this_frame=0x11837dc0) at C:/src/repos/binutils-gdb.git/gdb/frame.c:2429
#17 0x00000000005270bf in get_frame_address_in_block_if_available (this_frame=<optimized out>, pc=pc@entry=0xfd5f628)
    at C:/src/repos/binutils-gdb.git/gdb/frame.c:2492
#18 0x0000000000527240 in select_frame (fi=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/frame.c:1738
#19 0x0000000000528627 in select_frame (fi=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/frame.c:1727
#20 get_selected_frame (message=message@entry=0x0) at C:/src/repos/binutils-gdb.git/gdb/frame.c:1680
#21 0x00000000006712a5 in print_thread_info_1 (uiout=0x118b6f00, requested_threads=0x0, global_ids=0, pid=-1, show_global_ids=0)
    at C:/src/repos/binutils-gdb.git/gdb/thread.c:1193
#22 0x0000000000671ed7 in info_threads_command (arg=<optimized out>, from_tty=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/thread.c:1284
#23 0x0000000000480d22 in cmd_func (cmd=0xf4, args=0x2 <error: Cannot access memory at address 0x2>, from_tty=7976)
    at C:/src/repos/binutils-gdb.git/gdb/cli/cli-decode.c:1952
#24 0x0000000000674638 in execute_command (p=<optimized out>, p@entry=0x11c68790 "info threads", from_tty=1) at C:/src/repos/binutils-gdb.git/gdb/top.c:655
#25 0x0000000000516054 in command_handler (command=0x11c68790 "info threads") at C:/src/repos/binutils-gdb.git/gdb/event-top.c:587
#26 0x0000000000516f02 in command_line_handler (rl=...) at C:/src/repos/binutils-gdb.git/gdb/event-top.c:772
#27 0x0000000000516833 in gdb_rl_callback_handler (rl=0x11805f60 "info threads")
    at c:/msys64/mingw64/x86_64-w64-mingw32/include/c++/9.3.0/bits/unique_ptr.h:153
#28 0x00000000006e60fc in rl_callback_read_char () at C:/src/repos/binutils-gdb.git/readline/readline/callback.c:281
#29 0x00000000006e633f in rl_callback_read_char () at C:/src/repos/binutils-gdb.git/readline/readline/callback.c:222
#30 0x0000000000515bae in gdb_rl_callback_read_char_wrapper_noexcept () at C:/src/repos/binutils-gdb.git/gdb/event-top.c:176
#31 0x00000000005166e4 in gdb_rl_callback_read_char_wrapper (client_data=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/event-top.c:192
#32 0x00000000005159f2 in stdin_event_handler (error=<optimized out>, client_data=0x1180b720) at C:/src/repos/binutils-gdb.git/gdb/event-top.c:515
#33 0x0000000000514a10 in handle_file_event (ready_mask=2, file_ptr=0x11ce4ee0) at C:/src/repos/binutils-gdb.git/gdb/event-loop.c:701
#34 gdb_wait_for_event (block=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/event-loop.c:852
#35 gdb_wait_for_event (block=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/event-loop.c:714
#36 0x0000000000514bee in gdb_do_one_event () at C:/src/repos/binutils-gdb.git/gdb/event-loop.c:313
#37 0x0000000000514cde in gdb_do_one_event () at C:/src/repos/binutils-gdb.git/gdb/event-loop.c:337
#38 start_event_loop () at C:/src/repos/binutils-gdb.git/gdb/event-loop.c:337
#39 0x0000000000593ce2 in captured_command_loop () at C:/src/repos/binutils-gdb.git/gdb/main.c:360
#40 0x0000000000595b05 in captured_main (data=0xfd5fdc0) at C:/src/repos/binutils-gdb.git/gdb/main.c:1198
#41 gdb_main (args=args@entry=0xfd5fe20) at C:/src/repos/binutils-gdb.git/gdb/main.c:1213
#42 0x000000000098adc7 in main (argc=3, argv=0x1348a0) at C:/src/repos/binutils-gdb.git/gdb/gdb.c:32


Hannes

Re: [PATCH v3 00/29] Windows code sharing + bug fix

[Tom Tromey]

>>>>> "Jon" == Jon Turney <jon.turney@dronecode.org.uk> writes:

Jon> I wonder if SuspendThread() is actually needed at all, since it
Jon> doesn't make a huge amount of sense for the debugee to still be
Jon> running when WaitForDebugEvent() returns.

I think when single-stepping we need to suspend the other threads.
Other than that, I tend to agree.

IIRC I didn't try to generally clean up thread suspension in this
series.  I extended it a little, that's all.

Tom