[review] Add pending stop support to gdbserver's Windows port
Commit Message
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/432
......................................................................
Add pending stop support to gdbserver's Windows port
This changes gdbserver to also handle pending stops, the same way that
gdb does. This is PR gdb/22992.
Doing this required changing win32-low.c to use memory breakpoints and
to be implement the stopped_by_sw_breakpoint method.
Change-Id: I8d2d24a55dc081887deebdd952dd6512ae23fa71
gdb/gdbserver/ChangeLog
2019-10-29 Tom Tromey <tromey@adacore.com>
PR gdb/22992
* win32-low.c (win32_supports_z_point_type): Always handle
Z_PACKET_SW_BP.
(win32_insert_point): Call insert_memory_breakpoint when needed.
(win32_remove_point): Call remove_memory_breakpoint when needed.
(child_continue): Call matching_pending_stop.
(get_child_debug_event): Call fetch_pending_stop. Push pending
stop when needed.
(win32_wait): Set last_stop_was_breakpoint.
(win32_stopped_by_sw_breakpoint)
(win32_supports_stopped_by_sw_breakpoint): New functions.
(win32_target_ops): Update.
Change-Id: I74edd5a8dddb98429262599a8c3d4f9e16e7f770
---
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/win32-low.c
2 files changed, 91 insertions(+), 9 deletions(-)
Comments
Joel Brobecker has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/432
......................................................................
Patch Set 1:
Hey Pedro; is that something you would mind reviewing?
Thanks!
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/432
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
The comments to the gdb equivalent bits apply here as well, so I'll skip repeating them.
Maybe adding software breakpoints support could be split to a separate patch, for bisecting reasons, since that's a change that could plausibly uncover issues on its own.
| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +4,19 @@ AuthorDate: 2019-10-28 10:54:34 -0600
| +Commit: Tom Tromey <tromey@adacore.com>
| +CommitDate: 2019-10-29 10:08:42 -0600
| +
| +Add pending stop support to gdbserver's Windows port
| +
| +This changes gdbserver to also handle pending stops, the same way that
| +gdb does. This is PR gdb/22992.
| +
| +Doing this required changing win32-low.c to use memory breakpoints and
| +to be implement the stopped_by_sw_breakpoint method.
PS1, Line 13:
"to be implement" -> "to implement"
| +
| +Change-Id: I8d2d24a55dc081887deebdd952dd6512ae23fa71
| +
| +gdb/gdbserver/ChangeLog
| +2019-10-29 Tom Tromey <tromey@adacore.com>
| +
| + PR gdb/22992
| + * win32-low.c (win32_supports_z_point_type): Always handle
| + Z_PACKET_SW_BP.
@@ -1,5 +1,20 @@
2019-10-29 Tom Tromey <tromey@adacore.com>
+ PR gdb/22992
+ * win32-low.c (win32_supports_z_point_type): Always handle
+ Z_PACKET_SW_BP.
+ (win32_insert_point): Call insert_memory_breakpoint when needed.
+ (win32_remove_point): Call remove_memory_breakpoint when needed.
+ (child_continue): Call matching_pending_stop.
+ (get_child_debug_event): Call fetch_pending_stop. Push pending
+ stop when needed.
+ (win32_wait): Set last_stop_was_breakpoint.
+ (win32_stopped_by_sw_breakpoint)
+ (win32_supports_stopped_by_sw_breakpoint): New functions.
+ (win32_target_ops): Update.
+
+2019-10-29 Tom Tromey <tromey@adacore.com>
+
* win32-low.h (struct win32_target_ops) <decr_pc_after_break>: New
field.
* win32-i386-low.c (the_low_target): Update.
@@ -236,15 +236,18 @@
static int
win32_supports_z_point_type (char z_type)
{
- return (the_low_target.supports_z_point_type != NULL
- && the_low_target.supports_z_point_type (z_type));
+ return (z_type == Z_PACKET_SW_BP
+ || (the_low_target.supports_z_point_type != NULL
+ && the_low_target.supports_z_point_type (z_type)));
}
static int
win32_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
int size, struct raw_breakpoint *bp)
{
- if (the_low_target.insert_point != NULL)
+ if (type == raw_bkpt_type_sw)
+ return insert_memory_breakpoint (bp);
+ else if (the_low_target.insert_point != NULL)
return the_low_target.insert_point (type, addr, size, bp);
else
/* Unsupported (see target.h). */
@@ -255,7 +258,9 @@
win32_remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
int size, struct raw_breakpoint *bp)
{
- if (the_low_target.remove_point != NULL)
+ if (type == raw_bkpt_type_sw)
+ return remove_memory_breakpoint (bp);
+ else if (the_low_target.remove_point != NULL)
return the_low_target.remove_point (type, addr, size, bp);
else
/* Unsupported (see target.h). */
@@ -425,6 +430,10 @@
static BOOL
child_continue (DWORD continue_status, int thread_id)
{
+ desired_stop_thread_id = thread_id;
+ if (matching_pending_stop (debug_threads))
+ return TRUE;
+
/* The inferior will only continue after the ContinueDebugEvent
call. */
for_each_thread ([&] (thread_info *thread)
@@ -1231,6 +1240,16 @@
else
#endif
{
+ pending_stop stop;
+ if (fetch_pending_stop (debug_threads, &stop))
+ {
+ *ourstatus = stop.status;
+ current_event = stop.event;
+ ptid = debug_event_ptid (¤t_event);
+ current_thread = find_thread_ptid (ptid);
+ return 1;
+ }
+
/* Keep the wait time low enough for comfortable remote
interruption, but high enough so gdbserver doesn't become a
bottleneck. */
@@ -1318,7 +1337,7 @@
(unsigned) current_event.dwThreadId));
ourstatus->kind = TARGET_WAITKIND_EXITED;
ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
- child_continue (DBG_CONTINUE, -1);
+ child_continue (DBG_CONTINUE, desired_stop_thread_id);
CloseHandle (current_process_handle);
current_process_handle = NULL;
break;
@@ -1378,7 +1397,21 @@
}
ptid = debug_event_ptid (¤t_event);
- current_thread = find_thread_ptid (ptid);
+
+ if (desired_stop_thread_id != -1 && desired_stop_thread_id != ptid.lwp ())
+ {
+ /* Pending stop. See the comment by the definition of
+ "pending_stops" for details on why this is needed. */
+ OUTMSG2 (("get_windows_debug_event - "
+ "unexpected stop in 0x%x (expecting 0x%x)\n",
+ ptid.lwp (), desired_stop_thread_id));
+
+ pending_stops.push_back ({(DWORD) ptid.lwp (), *ourstatus, current_event});
+ ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+ }
+ else
+ current_thread = find_thread_ptid (ptid);
+
return 1;
}
@@ -1421,13 +1454,29 @@
regcache = get_thread_regcache (current_thread, 1);
child_fetch_inferior_registers (regcache, -1);
+
+ last_stop_was_breakpoint = false;
+ if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
+ && (current_event.u.Exception.ExceptionRecord.ExceptionCode
+ == EXCEPTION_BREAKPOINT))
+ {
+ CORE_ADDR pc = regcache_read_pc (regcache);
+ CORE_ADDR sw_breakpoint_pc
+ = pc - the_low_target.decr_pc_after_break;
+ if (gdb_breakpoint_here (sw_breakpoint_pc))
+ {
+ last_stop_was_breakpoint = true;
+ regcache_write_pc (regcache, sw_breakpoint_pc);
+ }
+ }
+
return debug_event_ptid (¤t_event);
default:
OUTMSG (("Ignoring unknown internal event, %d\n", ourstatus->kind));
/* fall-through */
case TARGET_WAITKIND_SPURIOUS:
/* do nothing, just continue */
- child_continue (continue_status, -1);
+ child_continue (continue_status, desired_stop_thread_id);
break;
}
}
@@ -1585,6 +1634,24 @@
return the_low_target.breakpoint;
}
+/* Implementation of the target_ops method
+ "stopped_by_sw_breakpoint". */
+
+static int
+win32_stopped_by_sw_breakpoint ()
+{
+ return last_stop_was_breakpoint;
+}
+
+/* Implementation of the target_ops method
+ "supports_stopped_by_sw_breakpoint". */
+
+static int
+win32_supports_stopped_by_sw_breakpoint ()
+{
+ return 1;
+}
+
/* Implementation of the target_ops method "read_pc". */
static CORE_ADDR
@@ -1624,8 +1691,8 @@
win32_supports_z_point_type,
win32_insert_point,
win32_remove_point,
- NULL, /* stopped_by_sw_breakpoint */
- NULL, /* supports_stopped_by_sw_breakpoint */
+ win32_stopped_by_sw_breakpoint,
+ win32_supports_stopped_by_sw_breakpoint,
NULL, /* stopped_by_hw_breakpoint */
NULL, /* supports_stopped_by_hw_breakpoint */
target_can_do_hardware_single_step,