[review v2] Handle pending stops from the Windows kernel

["Pedro Alves (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Pedro Alves has posted comments on this change.

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


Patch Set 2: Code-Review+2

(2 comments)

Minor nit below.  Otherwise this version LGTM.

| --- gdb/nat/windows-nat.h
| +++ gdb/nat/windows-nat.h
| @@ -64,15 +64,19 @@ #endif
|    CONTEXT context {};
|  
|    /* Whether debug registers changed since we last set CONTEXT back to
|       the thread.  */
|    bool debug_registers_changed = false;
|  
|    /* Nonzero if CONTEXT is invalidated and must be re-read from the
|       inferior thread.  */
|    bool reload_context = false;
|  
| +  /* True if this thread is currently stopped at a breakpoint.  This
| +     is used to offset the PC when needed.  */
| +  bool stopped_at_breakpoint = false;

PS2, Line 76:

I think it would be better if this said "stopped at a software
breakpoint", and the field was called stopped_at_sw_breakpoint (for
example).  This is because you won't do the offsetting with hardware
breakpoints.

| +
|    /* The name of the thread, allocated by xmalloc.  */
|    gdb::unique_xmalloc_ptr<char> name;
|  };
|  
|  #endif
| --- gdb/windows-nat.c
| +++ gdb/windows-nat.c
| @@ -556,16 +610,37 @@ windows_fetch_one_register (struct regcache *regcache,
|      {
|        /* GDB treats segment registers as 32bit registers, but they are
|  	 in fact only 16 bits long.  Make sure we do not read extra
|  	 bits from our source buffer.  */
|        long l = *((long *) context_offset) & 0xffff;
|        regcache->raw_supply (r, (char *) &l);
|      }
|    else
| -    regcache->raw_supply (r, context_offset);
| +    {
| +      if (th->stopped_at_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);
| +	    }
| +	}

PS2, Line 637:

Clever to do this here.

| +      regcache->raw_supply (r, context_offset);
| +    }
|  }
|  
|  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);

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I5d0dd64c3d2d8220b534d3e02aeaa6f6815264ab
Gerrit-Change-Number: 414
Gerrit-PatchSet: 2
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Joel Brobecker <brobecker@adacore.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Fri, 29 Nov 2019 19:08:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment