* mem-break.c (struct raw_breakpoint): New field shlib_disabled.

	(set_gdb_breakpoint_at): If GDB is inserting a breakpoint on top
	of another, then delete the previous, and validate all
	breakpoints.
	(validate_inserted_breakpoint): New.
	(delete_disabled_breakpoints): New.
	(validate_breakpoints): New.
	(check_mem_read): Validate breakpoints before trusting their
	shadow.  Delete disabled breakpoints.
	(check_mem_write): Validate breakpoints before trusting they
	should be inserted.  Delete disabled breakpoints.
	* mem-break.h (validate_breakpoints):
	* server.c (handle_query): Validate breakpoints when we see a
	qSymbol query.

4 files changed, 132 insertions, 2 deletions

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index eb6c0ba..21ea7c6 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,22 @@
 2010-04-01  Pedro Alves  <pedro@codesourcery.com>
 
+	* mem-break.c (struct raw_breakpoint): New field shlib_disabled.
+	(set_gdb_breakpoint_at): If GDB is inserting a breakpoint on top
+	of another, then delete the previous, and validate all
+	breakpoints.
+	(validate_inserted_breakpoint): New.
+	(delete_disabled_breakpoints): New.
+	(validate_breakpoints): New.
+	(check_mem_read): Validate breakpoints before trusting their
+	shadow.  Delete disabled breakpoints.
+	(check_mem_write): Validate breakpoints before trusting they
+	should be inserted.  Delete disabled breakpoints.
+	* mem-break.h (validate_breakpoints):
+	* server.c (handle_query): Validate breakpoints when we see a
+	qSymbol query.
+
+2010-04-01  Pedro Alves  <pedro@codesourcery.com>
+
 	* linux-low.c (linux_wait_1): Avoid setting need_step_over is
 	there's a GDB breakpoint at stop_pc.  Always report a trap to GDB
 	if we could tell there's a GDB breakpoint at stop_pc.
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 5256cb7..aa2f32f 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -65,6 +65,10 @@ struct raw_breakpoint
   /* Non-zero if this breakpoint is currently inserted in the
      inferior.  */
   int inserted;
+
+  /* Non-zero if this breakpoint is currently disabled because we no
+     longer detect it as inserted.  */
+  int shlib_disabled;
 };
 
 /* The type of a breakpoint.  */
@@ -326,6 +330,24 @@ set_gdb_breakpoint_at (CORE_ADDR where)
   if (breakpoint_data == NULL)
     return 1;
 
+  /* If we see GDB inserting a second breakpoint at the same address,
+     then the first breakpoint must have disappeared due to a shared
+     library unload.  On targets where the shared libraries are
+     handled by userspace, like SVR4, for example, GDBserver can't
+     tell if a library was loaded or unloaded.  Since we refcount
+     breakpoints, if we didn't do this, we'd just increase the
+     refcount of the previous breakpoint at this address, but the trap
+     was not planted in the inferior anymore, thus the breakpoint
+     would never be hit.  */
+  bp = find_gdb_breakpoint_at (where);
+  if (bp != NULL)
+    {
+      delete_gdb_breakpoint_at (where);
+
+      /* Might as well validate all other breakpoints.  */
+      validate_breakpoints ();
+    }
+
   bp = set_breakpoint_at (where, NULL);
   if (bp == NULL)
     return -1;
@@ -537,12 +559,70 @@ breakpoint_inserted_here (CORE_ADDR addr)
   return (bp != NULL && bp->inserted);
 }
 
