From: "Tom Tromey (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: Luis Machado <luis.machado@linaro.org>, gdb-patches@sourceware.org
Subject: [review v2] Share Windows thread-suspend and -resume code
Date: Tue, 26 Nov 2019 17:11:00 -0000 [thread overview]
Message-ID: <20191126171131.57F1B20AF6@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1572371871000.I4e2be5d13faae083d9a83e65425c6c05d5d0e353@gnutoolchain-gerrit.osci.io>
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/412
......................................................................
Share Windows thread-suspend and -resume code
This adds "suspend" and "resume" methods to windows_thread_info, and
changes gdb and gdbserver to share this code.
gdb/ChangeLog
2019-11-26 Tom Tromey <tromey@adacore.com>
* windows-nat.c (thread_rec): Use windows_thread_info::suspend.
(windows_continue): Use windows_continue::resume.
* nat/windows-nat.h (struct windows_thread_info) <suspend,
resume>: Declare new methods.
* nat/windows-nat.c: New file.
* configure.nat (NATDEPFILES): Add nat/windows-nat.o when needed.
gdb/gdbserver/ChangeLog
2019-11-26 Tom Tromey <tromey@adacore.com>
* win32-low.c (win32_require_context, suspend_one_thread): Use
windows_thread_info::suspend.
(continue_one_thread): Use windows_thread_info::resume.
* configure.srv (srv_tgtobj): Add windows-nat.o when needed.
Change-Id: I4e2be5d13faae083d9a83e65425c6c05d5d0e353
---
M gdb/ChangeLog
M gdb/configure.nat
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/configure.srv
M gdb/gdbserver/win32-low.c
A gdb/nat/windows-nat.c
M gdb/nat/windows-nat.h
M gdb/windows-nat.c
8 files changed, 97 insertions(+), 62 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b897db4..6862a12 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
2019-11-26 Tom Tromey <tromey@adacore.com>
+ * windows-nat.c (thread_rec): Use windows_thread_info::suspend.
+ (windows_continue): Use windows_continue::resume.
+ * nat/windows-nat.h (struct windows_thread_info) <suspend,
+ resume>: Declare new methods.
+ * nat/windows-nat.c: New file.
+ * configure.nat (NATDEPFILES): Add nat/windows-nat.o when needed.
+
+2019-11-26 Tom Tromey <tromey@adacore.com>
+
* windows-nat.c (windows_add_thread, windows_delete_thread)
(windows_nat_target::fetch_registers)
(windows_nat_target::store_registers, fake_create_process)
diff --git a/gdb/configure.nat b/gdb/configure.nat
index 77a2ee8..15390e3 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -75,10 +75,10 @@
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/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 2ebf068..99b949a 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,12 @@
2019-11-26 Tom Tromey <tromey@adacore.com>
+ * win32-low.c (win32_require_context, suspend_one_thread): Use
+ windows_thread_info::suspend.
+ (continue_one_thread): Use windows_thread_info::resume.
+ * configure.srv (srv_tgtobj): Add windows-nat.o when needed.
+
+2019-11-26 Tom Tromey <tromey@adacore.com>
+
* win32-i386-low.c (update_debug_registers)
(i386_prepare_to_resume, i386_thread_added): Update.
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 1a4ab8e..404ed11 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -63,7 +63,7 @@
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=""
@@ -86,7 +86,7 @@
srv_linux_thread_db=yes
;;
i[34567]86-*-cygwin*) srv_regobj=""
- srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
+ srv_tgtobj="x86-low.o x86-dregs.o win32-low.o windows-nat.o win32-i386-low.o"
srv_tgtobj="${srv_tgtobj} arch/i386.o"
;;
i[34567]86-*-linux*) srv_tgtobj="${srv_tgtobj} arch/i386.o"
@@ -108,7 +108,7 @@
;;
i[34567]86-*-mingw32ce*)
srv_regobj=""
- srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
+ srv_tgtobj="x86-low.o x86-dregs.o win32-low.o windows-nat.o win32-i386-low.o"
srv_tgtobj="${srv_tgtobj} arch/i386.o"
srv_tgtobj="${srv_tgtobj} wincecompat.o"
# hostio_last_error implementation is in win32-low.c
@@ -117,7 +117,7 @@
srv_mingwce=yes
;;
i[34567]86-*-mingw*) srv_regobj=""
- srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
+ srv_tgtobj="x86-low.o x86-dregs.o win32-low.o windows-nat.o win32-i386-low.o"
srv_tgtobj="${srv_tgtobj} arch/i386.o"
srv_mingw=yes
;;
@@ -364,12 +364,12 @@
ipa_obj="${ipa_obj} arch/amd64-ipa.o"
;;
x86_64-*-mingw*) srv_regobj=""
- srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
+ srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o windows-nat.o win32-i386-low.o"
srv_tgtobj="${srv_tgtobj} arch/amd64.o"
srv_mingw=yes
;;
x86_64-*-cygwin*) srv_regobj=""
- srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
+ srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o windows-nat.o win32-i386-low.o"
srv_tgtobj="${srv_tgtobj} arch/amd64.o"
;;
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 7e3afff..59749f0 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -172,18 +172,7 @@
{
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);
}
}
@@ -436,13 +425,7 @@
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 ();
}
}
}
@@ -1334,17 +1317,7 @@
{
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
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
new file mode 100644
index 0000000..a98ff42
--- /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 db21ca7..488c9fb 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -19,6 +19,8 @@
#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
@@ -32,6 +34,12 @@
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 0cc3e1a..a85b9ff 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -377,27 +377,7 @@
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;
@@ -1329,9 +1309,7 @@
}
th->context.ContextFlags = 0;
}
- if (th->suspended > 0)
- (void) ResumeThread (th->h);
- th->suspended = 0;
+ th->resume ();
}
res = ContinueDebugEvent (current_event.dwProcessId,
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I4e2be5d13faae083d9a83e65425c6c05d5d0e353
Gerrit-Change-Number: 412
Gerrit-PatchSet: 2
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset
next prev parent reply other threads:[~2019-11-26 17:11 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-29 17:58 [review] " Tom Tromey (Code Review)
2019-11-04 14:30 ` Luis Machado (Code Review)
2019-11-06 19:14 ` Tom Tromey (Code Review)
2019-11-26 17:11 ` Tom Tromey (Code Review) [this message]
2019-11-29 18:08 ` [review v2] " Pedro Alves (Code Review)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191126171131.57F1B20AF6@gnutoolchain-gerrit.osci.io \
--to=gerrit@gnutoolchain-gerrit.osci.io \
--cc=gdb-patches@sourceware.org \
--cc=luis.machado@linaro.org \
--cc=tromey@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).