[PATCH 7/7] Reset Windows hardware breakpoints on executable's entry point

[Hannes Domani]

Fixes these testsuite fails on Windows (actually more, but the others are
cascading failures):
FAIL: gdb.base/hbreak2.exp: hardware breakpoint insertion (the program exited)
FAIL: gdb.base/hbreak2.exp: run until function breakpoint (the program exited)
FAIL: gdb.base/hbreak2.exp: run to factorial(6) (the program exited)
FAIL: gdb.base/hbreak2.exp: run until hardware function breakpoint, optimized file (the program exited)

The problem happens if you only have hardware breakpoints active when
(re-)starting the program:

(gdb) start
Temporary breakpoint 1 at 0x401650: file C:/src/repos/binutils-gdb.git/gdb/tests
uite/gdb.base/break.c, line 43.
Starting program: C:\gdb\build64\gdb-git\gdb\testsuite\outputs\gdb.base\hbreak2\
hbreak2.exe

Temporary breakpoint 1, main (argc=1, argv=0x7e2120, envp=0x7e2900)
    at C:/src/repos/binutils-gdb.git/gdb/testsuite/gdb.base/break.c:43
43          if (argc == 12345) {  /* an unlikely value < 2^16, in case uninited
*/ /* set breakpoint 6 here */
(gdb) hb factorial
Hardware assisted breakpoint 2 at 0x401703: file C:/src/repos/binutils-gdb.git/g
db/testsuite/gdb.base/break.c, line 63.
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: C:\gdb\build64\gdb-git\gdb\testsuite\outputs\gdb.base\hbreak2\
hbreak2.exe
720
[Inferior 1 (process 7836) exited normally]

But if you stopped just once before reaching the hardware breakpoint, it
works fine:

(gdb) start
Temporary breakpoint 3 at 0x401650: file C:/src/repos/binutils-gdb.git/gdb/tests
uite/gdb.base/break.c, line 43.
Starting program: C:\gdb\build64\gdb-git\gdb\testsuite\outputs\gdb.base\hbreak2\
hbreak2.exe

Temporary breakpoint 3, main (argc=1, argv=0x322120, envp=0x322900)
    at C:/src/repos/binutils-gdb.git/gdb/testsuite/gdb.base/break.c:43
43          if (argc == 12345) {  /* an unlikely value < 2^16, in case uninited
*/ /* set breakpoint 6 here */
(gdb) c
Continuing.

Breakpoint 2, factorial (value=6)
    at C:/src/repos/binutils-gdb.git/gdb/testsuite/gdb.base/break.c:63
63        if (value > 1) {  /* set breakpoint 7 here */

I found out that cdb writes this error when trying to do the same:

Unable to set breakpoint error
The system resets thread contexts after the process
breakpoint so hardware breakpoints cannot be set.
Go to the executable's entry point and set it then.

So all the hardware breakpoints that were set before (or rather, their
debug register information) are practically lost when the process entry
point is reached.

This patch creates an internal breakpoint on the process entry point, which
when it is reached, resets all active hardware breakpoints, and continues
execution.
---
 gdb/windows-tdep.c | 130 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)

diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index aa0adeba99..90e4794fc5 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -37,6 +37,7 @@
 #include "coff/internal.h"
 #include "libcoff.h"
 #include "solist.h"
+#include "observable.h"
 
 #define CYGWIN_DLL_NAME "cygwin1.dll"
 
@@ -870,6 +871,99 @@ windows_get_siginfo_type (struct gdbarch *gdbarch)
   return siginfo_type;
 }
 
+/* Windows-specific cached data.  This is used by GDB for caching
+   purposes for each inferior.  This helps reduce the overhead of
+   transfering data from a remote target to the local host.  */
+struct windows_info
+{
+  CORE_ADDR entry_point = 0;
+};
+
+/* Per-inferior data key.  */
+static const struct inferior_key<windows_info> windows_inferior_data;
+
+/* Frees whatever allocated space there is to be freed and sets INF's
+   Windows cache data pointer to NULL.  */
+
+static void
+invalidate_windows_cache_inf (struct inferior *inf)
+{
+  windows_inferior_data.clear (inf);
+}
+
+/* Fetch the Windows cache info for INF.  This function always returns a
+   valid INFO pointer.  */
+
+static struct windows_info *
+get_windows_inferior_data (void)
+{
+  struct windows_info *info;
+  struct inferior *inf = current_inferior ();
+
+  info = windows_inferior_data.get (inf);
+  if (info == NULL)
+    info = windows_inferior_data.emplace (inf);
+
+  return info;
+}
+
+/* Breakpoint on entry point where any active hardware breakpoints will
+   be reset.  */
+static struct breakpoint_ops entry_point_breakpoint_ops;
+
+/* Reset active hardware breakpoints.  */
+
+static bool
+reset_hardware_breakpoints (struct breakpoint *b)
+{
+  if (b->type != bp_hardware_breakpoint
+      && b->type != bp_hardware_watchpoint
+      && b->type != bp_read_watchpoint
+      && b->type != bp_access_watchpoint)
+    return false;
+
+  struct bp_location *loc;
+  for (loc = b->loc; loc; loc = loc->next)
+    if (loc->enabled && loc->pspace == current_program_space
+	&& b->ops->remove_location (loc, REMOVE_BREAKPOINT) == 0)
+      b->ops->insert_location (loc);
+
+  return false;
+}
+
+/* This breakpoint type should never stop, but when reached, reset
+   the active hardware breakpoints.  */
+
+static void
+startup_breakpoint_check_status (bpstat bs)
+{
+  /* Never stop.  */
+  bs->stop = 0;
+
+  iterate_over_breakpoints (reset_hardware_breakpoints);
+}
+
+/* Update the breakpoint location to the current entry point.  */
+
+static void
+startup_breakpoint_re_set (struct breakpoint *b)
+{
+  struct windows_info *info = get_windows_inferior_data ();
+  CORE_ADDR entry_point = info->entry_point;
+
+  /* Do nothing if the entry point didn't change.  */
+  struct bp_location *loc;
+  for (loc = b->loc; loc; loc = loc->next)
+    if (loc->pspace == current_program_space && loc->address == entry_point)
+      return;
+
+  event_location_up location
+    = new_address_location (entry_point, nullptr, 0);
+  std::vector<symtab_and_line> sals;
+  sals = b->ops->decode_location (b, location.get (), current_program_space);
+  update_breakpoint_locations (b, current_program_space, sals, {});
+}
+
 /* Implement the "solib_create_inferior_hook" target_so_ops method.  */
 
 static void
@@ -914,6 +1008,30 @@ windows_solib_create_inferior_hook (int from_tty)
       if (vmaddr != exec_base)
 	objfile_rebase (symfile_objfile, exec_base - vmaddr);
     }
