[commit] Improve MIPS breakpoint handling in gdbserver

Daniel Jacobowitz drow@false.org
Wed Dec 19 05:18:00 GMT 2007


On MIPS, gdbserver attempts to continue past and reinsert its internal
breakpoints by use of a second breakpoint.  Then, it was trying to
reinsert the second breakpoint.  A series of lucky coincidences made
this harmless, but it's still quite wrong.

Fixed as attached, tested on mips-linux.

-- 
Daniel Jacobowitz
CodeSourcery

2007-12-18  Daniel Jacobowitz  <dan@codesourcery.com>

	* linux-low.c (linux_wait_for_event): Update messages.  Do not
	reinsert auto-delete breakpoints.
	* mem-break.c (struct breakpoint): Change return type of handler to
	int.
	(set_breakpoint_at): Update handler type.
	(reinsert_breakpoint_handler): Return 1 instead of calling
	delete_breakpoint.
	(reinsert_breakpoint_by_bp): Check for the original breakpoint before
	setting a new one.
	(check_breakpoints): Delete auto-delete breakpoints and return 2.
	* mem-break.h (set_breakpoint_at): Update handler type.
	* thread-db.c (thread_db_create_event, thread_db_create_event): Update.
	* win32-low.c (auto_delete_breakpoint): New.
	(get_child_debug_event): Use it.

Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.67
diff -u -p -r1.67 linux-low.c
--- linux-low.c	7 Dec 2007 01:41:29 -0000	1.67
+++ linux-low.c	18 Dec 2007 21:48:47 -0000
@@ -623,6 +623,7 @@ linux_wait_for_event (struct thread_info
   CORE_ADDR stop_pc;
   struct process_info *event_child;
   int wstat;
+  int bp_status;
 
   /* Check for a process with a pending status.  */
   /* It is possible that the user changed the pending task's registers since
@@ -786,18 +787,20 @@ linux_wait_for_event (struct thread_info
 	  continue;
 	}
 
-      if (debug_threads)
-	fprintf (stderr, "Hit a (non-reinsert) breakpoint.\n");
+      bp_status = check_breakpoints (stop_pc);
 
-      if (check_breakpoints (stop_pc) != 0)
+      if (bp_status != 0)
 	{
+	  if (debug_threads)
+	    fprintf (stderr, "Hit a gdbserver breakpoint.\n");
+
 	  /* We hit one of our own breakpoints.  We mark it as a pending
 	     breakpoint, so that check_removed_breakpoint () will do the PC
 	     adjustment for us at the appropriate time.  */
 	  event_child->pending_is_breakpoint = 1;
 	  event_child->pending_stop_pc = stop_pc;
 
-	  /* Now we need to put the breakpoint back.  We continue in the event
+	  /* We may need to put the breakpoint back.  We continue in the event
 	     loop instead of simply replacing the breakpoint right away,
 	     in order to not lose signals sent to the thread that hit the
 	     breakpoint.  Unfortunately this increases the window where another
@@ -815,7 +818,10 @@ linux_wait_for_event (struct thread_info
 	     Otherwise, call the target function to figure out where we need
 	     our temporary breakpoint, create it, and continue executing this
 	     process.  */
-	  if (the_low_target.breakpoint_reinsert_addr == NULL)
+	  if (bp_status == 2)
+	    /* No need to reinsert.  */
+	    linux_resume_one_process (&event_child->head, 0, 0, NULL);
+	  else if (the_low_target.breakpoint_reinsert_addr == NULL)
 	    {
 	      event_child->bp_reinsert = stop_pc;
 	      uninsert_breakpoint (stop_pc);
@@ -831,6 +837,9 @@ linux_wait_for_event (struct thread_info
 	  continue;
 	}
 
+      if (debug_threads)
+	fprintf (stderr, "Hit a non-gdbserver breakpoint.\n");
+
       /* If we were single-stepping, we definitely want to report the
 	 SIGTRAP.  The single-step operation has completed, so also
          clear the stepping flag; in general this does not matter,
Index: mem-break.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/mem-break.c,v
retrieving revision 1.9
diff -u -p -r1.9 mem-break.c
--- mem-break.c	23 Aug 2007 18:08:48 -0000	1.9
+++ mem-break.c	18 Dec 2007 21:48:47 -0000
@@ -39,14 +39,16 @@ struct breakpoint
      in the *next chain somewhere).  */
   struct breakpoint *breakpoint_to_reinsert;
 
-  /* Function to call when we hit this breakpoint.  */
-  void (*handler) (CORE_ADDR);
+  /* Function to call when we hit this breakpoint.  If it returns 1,
+     the breakpoint will be deleted; 0, it will be reinserted for
+     another round.  */
+  int (*handler) (CORE_ADDR);
 };
 
 struct breakpoint *breakpoints;
 
 void
-set_breakpoint_at (CORE_ADDR where, void (*handler) (CORE_ADDR))
+set_breakpoint_at (CORE_ADDR where, int (*handler) (CORE_ADDR))
 {
   struct breakpoint *bp;
 
@@ -119,7 +121,7 @@ delete_breakpoint_at (CORE_ADDR addr)
     delete_breakpoint (bp);
 }
 
-static void
+static int
 reinsert_breakpoint_handler (CORE_ADDR stop_pc)
 {
   struct breakpoint *stop_bp, *orig_bp;
@@ -135,7 +137,7 @@ reinsert_breakpoint_handler (CORE_ADDR s
   (*the_target->write_memory) (orig_bp->pc, breakpoint_data,
 			       breakpoint_len);
   orig_bp->reinserting = 0;
-  delete_breakpoint (stop_bp);
+  return 1;
 }
 
 void
@@ -143,12 +145,12 @@ reinsert_breakpoint_by_bp (CORE_ADDR sto
 {
   struct breakpoint *bp, *orig_bp;
 
-  set_breakpoint_at (stop_at, reinsert_breakpoint_handler);
-
   orig_bp = find_breakpoint_at (stop_pc);
   if (orig_bp == NULL)
     error ("Could not find original breakpoint in list.");
 
+  set_breakpoint_at (stop_at, reinsert_breakpoint_handler);
+
   bp = find_breakpoint_at (stop_at);
   if (bp == NULL)
     error ("Could not find breakpoint in list (reinserting by breakpoint).");
@@ -203,8 +205,13 @@ check_breakpoints (CORE_ADDR stop_pc)
       return 0;
     }
 
-  (*bp->handler) (bp->pc);
-  return 1;
+  if ((*bp->handler) (bp->pc))
+    {
+      delete_breakpoint (bp);
+      return 2;
+    }
+  else
+    return 1;
 }
 
 void
Index: mem-break.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/mem-break.h,v
retrieving revision 1.7
diff -u -p -r1.7 mem-break.h
--- mem-break.h	23 Aug 2007 18:08:48 -0000	1.7
+++ mem-break.h	18 Dec 2007 21:48:47 -0000
@@ -24,10 +24,11 @@
 /* Breakpoints are opaque.  */
 
 /* Create a new breakpoint at WHERE, and call HANDLER when
-   it is hit.  */
+   it is hit.  HANDLER should return 1 if the breakpoint
+   should be deleted, 0 otherwise.  */
 
 void set_breakpoint_at (CORE_ADDR where,
-			void (*handler) (CORE_ADDR));
+			int (*handler) (CORE_ADDR));
 
 /* Delete a breakpoint previously inserted at ADDR with
    set_breakpoint_at.  */
Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/thread-db.c,v
retrieving revision 1.13
diff -u -p -r1.13 thread-db.c
--- thread-db.c	23 Oct 2007 20:05:03 -0000	1.13
+++ thread-db.c	18 Dec 2007 21:48:47 -0000
@@ -130,7 +130,7 @@ thread_db_state_str (td_thr_state_e stat
 }
 #endif
 
-static void
+static int
 thread_db_create_event (CORE_ADDR where)
 {
   td_event_msg_t msg;
@@ -159,14 +159,18 @@ thread_db_create_event (CORE_ADDR where)
   /* msg.event == TD_EVENT_CREATE */
 
   find_new_threads_callback (msg.th_p, NULL);
+
+  return 0;
 }
 
 #if 0
-static void
+static int
 thread_db_death_event (CORE_ADDR where)
 {
   if (debug_threads)
     fprintf (stderr, "Thread death event.\n");
+
+  return 0;
 }
 #endif
 
Index: win32-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
retrieving revision 1.21
diff -u -p -r1.21 win32-low.c
--- win32-low.c	3 Dec 2007 01:42:06 -0000	1.21
+++ win32-low.c	18 Dec 2007 21:48:47 -0000
@@ -1334,6 +1334,14 @@ fake_breakpoint_event (void)
   for_each_inferior (&all_threads, suspend_one_thread);
 }
 
+#ifdef _WIN32_WCE
+static int
+auto_delete_breakpoint (CORE_ADDR stop_pc)
+{
+  return 1;
+}
+#endif
+
 /* Get the next event from the child.  */
 
 static int
@@ -1445,7 +1453,7 @@ get_child_debug_event (struct target_wai
 	     it is hit.	 */
 	  set_breakpoint_at ((CORE_ADDR) (long) current_event.u
 			     .CreateProcessInfo.lpStartAddress,
-			     delete_breakpoint_at);
+			     auto_delete_breakpoint);
 	}
 #endif
       break;



More information about the Gdb-patches mailing list