Fix Windows debugging regression

Message ID 20200417152908.6263-1-tromey@adacore.com
State New
Headers show
Series
  • Fix Windows debugging regression
Related show

Commit Message

Tom Tromey April 17, 2020, 3:29 p.m.
The updated pending stop series introduced a regression in Windows
debugging.  When stopped at a software breakpoint, we would adjust the
PC each time it was requested -- however, more than a single
adjustment is incorrect.  This patch introduces a new flag that is
used to ensure the adjustment only happens a single time.

No similar change is needed in gdbserver, because it adjusts the PC in
a different way.

I still can't run the gdb test suite on Windows, but I can run the
internal AdaCore test suite there; and this fixes the regressions
there.

gdb/ChangeLog
2020-04-17  Tom Tromey  <tromey@adacore.com>

	* nat/windows-nat.h (struct windows_thread_info)
	<pc_adjusted>: New member.
	* windows-nat.c (windows_fetch_one_register): Check
	pc_adjusted.
	(windows_nat_target::get_windows_debug_event)
	(windows_nat_target::wait): Set pc_adjusted.
---
 gdb/ChangeLog         |  9 +++++++++
 gdb/nat/windows-nat.h |  5 +++++
 gdb/windows-nat.c     | 10 +++++++++-
 3 files changed, 23 insertions(+), 1 deletion(-)

-- 
2.21.1

Comments

Tom Tromey April 24, 2020, 12:47 p.m. | #1
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:


Tom> The updated pending stop series introduced a regression in Windows
Tom> debugging.  When stopped at a software breakpoint, we would adjust the
Tom> PC each time it was requested -- however, more than a single
Tom> adjustment is incorrect.  This patch introduces a new flag that is
Tom> used to ensure the adjustment only happens a single time.

Tom> No similar change is needed in gdbserver, because it adjusts the PC in
Tom> a different way.

Tom> I still can't run the gdb test suite on Windows, but I can run the
Tom> internal AdaCore test suite there; and this fixes the regressions
Tom> there.

Tom> gdb/ChangeLog
Tom> 2020-04-17  Tom Tromey  <tromey@adacore.com>

Tom> 	* nat/windows-nat.h (struct windows_thread_info)
Tom> 	<pc_adjusted>: New member.
Tom> 	* windows-nat.c (windows_fetch_one_register): Check
Tom> 	pc_adjusted.
Tom> 	(windows_nat_target::get_windows_debug_event)
Tom> 	(windows_nat_target::wait): Set pc_adjusted.

I'm checking this in.

Tom

Patch

diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 8d0fa9bd216..80c652b22a7 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -93,6 +93,11 @@  struct windows_thread_info
      breakpoint.  This is used to offset the PC when needed.  */
   bool stopped_at_software_breakpoint = false;
 
+  /* True if we've adjusted the PC after hitting a software
+     breakpoint, false otherwise.  This lets us avoid multiple
+     adjustments if the registers are read multiple times.  */
+  bool pc_adjusted = 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 b857f82eb89..f52af9a1274 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -604,6 +604,7 @@  windows_fetch_one_register (struct regcache *regcache,
   else
     {
       if (th->stopped_at_software_breakpoint
+	  && !th->pc_adjusted
 	  && r == gdbarch_pc_regnum (gdbarch))
 	{
 	  int size = register_size (gdbarch, r);
@@ -622,6 +623,8 @@  windows_fetch_one_register (struct regcache *regcache,
 	      value -= gdbarch_decr_pc_after_break (gdbarch);
 	      memcpy (context_offset, &value, size);
 	    }
+	  /* Make sure we only rewrite the PC a single time.  */
+	  th->pc_adjusted = true;
 	}
       regcache->raw_supply (r, context_offset);
     }
@@ -1757,6 +1760,7 @@  windows_nat_target::get_windows_debug_event (int pid,
 	  ptid_t ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
 	  th = thread_rec (ptid, INVALIDATE_CONTEXT);
 	  th->stopped_at_software_breakpoint = true;
+	  th->pc_adjusted = false;
 	}
       pending_stops.push_back ({thread_id, *ourstatus, current_event});
       thread_id = 0;
@@ -1835,7 +1839,11 @@  windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 		      || (current_event.u.Exception.ExceptionRecord.ExceptionCode
 			  == STATUS_WX86_BREAKPOINT))
 		  && windows_initialization_done)
-		current_windows_thread->stopped_at_software_breakpoint = true;
+		{
+		  current_windows_thread->stopped_at_software_breakpoint
+		    = true;
+		  current_windows_thread->pc_adjusted = false;
+		}
 	    }
 
 	  return result;