+
+  /* Create the entry point breakpoint if it doesn't exist already.  */
+  if (target_has_execution && exec_base != 0)
+    {
+      struct windows_info *info = get_windows_inferior_data ();
+      CORE_ADDR entry_point = exec_base
+	+ pe_data (exec_bfd)->pe_opthdr.AddressOfEntryPoint;
+      info->entry_point = entry_point;
+
+      breakpoint *startup_breakpoint
+	= iterate_over_breakpoints ([] (breakpoint *bp)
+	  {
+	    return bp->ops == &entry_point_breakpoint_ops;
+	  });
+      if (startup_breakpoint == nullptr)
+	{
+	  event_location_up location
+	    = new_address_location (entry_point, nullptr, 0);
+	  create_breakpoint (target_gdbarch(), location.get(), nullptr, -1,
+			     nullptr, 0, 1, bp_breakpoint, 0,
+			     AUTO_BOOLEAN_FALSE, &entry_point_breakpoint_ops,
+			     0, 1, 1, 0);
+	}
+    }
 }
 
 static struct target_so_ops windows_so_ops;
@@ -1095,6 +1213,18 @@ _initialize_windows_tdep ()
   windows_gdbarch_data_handle
     = gdbarch_data_register_post_init (init_windows_gdbarch_data);
 
+  /* Observers used to invalidate the cache when needed.  */
+  gdb::observers::inferior_exit.attach (invalidate_windows_cache_inf);
+  gdb::observers::inferior_appeared.attach (invalidate_windows_cache_inf);
+
+  initialize_breakpoint_ops ();
+  /* Entry point breakpoint.  */
+  entry_point_breakpoint_ops = bkpt_breakpoint_ops;
+  entry_point_breakpoint_ops.check_status
+    = startup_breakpoint_check_status;
+  entry_point_breakpoint_ops.re_set
+    = startup_breakpoint_re_set;
+
   init_w32_command_list ();
   add_cmd ("thread-information-block", class_info, display_tib,
 	   _("Display thread information block."),
-- 
2.26.2