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

classic Classic list List threaded Threaded
42 messages Options
123
Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
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


Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 02/29] Rename win32_thread_info to windows_thread_info

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

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

gdbserver/ChangeLog
2020-03-13  Tom Tromey  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
This changes windows_thread_info::name to be a unique_xmalloc_ptr,
removing some manual memory management.

gdb/ChangeLog
2020-03-13  Tom Tromey  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
Add a destructor to windows_thread_info that calls CloseHandle.

gdb/ChangeLog
2020-03-13  Tom Tromey  <[hidden email]>

        * 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
This wraps the shared windows-nat code in a windows_nat namespace.
This helps avoid name clashes.

gdb/ChangeLog
2020-03-13  Tom Tromey  <[hidden email]>

        * 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
This moves some Windows-related globals into nat/windows-nat.c,
sharing them between gdb and gdbserver.

gdb/ChangeLog
2020-03-13  Tom Tromey  <[hidden email]>

        * 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 17/29] Normalize handle_output_debug_string API

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-4
In reply to this post by Tom Tromey-4
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  <[hidden email]>

        * 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  <[hidden email]>

        * 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

123