[review] Implement stopped_by_sw_breakpoint for Windows gdbserver

Pedro Alves (Code Review) gerrit@gnutoolchain-gerrit.osci.io
Fri Nov 29 21:13:00 GMT 2019


Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/723
......................................................................


Patch Set 1:

(2 comments)

| --- gdb/gdbserver/win32-low.c
| +++ gdb/gdbserver/win32-low.c
| @@ -1178,9 +1183,34 @@ windows_nat::handle_ms_vc_exception (const EXCEPTION_RECORD *rec)
|  }
|  
| +/* A helper function that will, if needed, set 'stopped_at_breakpoint'
| +   on the thread and adjust the PC.  */
| +
| +static void
| +maybe_adjust_pc ()
| +{
| +  struct regcache *regcache = get_thread_regcache (current_thread, 1);
| +  child_fetch_inferior_registers (regcache, -1);

PS1, Line 1192:

> How about moving this into the conditional block so that we only fetch registers when we know we need them?

Also note that as is, the code reads a little odd, for doing
thread_rec (...INVALIDATE_CONTEXT) right after
child_fetch_inferior_registers.  I mean,
child_fetch_inferior_registers will pull in CONTEXT, and then right
after we invalidate & fetch it again.

| +
| +  windows_thread_info *th = thread_rec (current_thread_ptid (),
| +					INVALIDATE_CONTEXT);
| +  th->stopped_at_breakpoint = false;
| +
| +  if (current_event.dwDebugEventCode == EXCEPTION_DEBUG_EVENT
| +      && (current_event.u.Exception.ExceptionRecord.ExceptionCode
| +	  == EXCEPTION_BREAKPOINT)
| +      && child_initialization_done)
| +    {
| +      th->stopped_at_breakpoint = true;
| +      CORE_ADDR pc = regcache_read_pc (regcache);
| +      CORE_ADDR sw_breakpoint_pc = pc - the_low_target.decr_pc_after_break;
| +      regcache_write_pc (regcache, sw_breakpoint_pc);
| +    }

PS1, Line 1207:

I'm also curious about why this decr_pc handling isn't done within
child_fetch_inferior_registers to mirror how it's handled on the gdb
side.

| +}
| +
|  /* Get the next event from the child.  */
|  
|  static int
|  get_child_debug_event (DWORD *continue_status,
|  		       struct target_waitstatus *ourstatus)
|  {
|    ptid_t ptid;

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ib603fca745bc4ae43b6399f40919b1b399de5d51
Gerrit-Change-Number: 723
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Fri, 29 Nov 2019 21:13:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Luis Machado <luis.machado@linaro.org>
Gerrit-MessageType: comment



More information about the Gdb-patches mailing list