+static int
+validate_inserted_breakpoint (struct raw_breakpoint *bp)
+{
+  unsigned char *buf;
+  int err;
+
+  gdb_assert (bp->inserted);
+
+  buf = alloca (breakpoint_len);
+  err = (*the_target->read_memory) (bp->pc, buf, breakpoint_len);
+  if (err || memcmp (buf, breakpoint_data, breakpoint_len) != 0)
+    {
+      /* Tag it as gone.  */
+      bp->inserted = 0;
+      bp->shlib_disabled = 1;
+      return 0;
+    }
+
+  return 1;
+}
+
+static void
+delete_disabled_breakpoints (void)
+{
+  struct process_info *proc = current_process ();
+  struct breakpoint *bp, *next;
+
+  for (bp = proc->breakpoints; bp != NULL; bp = next)
+    {
+      next = bp->next;
+      if (bp->raw->shlib_disabled)
+	delete_breakpoint_1 (proc, bp);
+    }
+}
+
+/* Check if breakpoints we inserted still appear to be inserted.  They
+   may disappear due to a shared library unload, and worse, a new
+   shared library may be reloaded at the same address as the
+   previously unloaded one.  If that happens, we should make sure that
+   the shadow memory of the old breakpoints isn't used when reading or
+   writing memory.  */
+
+void
+validate_breakpoints (void)
+{
+  struct process_info *proc = current_process ();
+  struct breakpoint *bp;
+
+  for (bp = proc->breakpoints; bp != NULL; bp = bp->next)
+    {
+      if (bp->raw->inserted)
+	validate_inserted_breakpoint (bp->raw);
+    }
+
+  delete_disabled_breakpoints ();
+}
+
 void
 check_mem_read (CORE_ADDR mem_addr, unsigned char *buf, int mem_len)
 {
   struct process_info *proc = current_process ();
   struct raw_breakpoint *bp = proc->raw_breakpoints;
   CORE_ADDR mem_end = mem_addr + mem_len;
+  int disabled_one = 0;
 
   for (; bp != NULL; bp = bp->next)
     {
@@ -568,8 +648,16 @@ check_mem_read (CORE_ADDR mem_addr, unsigned char *buf, int mem_len)
       buf_offset = start - mem_addr;
 
       if (bp->inserted)
-	memcpy (buf + buf_offset, bp->old_data + copy_offset, copy_len);
+	{
+	  if (validate_inserted_breakpoint (bp))
+	    memcpy (buf + buf_offset, bp->old_data + copy_offset, copy_len);
+	  else
+	    disabled_one = 1;
+	}
     }
+
+  if (disabled_one)
+    delete_disabled_breakpoints ();
 }
 
 void
@@ -578,6 +666,7 @@ check_mem_write (CORE_ADDR mem_addr, unsigned char *buf, int mem_len)
   struct process_info *proc = current_process ();
   struct raw_breakpoint *bp = proc->raw_breakpoints;
   CORE_ADDR mem_end = mem_addr + mem_len;
+  int disabled_one = 0;
 
   for (; bp != NULL; bp = bp->next)
     {
@@ -604,8 +693,16 @@ check_mem_write (CORE_ADDR mem_addr, unsigned char *buf, int mem_len)
 
       memcpy (bp->old_data + copy_offset, buf + buf_offset, copy_len);
       if (bp->inserted)
-	memcpy (buf + buf_offset, breakpoint_data + copy_offset, copy_len);
+	{
+	  if (validate_inserted_breakpoint (bp))
+	    memcpy (buf + buf_offset, breakpoint_data + copy_offset, copy_len);
+	  else
+	    disabled_one = 1;
+	}
     }
+
+  if (disabled_one)
+    delete_disabled_breakpoints ();
 }
 
 /* Delete all breakpoints, and un-insert them from the inferior.  */
diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h
index 01087fd..a226cc7 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -104,4 +104,8 @@ void delete_all_breakpoints (void);
 
 void free_all_breakpoints (struct process_info *proc);
 
+/* Check if breakpoints still seem to be inserted in the inferior.  */
+
+void validate_breakpoints (void);
+
 #endif /* MEM_BREAK_H */
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 232085a..c6fc005 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -858,6 +858,18 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 
   if (strcmp ("qSymbol::", own_buf) == 0)
     {
+      /* GDB is suggesting new symbols have been loaded.  This may
+	 mean a new shared library has been detected as loaded, so
+	 take the opportunity to check if breakpoints we think are
+	 inserted, still are.  Note that it isn't guaranteed that
+	 we'll see this when a shared library is loaded, and nor will
+	 we see this for unloads (although breakpoints in unloaded
+	 libraries shouldn't trigger), as GDB may not find symbols for
+	 the library at all.  We also re-validate breakpoints when we
+	 see a second GDB breakpoint for the same address, and or when
+	 we access breakpoint shadows.  */
+      validate_breakpoints ();
+
       if (target_running () && the_target->look_up_symbols != NULL)
 	(*the_target->look_up_symbols) ();