[PATCH 2/4] Introduce 'enum remove_bp_reason'

[PATCH 2/4] Introduce 'enum remove_bp_reason'

[Pedro Alves]

Makes the code more obvious.

gdb/ChangeLog:
2016-02-14  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (insertion_state_t): Delete.
	(enum remove_bp_reason): New.
	(detach_breakpoints, remove_breakpoint_1, remove_breakpoint):
	Adjust to use enum remove_bp_reason instead of insertion_state_t.
---
 gdb/breakpoint.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 05da8ed..9b37508 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -194,15 +194,20 @@ static void commands_command (char *, int);
 
 static void condition_command (char *, int);
 
-typedef enum
-  {
-    mark_inserted,
-    mark_uninserted
-  }
-insertion_state_t;
+/* Why are we removing the breakpoint from the target?  */
+
+enum remove_bp_reason
+{
+  /* A regular remove.  Remove the breakpoint and forget everything
+     about it.  */
+  REMOVE_BREAKPOINT,
+
+  /* Detach the breakpoints from a fork child.  */
+  DETACH_BREAKPOINT,
+};
 
 static int remove_breakpoint (struct bp_location *);
-static int remove_breakpoint_1 (struct bp_location *, insertion_state_t);
+static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason);
 
 static enum print_stop_action print_bp_stop_message (bpstat bs);
 
@@ -3922,7 +3927,7 @@ detach_breakpoints (ptid_t ptid)
       continue;
 
     if (bl->inserted)
-      val |= remove_breakpoint_1 (bl, mark_inserted);
+      val |= remove_breakpoint_1 (bl, DETACH_BREAKPOINT);
   }
 
   do_cleanups (old_chain);
@@ -3936,7 +3941,7 @@ detach_breakpoints (ptid_t ptid)
    *not* look at bl->pspace->aspace here.  */
 
 static int
-remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is)
+remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
 {
   int val;
 
@@ -4048,18 +4053,18 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is)
 
       if (val)
 	return val;
-      bl->inserted = (is == mark_inserted);
+      bl->inserted = (reason == DETACH_BREAKPOINT);
     }
   else if (bl->loc_type == bp_loc_hardware_watchpoint)
     {
       gdb_assert (bl->owner->ops != NULL
 		  && bl->owner->ops->remove_location != NULL);
 
-      bl->inserted = (is == mark_inserted);
+      bl->inserted = (reason == DETACH_BREAKPOINT);
       bl->owner->ops->remove_location (bl);
 
       /* Failure to remove any of the hardware watchpoints comes here.  */
-      if ((is == mark_uninserted) && (bl->inserted))
+      if (reason == REMOVE_BREAKPOINT && bl->inserted)
 	warning (_("Could not remove hardware watchpoint %d."),
 		 bl->owner->number);
     }
@@ -4074,7 +4079,7 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is)
       if (val)
 	return val;
 
-      bl->inserted = (is == mark_inserted);
+      bl->inserted = (reason == DETACH_BREAKPOINT);
     }
 
   return 0;
@@ -4097,7 +4102,7 @@ remove_breakpoint (struct bp_location *bl)
 
   switch_to_program_space_and_thread (bl->pspace);
 
-  ret = remove_breakpoint_1 (bl, mark_uninserted);
+  ret = remove_breakpoint_1 (bl, REMOVE_BREAKPOINT);
 
   do_cleanups (old_chain);
   return ret;
-- 
1.9.3

[PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error)

[Pedro Alves]

This series fixes:

 (gdb) record
 break marker2
 Breakpoint 2 at 0x40067a: file /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.reverse/waitpid-reverse.c, line 30.
 (gdb) PASS: gdb.reverse/waitpid-reverse.exp: set breakpoint at marker2
 continue
 Continuing.
 /home/pedro/gdb/mygit/src/gdb/record-full.c:1720: internal-error: record_full_remove_breakpoint: removing unknown breakpoint
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n)
 KFAIL: gdb.reverse/waitpid-reverse.exp: continue to breakpoint: marker2 (GDB internal error) (PRMS: gdb/19187)

The problem is that the target side can't distinguish between deleting
a breakpoint and detaching a breakpoint.

Right after a fork is detected, we detach breakpoints from the child
(detach_breakpoints), which calls into target_remove_breakpoint with
inferior_ptid pointing at the child process, but leaves the breakpoint
marked inserted (in the parent).

Since record-full.c's target_remove_breakpoint implementation doesn't
know we're detaching a breakpoint, it removes the breakpoint from its
own breakpoint table.

Then when we later remove the breakpoint from the parent, we fail that
assertion, since the breakpoint is unexpectedly not found in the
record-full.c breakpoint table.

The first three patches pass down the reason for removing the
breakpoint all the way to the target.

Patch 4 then simply makes record-full.c not delete the breakpoint from
its breakpoint table when we're detaching the breakpoint from a fork
child.

Pedro Alves (4):
  Simplify remove_breakpoint interface
  Introduce 'enum remove_bp_reason'
  Plumb enum remove_bp_reason all the way to target_remove_breakpoint
  Fix PR gdb/19187 (process record over a fork causes internal error)

 gdb/break-catch-sig.c                         |  3 +-
 gdb/break-catch-syscall.c                     |  2 +-
 gdb/breakpoint.c                              | 70 +++++++++++++--------------
 gdb/breakpoint.h                              | 14 +++++-
 gdb/corelow.c                                 | 10 +++-
 gdb/exec.c                                    | 10 +++-
 gdb/mem-break.c                               |  3 +-
 gdb/record-btrace.c                           |  6 ++-
 gdb/record-full.c                             | 15 ++++--
 gdb/remote.c                                  |  5 +-
 gdb/target-debug.h                            |  2 +
 gdb/target-delegates.c                        | 10 ++--
 gdb/target.c                                  |  5 +-
 gdb/target.h                                  |  9 ++--
 gdb/testsuite/gdb.reverse/waitpid-reverse.exp |  4 +-
 15 files changed, 105 insertions(+), 63 deletions(-)

-- 
1.9.3

[PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint

[Pedro Alves]

So the target knows whether we're detaching breakpoints.
Nothing uses the parameter in this patch yet.

gdb/ChangeLog:
2016-02-14  Pedro Alves  <palves@redhat.com>

	* break-catch-sig.c (signal_catchpoint_remove_location): Adjust
	interface.
	* break-catch-syscall.c (remove_catch_syscall):
	* breakpoint.c (enum remove_bp_reason): Moved to breakpoint.h.
	(remove_breakpoint_1): Pass 'reason' down.
	(remove_catch_fork, remove_catch_vfork, remove_catch_solib)
	(remove_catch_exec, remove_watchpoint, remove_masked_watchpoint)
	(base_breakpoint_remove_location, bkpt_remove_location)
	(bkpt_probe_remove_location, bkpt_probe_remove_location): Adjust
	interface.
	* breakpoint.h (enum remove_bp_reason): Moved here from
	breakpoint.c.
	(struct breakpoint_ops) <remove_location>: Add 'reason' parameter.
	* corelow.c (core_remove_breakpoint): New function.
	(init_core_ops): Install it as to_remove_breakpoint method.
	* exec.c (exec_remove_breakpoint): New function.
	(init_exec_ops): Install it as to_remove_breakpoint method.
	* mem-break.c (memory_remove_breakpoint): Adjust interface.
	* record-btrace.c (record_btrace_remove_breakpoint): Adjust
	interface.
	* record-full.c (record_full_remove_breakpoint)
	(record_full_core_remove_breakpoint): Adjust interface.
	* remote.c (remote_remove_breakpoint): Adjust interface.
	* target-debug.h (target_debug_print_enum_remove_bp_reason): New
	macro.
	* target-delegates.c: Regenerate.
	* target.c (target_remove_breakpoint): Add 'reason' parameter.
	* target.h (struct target_ops) <to_remove_breakpoint>: Add
	'reason' parameter.
	(target_remove_breakpoint, memory_remove_breakpoint): Add 'reason'
	parameter.
---
 gdb/break-catch-sig.c     |  3 ++-
 gdb/break-catch-syscall.c |  2 +-
 gdb/breakpoint.c          | 47 +++++++++++++++++++----------------------------
 gdb/breakpoint.h          | 14 +++++++++++++-
 gdb/corelow.c             | 10 +++++++++-
 gdb/exec.c                | 10 +++++++++-
 gdb/mem-break.c           |  3 ++-
 gdb/record-btrace.c       |  6 ++++--
 gdb/record-full.c         |  8 +++++---
 gdb/remote.c              |  5 +++--
 gdb/target-debug.h        |  2 ++
 gdb/target-delegates.c    | 10 ++++++----
 gdb/target.c              |  5 +++--
 gdb/target.h              |  9 ++++++---
 14 files changed, 84 insertions(+), 50 deletions(-)

diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index dcfae42..06ee44c 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -137,7 +137,8 @@ signal_catchpoint_insert_location (struct bp_location *bl)
    catchpoints.  */
 
 static int
-signal_catchpoint_remove_location (struct bp_location *bl)
+signal_catchpoint_remove_location (struct bp_location *bl,
+				   enum remove_bp_reason reason)
 {
   struct signal_catchpoint *c = (struct signal_catchpoint *) bl->owner;
   int i;
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index dbebdda..ec73a93 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -158,7 +158,7 @@ insert_catch_syscall (struct bp_location *bl)
    catchpoints.  */
 
 static int
-remove_catch_syscall (struct bp_location *bl)
+remove_catch_syscall (struct bp_location *bl, enum remove_bp_reason reason)
 {
   struct syscall_catchpoint *c = (struct syscall_catchpoint *) bl->owner;
   struct inferior *inf = current_inferior ();
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9b37508..311f06e 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -194,18 +194,6 @@ static void commands_command (char *, int);
 
 static void condition_command (char *, int);
 
-/* Why are we removing the breakpoint from the target?  */
-
-enum remove_bp_reason
-{
-  /* A regular remove.  Remove the breakpoint and forget everything
-     about it.  */
-  REMOVE_BREAKPOINT,
-
-  /* Detach the breakpoints from a fork child.  */
-  DETACH_BREAKPOINT,
-};
-
 static int remove_breakpoint (struct bp_location *);
 static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason);
 
@@ -3989,7 +3977,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
 	      && !memory_validate_breakpoint (bl->gdbarch, &bl->target_info))
 	    val = 0;
 	  else
-	    val = bl->owner->ops->remove_location (bl);
+	    val = bl->owner->ops->remove_location (bl, reason);
 	}
       else
 	{
@@ -4007,7 +3995,8 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
 					       &bl->overlay_target_info);
 		else
 		  target_remove_breakpoint (bl->gdbarch,
-					    &bl->overlay_target_info);
+					    &bl->overlay_target_info,
+					    reason);
 	      }
 	  /* Did we set a breakpoint at the VMA? 
 	     If so, we will have marked the breakpoint 'inserted'.  */
@@ -4023,7 +4012,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
 		 wrong code with the saved shadow contents.  */
 	      if (bl->loc_type == bp_loc_hardware_breakpoint
 		  || section_is_mapped (bl->section))
-		val = bl->owner->ops->remove_location (bl);
+		val = bl->owner->ops->remove_location (bl, reason);
 	      else
 		val = 0;
 	    }
@@ -4061,7 +4050,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
 		  && bl->owner->ops->remove_location != NULL);
 
       bl->inserted = (reason == DETACH_BREAKPOINT);
-      bl->owner->ops->remove_location (bl);
+      bl->owner->ops->remove_location (bl, reason);
 
       /* Failure to remove any of the hardware watchpoints comes here.  */
       if (reason == REMOVE_BREAKPOINT && bl->inserted)
@@ -4075,7 +4064,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
       gdb_assert (bl->owner->ops != NULL
 		  && bl->owner->ops->remove_location != NULL);
 
-      val = bl->owner->ops->remove_location (bl);
+      val = bl->owner->ops->remove_location (bl, reason);
       if (val)
 	return val;
 
@@ -8086,7 +8075,7 @@ insert_catch_fork (struct bp_location *bl)
    catchpoints.  */
 
 static int
-remove_catch_fork (struct bp_location *bl)
+remove_catch_fork (struct bp_location *bl, enum remove_bp_reason reason)
 {
   return target_remove_fork_catchpoint (ptid_get_pid (inferior_ptid));
 }
@@ -8204,7 +8193,7 @@ insert_catch_vfork (struct bp_location *bl)
    catchpoints.  */
 
 static int
-remove_catch_vfork (struct bp_location *bl)
+remove_catch_vfork (struct bp_location *bl, enum remove_bp_reason reason)
 {
   return target_remove_vfork_catchpoint (ptid_get_pid (inferior_ptid));
 }
@@ -8347,7 +8336,7 @@ insert_catch_solib (struct bp_location *ignore)
 }
 
 static int
-remove_catch_solib (struct bp_location *ignore)
+remove_catch_solib (struct bp_location *ignore, enum remove_bp_reason reason)
 {
   return 0;
 }
@@ -8677,7 +8666,7 @@ insert_catch_exec (struct bp_location *bl)
 }
 
 static int
-remove_catch_exec (struct bp_location *bl)
+remove_catch_exec (struct bp_location *bl, enum remove_bp_reason reason)
 {
   return target_remove_exec_catchpoint (ptid_get_pid (inferior_ptid));
 }
@@ -10686,7 +10675,7 @@ insert_watchpoint (struct bp_location *bl)
 /* Implement the "remove" breakpoint_ops method for hardware watchpoints.  */
 
 static int
-remove_watchpoint (struct bp_location *bl)
+remove_watchpoint (struct bp_location *bl, enum remove_bp_reason reason)
 {
   struct watchpoint *w = (struct watchpoint *) bl->owner;
   int length = w->exact ? 1 : bl->length;
@@ -10941,7 +10930,7 @@ insert_masked_watchpoint (struct bp_location *bl)
    masked hardware watchpoints.  */
 
 static int
-remove_masked_watchpoint (struct bp_location *bl)
+remove_masked_watchpoint (struct bp_location *bl, enum remove_bp_reason reason)
 {
   struct watchpoint *w = (struct watchpoint *) bl->owner;
 
@@ -12934,7 +12923,8 @@ base_breakpoint_insert_location (struct bp_location *bl)
 }
 
 static int
-base_breakpoint_remove_location (struct bp_location *bl)
+base_breakpoint_remove_location (struct bp_location *bl,
+				 enum remove_bp_reason reason)
 {
   internal_error_pure_virtual_called ();
 }
@@ -13096,12 +13086,12 @@ bkpt_insert_location (struct bp_location *bl)
 }
 
 static int
-bkpt_remove_location (struct bp_location *bl)
+bkpt_remove_location (struct bp_location *bl, enum remove_bp_reason reason)
 {
   if (bl->loc_type == bp_loc_hardware_breakpoint)
     return target_remove_hw_breakpoint (bl->gdbarch, &bl->target_info);
   else
-    return target_remove_breakpoint (bl->gdbarch, &bl->target_info);
+    return target_remove_breakpoint (bl->gdbarch, &bl->target_info, reason);
 }
 
 static int
@@ -13444,7 +13434,8 @@ bkpt_probe_insert_location (struct bp_location *bl)
 }
 
 static int
-bkpt_probe_remove_location (struct bp_location *bl)
+bkpt_probe_remove_location (struct bp_location *bl,
+			    enum remove_bp_reason reason)
 {
   /* Let's clear the semaphore before removing the location.  */
   if (bl->probe.probe->pops->clear_semaphore != NULL)
@@ -13452,7 +13443,7 @@ bkpt_probe_remove_location (struct bp_location *bl)
 					    bl->probe.objfile,
 					    bl->gdbarch);
 
-  return bkpt_remove_location (bl);
+  return bkpt_remove_location (bl, reason);
 }
 
 static void
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 054eab4d..c7d7462 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -39,6 +39,18 @@ struct linespec_result;
 struct linespec_sals;
 struct event_location;
 
+/* Why are we removing the breakpoint from the target?  */
+
+enum remove_bp_reason
+{
+  /* A regular remove.  Remove the breakpoint and forget everything
+     about it.  */
+  REMOVE_BREAKPOINT,
+
+  /* Detach the breakpoints from a fork child.  */
+  DETACH_BREAKPOINT,
+};
+
 /* This is the maximum number of bytes a breakpoint instruction can
    take.  Feel free to increase it.  It's just used in a few places to
    size arrays that should be independent of the target
@@ -519,7 +531,7 @@ struct breakpoint_ops
      with the "insert" method above.  Return 0 for success, 1 if the
      breakpoint, watchpoint or catchpoint type is not supported,
      -1 for failure.  */
-  int (*remove_location) (struct bp_location *);
+  int (*remove_location) (struct bp_location *, enum remove_bp_reason reason);
 
   /* Return true if it the target has stopped due to hitting
      breakpoint location BL.  This function does not check if we
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 224719e..e8c9cc8 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -920,6 +920,14 @@ ignore (struct target_ops *ops, struct gdbarch *gdbarch,
   return 0;
 }
 
+static int
+core_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
+			struct bp_target_info *bp_tgt,
+			enum remove_bp_reason reason)
+{
+  return 0;
+}
+
 
 /* Okay, let's be honest: threads gleaned from a core file aren't
    exactly lively, are they?  On the other hand, if we don't claim
@@ -1043,7 +1051,7 @@ init_core_ops (void)
   core_ops.to_xfer_partial = core_xfer_partial;
   core_ops.to_files_info = core_files_info;
   core_ops.to_insert_breakpoint = ignore;
-  core_ops.to_remove_breakpoint = ignore;
+  core_ops.to_remove_breakpoint = core_remove_breakpoint;
   core_ops.to_thread_alive = core_thread_alive;
   core_ops.to_read_description = core_read_description;
   core_ops.to_pid_to_str = core_pid_to_str;
diff --git a/gdb/exec.c b/gdb/exec.c
index 23c89c2..4dba3e1 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -1006,6 +1006,14 @@ ignore (struct target_ops *ops, struct gdbarch *gdbarch,
 }
 
 static int
+exec_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
+			struct bp_target_info *bp_tgt,
+			enum remove_bp_reason reason)
+{
+  return 0;
+}
+
+static int
 exec_has_memory (struct target_ops *ops)
 {
   /* We can provide memory if we have any file/target sections to read
@@ -1036,7 +1044,7 @@ Specify the filename of the executable file.";
   exec_ops.to_get_section_table = exec_get_section_table;
   exec_ops.to_files_info = exec_files_info;
   exec_ops.to_insert_breakpoint = ignore;
-  exec_ops.to_remove_breakpoint = ignore;
+  exec_ops.to_remove_breakpoint = exec_remove_breakpoint;
   exec_ops.to_stratum = file_stratum;
   exec_ops.to_has_memory = exec_has_memory;
   exec_ops.to_make_corefile_notes = exec_make_note_section;
diff --git a/gdb/mem-break.c b/gdb/mem-break.c
index 9bb4c45..803f62b 100644
--- a/gdb/mem-break.c
+++ b/gdb/mem-break.c
@@ -93,7 +93,8 @@ memory_insert_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
 
 int
 memory_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
-			  struct bp_target_info *bp_tgt)
+			  struct bp_target_info *bp_tgt,
+			  enum remove_bp_reason reason)
 {
   return gdbarch_memory_remove_breakpoint (gdbarch, bp_tgt);
 }
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 77b5180..6a5ffa4 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1406,7 +1406,8 @@ record_btrace_insert_breakpoint (struct target_ops *ops,
 static int
 record_btrace_remove_breakpoint (struct target_ops *ops,
 				 struct gdbarch *gdbarch,
-				 struct bp_target_info *bp_tgt)
+				 struct bp_target_info *bp_tgt,
+				 enum remove_bp_reason reason)
 {
   const char *old;
   int ret;
@@ -1419,7 +1420,8 @@ record_btrace_remove_breakpoint (struct target_ops *ops,
   ret = 0;
   TRY
     {
-      ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, bp_tgt);
+      ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, bp_tgt,
+						reason);
     }
   CATCH (except, RETURN_MASK_ALL)
     {
diff --git a/gdb/record-full.c b/gdb/record-full.c
index f6023bf..418a346 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1684,7 +1684,8 @@ record_full_insert_breakpoint (struct target_ops *ops,
 static int
 record_full_remove_breakpoint (struct target_ops *ops,
 			       struct gdbarch *gdbarch,
-			       struct bp_target_info *bp_tgt)
+			       struct bp_target_info *bp_tgt,
+			       enum remove_bp_reason reason)
 {
   struct record_full_breakpoint *bp;
   int ix;
@@ -1704,7 +1705,7 @@ record_full_remove_breakpoint (struct target_ops *ops,
 
 	      old_cleanups = record_full_gdb_operation_disable_set ();
 	      ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch,
-							bp_tgt);
+							bp_tgt, reason);
 	      do_cleanups (old_cleanups);
 
 	      if (ret != 0)
@@ -2171,7 +2172,8 @@ record_full_core_insert_breakpoint (struct target_ops *ops,
 static int
 record_full_core_remove_breakpoint (struct target_ops *ops,
 				    struct gdbarch *gdbarch,
-				    struct bp_target_info *bp_tgt)
+				    struct bp_target_info *bp_tgt,
+				    enum remove_bp_reason reason)
 {
   return 0;
 }
diff --git a/gdb/remote.c b/gdb/remote.c
index 6d56f19..404a5c6 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9339,7 +9339,8 @@ Target doesn't support breakpoints that have target side commands."));
 static int
 remote_remove_breakpoint (struct target_ops *ops,
 			  struct gdbarch *gdbarch,
-			  struct bp_target_info *bp_tgt)
+			  struct bp_target_info *bp_tgt,
+			  enum remove_bp_reason reason)
 {
   CORE_ADDR addr = bp_tgt->placed_address;
   struct remote_state *rs = get_remote_state ();
@@ -9368,7 +9369,7 @@ remote_remove_breakpoint (struct target_ops *ops,
       return (rs->buf[0] == 'E');
     }
 
-  return memory_remove_breakpoint (ops, gdbarch, bp_tgt);
+  return memory_remove_breakpoint (ops, gdbarch, bp_tgt, reason);
 }
 
 static enum Z_packet_type
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 5f44d11..ef7e14d 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -158,6 +158,8 @@
   target_debug_do_print (plongest (X))
 #define target_debug_print_struct_inferior_p(X)	\
   target_debug_do_print (host_address_to_string (X))
+#define target_debug_print_enum_remove_bp_reason(X) \
+  target_debug_do_print (plongest (X))
 
 static void
 target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index d23bc75..e21746f 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -262,24 +262,26 @@ debug_insert_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct b
 }
 
 static int
-delegate_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2)
+delegate_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2, enum remove_bp_reason arg3)
 {
   self = self->beneath;
-  return self->to_remove_breakpoint (self, arg1, arg2);
+  return self->to_remove_breakpoint (self, arg1, arg2, arg3);
 }
 
 static int
-debug_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2)
+debug_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2, enum remove_bp_reason arg3)
 {
   int result;
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_remove_breakpoint (...)\n", debug_target.to_shortname);
-  result = debug_target.to_remove_breakpoint (&debug_target, arg1, arg2);
+  result = debug_target.to_remove_breakpoint (&debug_target, arg1, arg2, arg3);
   fprintf_unfiltered (gdb_stdlog, "<- %s->to_remove_breakpoint (", debug_target.to_shortname);
   target_debug_print_struct_target_ops_p (&debug_target);
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_struct_gdbarch_p (arg1);
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_struct_bp_target_info_p (arg2);
+  fputs_unfiltered (", ", gdb_stdlog);
+  target_debug_print_enum_remove_bp_reason (arg3);
   fputs_unfiltered (") = ", gdb_stdlog);
   target_debug_print_int (result);
   fputs_unfiltered ("\n", gdb_stdlog);
diff --git a/gdb/target.c b/gdb/target.c
index ac66a3a..a0d5937 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2081,7 +2081,8 @@ target_insert_breakpoint (struct gdbarch *gdbarch,
 
 int
 target_remove_breakpoint (struct gdbarch *gdbarch,
-			  struct bp_target_info *bp_tgt)
+			  struct bp_target_info *bp_tgt,
+			  enum remove_bp_reason reason)
 {
   /* This is kind of a weird case to handle, but the permission might
      have been changed after breakpoints were inserted - in which case
@@ -2094,7 +2095,7 @@ target_remove_breakpoint (struct gdbarch *gdbarch,
     }
 
   return current_target.to_remove_breakpoint (&current_target,
-					      gdbarch, bp_tgt);
+					      gdbarch, bp_tgt, reason);
 }
 
 static void
diff --git a/gdb/target.h b/gdb/target.h
index e1419a9..7c8513a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -478,7 +478,8 @@ struct target_ops
 				 struct bp_target_info *)
       TARGET_DEFAULT_FUNC (memory_insert_breakpoint);
     int (*to_remove_breakpoint) (struct target_ops *, struct gdbarch *,
-				 struct bp_target_info *)
+				 struct bp_target_info *,
+				 enum remove_bp_reason)
       TARGET_DEFAULT_FUNC (memory_remove_breakpoint);
 
     /* Returns true if the target stopped because it executed a
@@ -1496,7 +1497,8 @@ extern int target_insert_breakpoint (struct gdbarch *gdbarch,
    machine.  Result is 0 for success, non-zero for error.  */
 
 extern int target_remove_breakpoint (struct gdbarch *gdbarch,
-				     struct bp_target_info *bp_tgt);
+				     struct bp_target_info *bp_tgt,
+				     enum remove_bp_reason reason);
 
 /* Returns true if the terminal settings of the inferior are in
    effect.  */
@@ -2356,7 +2358,8 @@ extern struct target_section_table *target_get_section_table
 /* From mem-break.c */
 
 extern int memory_remove_breakpoint (struct target_ops *, struct gdbarch *,
-				     struct bp_target_info *);
+				     struct bp_target_info *,
+				     enum remove_bp_reason);
 
 extern int memory_insert_breakpoint (struct target_ops *, struct gdbarch *,
 				     struct bp_target_info *);
-- 
1.9.3

[PATCH 1/4] Simplify remove_breakpoint interface

[Pedro Alves]

All callers pass mark_uninserted, so there's no need for the 'is'
parameter.

gdb/ChangeLog:
2016-02-14  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (remove_breakpoint): Remove 'is' parameter and
	always pass mark_uninserted to remove_breakpoint_1.
	(insert_breakpoint_locations, remove_breakpoints)
	(remove_breakpoints_pid, update_global_location_list): Update
	callers.
---
 gdb/breakpoint.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f99a7ab..05da8ed 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -201,7 +201,7 @@ typedef enum
   }
 insertion_state_t;
 
-static int remove_breakpoint (struct bp_location *, insertion_state_t);
+static int remove_breakpoint (struct bp_location *);
 static int remove_breakpoint_1 (struct bp_location *, insertion_state_t);
 
 static enum print_stop_action print_bp_stop_message (bpstat bs);
@@ -3182,7 +3182,7 @@ insert_breakpoint_locations (void)
 	{
 	  for (loc = bpt->loc; loc; loc = loc->next)
 	    if (loc->inserted)
-	      remove_breakpoint (loc, mark_uninserted);
+	      remove_breakpoint (loc);
 
 	  hw_breakpoint_error = 1;
 	  fprintf_unfiltered (tmp_error_stream,
@@ -3222,7 +3222,7 @@ remove_breakpoints (void)
   ALL_BP_LOCATIONS (bl, blp_tmp)
   {
     if (bl->inserted && !is_tracepoint (bl->owner))
-      val |= remove_breakpoint (bl, mark_uninserted);
+      val |= remove_breakpoint (bl);
   }
   return val;
 }
@@ -3267,7 +3267,7 @@ remove_breakpoints_pid (int pid)
 
     if (bl->inserted && !bl->target_info.persist)
       {
-	val = remove_breakpoint (bl, mark_uninserted);
+	val = remove_breakpoint (bl);
 	if (val != 0)
 	  return val;
       }
@@ -4081,7 +4081,7 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is)
 }
 
 static int
-remove_breakpoint (struct bp_location *bl, insertion_state_t is)
+remove_breakpoint (struct bp_location *bl)
 {
   int ret;
   struct cleanup *old_chain;
@@ -4097,7 +4097,7 @@ remove_breakpoint (struct bp_location *bl, insertion_state_t is)
 
   switch_to_program_space_and_thread (bl->pspace);
 
-  ret = remove_breakpoint_1 (bl, is);
+  ret = remove_breakpoint_1 (bl, mark_uninserted);
 
   do_cleanups (old_chain);
   return ret;
@@ -12572,7 +12572,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
 
 	  if (!keep_in_target)
 	    {
-	      if (remove_breakpoint (old_loc, mark_uninserted))
+	      if (remove_breakpoint (old_loc))
 		{
 		  /* This is just about all we can do.  We could keep
 		     this location on the global list, and try to
-- 
1.9.3

[PATCH 4/4] Fix PR gdb/19187 (process record over a fork causes internal error)

[Pedro Alves]

Right after a fork is detected, we detach breakpoints from the child
(detach_breakpoints), which calls into target_remove_breakpoint with
inferior_ptid pointing at the child process, but leaves the breakpoint
marked inserted (in the parent).

The problem is that record-full.c always deletes all knowledge of the
breakpoint.  Then when we later really delete the breakpoint from the
parent, we fail the assertion, since the breakpoint is unexpectedly
not found in the record-full.c breakpoint table.

The fix is simply to not forget about the breakpoint if we're
detaching it from a fork child.

gdb/ChangeLog:
2016-02-14  Pedro Alves  <palves@redhat.com>

	PR gdb/19187
	* record-full.c (record_full_remove_breakpoint): Don't remove the
	breakpoint from the record_full_breakpoints VEC if we're detaching
	the breakpoint from a fork child.

gdb/testsuite/ChangeLog:
2016-02-14  Pedro Alves  <palves@redhat.com>

	PR gdb/19187
	* gdb.reverse/waitpid-reverse.exp: Add comment and remove
	setup_kfails.
---
 gdb/record-full.c                             | 7 +++++--
 gdb/testsuite/gdb.reverse/waitpid-reverse.exp | 4 ++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/gdb/record-full.c b/gdb/record-full.c
index 418a346..e325869 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1712,8 +1712,11 @@ record_full_remove_breakpoint (struct target_ops *ops,
 		return ret;
 	    }
 
-	  VEC_unordered_remove (record_full_breakpoint_p,
-				record_full_breakpoints, ix);
+	  if (reason == REMOVE_BREAKPOINT)
+	    {
+	      VEC_unordered_remove (record_full_breakpoint_p,
+				    record_full_breakpoints, ix);
+	    }
 	  return 0;
 	}
     }
diff --git a/gdb/testsuite/gdb.reverse/waitpid-reverse.exp b/gdb/testsuite/gdb.reverse/waitpid-reverse.exp
index d583953..e8a1690 100644
--- a/gdb/testsuite/gdb.reverse/waitpid-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/waitpid-reverse.exp
@@ -18,6 +18,8 @@
 #
 # This test tests waitpid syscall for reverse execution.
 #
+# Also serves as regression test for gdb/19187 (recording across a
+# fork).
 
 if ![supports_reverse] {
     return
@@ -40,14 +42,12 @@ gdb_test "break marker2" \
     "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal.*" \
     "set breakpoint at marker2"
 
-setup_kfail "gdb/19187" *-*-*
 gdb_continue_to_breakpoint "marker2" ".*$srcfile:.*"
 
 gdb_test "break marker1" \
     "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal.*" \
     "set breakpoint at marker1"
 
-setup_kfail "gdb/19187" *-*-*
 gdb_test "reverse-continue" ".*$srcfile:$decimal.*" "reverse to marker1"
 
 # If the variable was recorded properly on syscall, the old contents (-1)
-- 
1.9.3

Re: [PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

Hi Pedro,

> So the target knows whether we're detaching breakpoints.
> Nothing uses the parameter in this patch yet.
>
> gdb/ChangeLog:
> 2016-02-14  Pedro Alves  <palves@redhat.com>
>
> 	* break-catch-sig.c (signal_catchpoint_remove_location): Adjust
> 	interface.
> 	* break-catch-syscall.c (remove_catch_syscall):
> 	* breakpoint.c (enum remove_bp_reason): Moved to breakpoint.h.
> 	(remove_breakpoint_1): Pass 'reason' down.
> 	(remove_catch_fork, remove_catch_vfork, remove_catch_solib)
> 	(remove_catch_exec, remove_watchpoint, remove_masked_watchpoint)
> 	(base_breakpoint_remove_location, bkpt_remove_location)
> 	(bkpt_probe_remove_location, bkpt_probe_remove_location): Adjust
> 	interface.
> 	* breakpoint.h (enum remove_bp_reason): Moved here from
> 	breakpoint.c.
> 	(struct breakpoint_ops) <remove_location>: Add 'reason' parameter.
> 	* corelow.c (core_remove_breakpoint): New function.
> 	(init_core_ops): Install it as to_remove_breakpoint method.
> 	* exec.c (exec_remove_breakpoint): New function.
> 	(init_exec_ops): Install it as to_remove_breakpoint method.
> 	* mem-break.c (memory_remove_breakpoint): Adjust interface.
> 	* record-btrace.c (record_btrace_remove_breakpoint): Adjust
> 	interface.
> 	* record-full.c (record_full_remove_breakpoint)
> 	(record_full_core_remove_breakpoint): Adjust interface.
> 	* remote.c (remote_remove_breakpoint): Adjust interface.
> 	* target-debug.h (target_debug_print_enum_remove_bp_reason): New
> 	macro.
> 	* target-delegates.c: Regenerate.
> 	* target.c (target_remove_breakpoint): Add 'reason' parameter.
> 	* target.h (struct target_ops) <to_remove_breakpoint>: Add
> 	'reason' parameter.
> 	(target_remove_breakpoint, memory_remove_breakpoint): Add 'reason'
> 	parameter.

We also need to add 'reason' in m32r_remove_breakpoint,
mips_remove_breakpoint and procfs_remove_breakpoint, otherwise the patch
breaks the all-targets build.

This patch series look good to me.

-- 
Yao (齐尧)

Re: [PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint

[Pedro Alves]

On 02/15/2016 09:12 AM, Yao Qi wrote:

> We also need to add 'reason' in m32r_remove_breakpoint,
> mips_remove_breakpoint and procfs_remove_breakpoint, otherwise the patch
> breaks the all-targets build.
> 
> This patch series look good to me.

Indeed.  Thanks for noticing.  I had a mental note to go over those, but
then lost it before posting...  :-)

Here's the updated patch.  I also added the missing comments to the
new functions in exec.c and corelow.c.

[PATCH] Plumb enum remove_bp_reason all the way to target_remove_breakpoint

So the target knows whether we're detaching breakpoints.
Nothing uses the parameter in this patch yet.

gdb/ChangeLog:
2016-02-15  Pedro Alves  <palves@redhat.com>

	* break-catch-sig.c (signal_catchpoint_remove_location): Adjust
	interface.
	* break-catch-syscall.c (remove_catch_syscall):
	* breakpoint.c (enum remove_bp_reason): Moved to breakpoint.h.
	(remove_breakpoint_1): Pass 'reason' down.
	(remove_catch_fork, remove_catch_vfork, remove_catch_solib)
	(remove_catch_exec, remove_watchpoint, remove_masked_watchpoint)
	(base_breakpoint_remove_location, bkpt_remove_location)
	(bkpt_probe_remove_location, bkpt_probe_remove_location): Adjust
	interface.
	* breakpoint.h (enum remove_bp_reason): Moved here from
	breakpoint.c.
	(struct breakpoint_ops) <remove_location>: Add 'reason' parameter.
	* corelow.c (core_remove_breakpoint): New function.
	(init_core_ops): Install it as to_remove_breakpoint method.
	* exec.c (exec_remove_breakpoint): New function.
	(init_exec_ops): Install it as to_remove_breakpoint method.
	* mem-break.c (memory_remove_breakpoint): Adjust interface.
	* record-btrace.c (record_btrace_remove_breakpoint): Adjust
	interface.
	* record-full.c (record_full_remove_breakpoint)
	(record_full_core_remove_breakpoint): Adjust interface.
	* remote-m32r.c (m32r_remove_breakpoint): Adjust interface.
	* remote-mips.c (mips_remove_breakpoint): Adjust interface.
	* remote.c (remote_remove_breakpoint): Adjust interface.
	* target-debug.h (target_debug_print_enum_remove_bp_reason): New
	macro.
	* target-delegates.c: Regenerate.
	* target.c (target_remove_breakpoint): Add 'reason' parameter.
	* target.h (struct target_ops) <to_remove_breakpoint>: Add
	'reason' parameter.
	(target_remove_breakpoint, memory_remove_breakpoint): Add 'reason'
	parameter.
---
 gdb/break-catch-sig.c     |  3 ++-
 gdb/break-catch-syscall.c |  2 +-
 gdb/breakpoint.c          | 47 +++++++++++++++++++----------------------------
 gdb/breakpoint.h          | 14 +++++++++++++-
 gdb/corelow.c             | 12 +++++++++++-
 gdb/exec.c                | 12 +++++++++++-
 gdb/mem-break.c           |  3 ++-
 gdb/nto-procfs.c          |  3 ++-
 gdb/record-btrace.c       |  6 ++++--
 gdb/record-full.c         |  8 +++++---
 gdb/remote-m32r-sdi.c     |  3 ++-
 gdb/remote-mips.c         |  5 +++--
 gdb/remote.c              |  5 +++--
 gdb/target-debug.h        |  2 ++
 gdb/target-delegates.c    | 10 ++++++----
 gdb/target.c              |  5 +++--
 gdb/target.h              |  9 ++++++---
 17 files changed, 95 insertions(+), 54 deletions(-)

diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index dcfae42..06ee44c 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -137,7 +137,8 @@ signal_catchpoint_insert_location (struct bp_location *bl)
    catchpoints.  */
 
 static int
-signal_catchpoint_remove_location (struct bp_location *bl)
+signal_catchpoint_remove_location (struct bp_location *bl,
+				   enum remove_bp_reason reason)
 {
   struct signal_catchpoint *c = (struct signal_catchpoint *) bl->owner;
   int i;
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index dbebdda..ec73a93 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -158,7 +158,7 @@ insert_catch_syscall (struct bp_location *bl)
    catchpoints.  */
 
 static int
-remove_catch_syscall (struct bp_location *bl)
+remove_catch_syscall (struct bp_location *bl, enum remove_bp_reason reason)
 {
   struct syscall_catchpoint *c = (struct syscall_catchpoint *) bl->owner;
   struct inferior *inf = current_inferior ();
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9b37508..311f06e 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -194,18 +194,6 @@ static void commands_command (char *, int);
 
 static void condition_command (char *, int);
 
-/* Why are we removing the breakpoint from the target?  */
-
-enum remove_bp_reason
-{
-  /* A regular remove.  Remove the breakpoint and forget everything
-     about it.  */
-  REMOVE_BREAKPOINT,
-
-  /* Detach the breakpoints from a fork child.  */
-  DETACH_BREAKPOINT,
-};
-
 static int remove_breakpoint (struct bp_location *);
 static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason);
 
@@ -3989,7 +3977,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
 	      && !memory_validate_breakpoint (bl->gdbarch, &bl->target_info))
 	    val = 0;
 	  else
-	    val = bl->owner->ops->remove_location (bl);
+	    val = bl->owner->ops->remove_location (bl, reason);
 	}
       else
 	{
@@ -4007,7 +3995,8 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
 					       &bl->overlay_target_info);
 		else
 		  target_remove_breakpoint (bl->gdbarch,
-					    &bl->overlay_target_info);
+					    &bl->overlay_target_info,
+					    reason);
 	      }
 	  /* Did we set a breakpoint at the VMA? 
 	     If so, we will have marked the breakpoint 'inserted'.  */
@@ -4023,7 +4012,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
 		 wrong code with the saved shadow contents.  */
 	      if (bl->loc_type == bp_loc_hardware_breakpoint
 		  || section_is_mapped (bl->section))
-		val = bl->owner->ops->remove_location (bl);
+		val = bl->owner->ops->remove_location (bl, reason);
 	      else
 		val = 0;
 	    }
@@ -4061,7 +4050,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
 		  && bl->owner->ops->remove_location != NULL);
 
       bl->inserted = (reason == DETACH_BREAKPOINT);
-      bl->owner->ops->remove_location (bl);
+      bl->owner->ops->remove_location (bl, reason);
 
       /* Failure to remove any of the hardware watchpoints comes here.  */
       if (reason == REMOVE_BREAKPOINT && bl->inserted)
@@ -4075,7 +4064,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason)
       gdb_assert (bl->owner->ops != NULL
 		  && bl->owner->ops->remove_location != NULL);
 
-      val = bl->owner->ops->remove_location (bl);
+      val = bl->owner->ops->remove_location (bl, reason);
       if (val)
 	return val;
 
@@ -8086,7 +8075,7 @@ insert_catch_fork (struct bp_location *bl)
    catchpoints.  */
 
 static int
-remove_catch_fork (struct bp_location *bl)
+remove_catch_fork (struct bp_location *bl, enum remove_bp_reason reason)
 {
   return target_remove_fork_catchpoint (ptid_get_pid (inferior_ptid));
 }
@@ -8204,7 +8193,7 @@ insert_catch_vfork (struct bp_location *bl)
    catchpoints.  */
 
 static int
-remove_catch_vfork (struct bp_location *bl)
+remove_catch_vfork (struct bp_location *bl, enum remove_bp_reason reason)
 {
   return target_remove_vfork_catchpoint (ptid_get_pid (inferior_ptid));
 }
@@ -8347,7 +8336,7 @@ insert_catch_solib (struct bp_location *ignore)
 }
 
 static int
-remove_catch_solib (struct bp_location *ignore)
+remove_catch_solib (struct bp_location *ignore, enum remove_bp_reason reason)
 {
   return 0;
 }
@@ -8677,7 +8666,7 @@ insert_catch_exec (struct bp_location *bl)
 }
 
 static int
-remove_catch_exec (struct bp_location *bl)
+remove_catch_exec (struct bp_location *bl, enum remove_bp_reason reason)
 {
   return target_remove_exec_catchpoint (ptid_get_pid (inferior_ptid));
 }
@@ -10686,7 +10675,7 @@ insert_watchpoint (struct bp_location *bl)
 /* Implement the "remove" breakpoint_ops method for hardware watchpoints.  */
 
 static int
-remove_watchpoint (struct bp_location *bl)
+remove_watchpoint (struct bp_location *bl, enum remove_bp_reason reason)
 {
   struct watchpoint *w = (struct watchpoint *) bl->owner;
   int length = w->exact ? 1 : bl->length;
@@ -10941,7 +10930,7 @@ insert_masked_watchpoint (struct bp_location *bl)
    masked hardware watchpoints.  */
 
 static int
-remove_masked_watchpoint (struct bp_location *bl)
+remove_masked_watchpoint (struct bp_location *bl, enum remove_bp_reason reason)
 {
   struct watchpoint *w = (struct watchpoint *) bl->owner;
 
@@ -12934,7 +12923,8 @@ base_breakpoint_insert_location (struct bp_location *bl)
 }
 
 static int
-base_breakpoint_remove_location (struct bp_location *bl)
+base_breakpoint_remove_location (struct bp_location *bl,
+				 enum remove_bp_reason reason)
 {
   internal_error_pure_virtual_called ();
 }
@@ -13096,12 +13086,12 @@ bkpt_insert_location (struct bp_location *bl)
 }
 
 static int
-bkpt_remove_location (struct bp_location *bl)
+bkpt_remove_location (struct bp_location *bl, enum remove_bp_reason reason)
 {
   if (bl->loc_type == bp_loc_hardware_breakpoint)
     return target_remove_hw_breakpoint (bl->gdbarch, &bl->target_info);
   else
-    return target_remove_breakpoint (bl->gdbarch, &bl->target_info);
+    return target_remove_breakpoint (bl->gdbarch, &bl->target_info, reason);
 }
 
 static int
@@ -13444,7 +13434,8 @@ bkpt_probe_insert_location (struct bp_location *bl)
 }
 
 static int
-bkpt_probe_remove_location (struct bp_location *bl)
+bkpt_probe_remove_location (struct bp_location *bl,
+			    enum remove_bp_reason reason)
 {
   /* Let's clear the semaphore before removing the location.  */
   if (bl->probe.probe->pops->clear_semaphore != NULL)
@@ -13452,7 +13443,7 @@ bkpt_probe_remove_location (struct bp_location *bl)
 					    bl->probe.objfile,
 					    bl->gdbarch);
 
-  return bkpt_remove_location (bl);
+  return bkpt_remove_location (bl, reason);
 }
 
 static void
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 054eab4d..c7d7462 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -39,6 +39,18 @@ struct linespec_result;
 struct linespec_sals;
 struct event_location;
 
+/* Why are we removing the breakpoint from the target?  */
+
+enum remove_bp_reason
+{
+  /* A regular remove.  Remove the breakpoint and forget everything
+     about it.  */
+  REMOVE_BREAKPOINT,
+
+  /* Detach the breakpoints from a fork child.  */
+  DETACH_BREAKPOINT,
+};
+
 /* This is the maximum number of bytes a breakpoint instruction can
    take.  Feel free to increase it.  It's just used in a few places to
    size arrays that should be independent of the target
@@ -519,7 +531,7 @@ struct breakpoint_ops
      with the "insert" method above.  Return 0 for success, 1 if the
      breakpoint, watchpoint or catchpoint type is not supported,
      -1 for failure.  */
-  int (*remove_location) (struct bp_location *);
+  int (*remove_location) (struct bp_location *, enum remove_bp_reason reason);
 
   /* Return true if it the target has stopped due to hitting
      breakpoint location BL.  This function does not check if we
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 224719e..376b7c9 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -920,6 +920,16 @@ ignore (struct target_ops *ops, struct gdbarch *gdbarch,
   return 0;
 }
 
+/* Implement the to_remove_breakpoint method.  */
+
+static int
+core_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
+			struct bp_target_info *bp_tgt,
+			enum remove_bp_reason reason)
+{
+  return 0;
+}
+
 
 /* Okay, let's be honest: threads gleaned from a core file aren't
    exactly lively, are they?  On the other hand, if we don't claim
@@ -1043,7 +1053,7 @@ init_core_ops (void)
   core_ops.to_xfer_partial = core_xfer_partial;
   core_ops.to_files_info = core_files_info;
   core_ops.to_insert_breakpoint = ignore;
-  core_ops.to_remove_breakpoint = ignore;
+  core_ops.to_remove_breakpoint = core_remove_breakpoint;
   core_ops.to_thread_alive = core_thread_alive;
   core_ops.to_read_description = core_read_description;
   core_ops.to_pid_to_str = core_pid_to_str;
diff --git a/gdb/exec.c b/gdb/exec.c
index 23c89c2..d7f42a2 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -1005,6 +1005,16 @@ ignore (struct target_ops *ops, struct gdbarch *gdbarch,
   return 0;
 }
 
+/* Implement the to_remove_breakpoint method.  */
+
+static int
+exec_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
+			struct bp_target_info *bp_tgt,
+			enum remove_bp_reason reason)
+{
+  return 0;
+}
+
 static int
 exec_has_memory (struct target_ops *ops)
 {
@@ -1036,7 +1046,7 @@ Specify the filename of the executable file.";
   exec_ops.to_get_section_table = exec_get_section_table;
   exec_ops.to_files_info = exec_files_info;
   exec_ops.to_insert_breakpoint = ignore;
-  exec_ops.to_remove_breakpoint = ignore;
+  exec_ops.to_remove_breakpoint = exec_remove_breakpoint;
   exec_ops.to_stratum = file_stratum;
   exec_ops.to_has_memory = exec_has_memory;
   exec_ops.to_make_corefile_notes = exec_make_note_section;
diff --git a/gdb/mem-break.c b/gdb/mem-break.c
index 9bb4c45..803f62b 100644
--- a/gdb/mem-break.c
+++ b/gdb/mem-break.c
@@ -93,7 +93,8 @@ memory_insert_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
 
 int
 memory_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
-			  struct bp_target_info *bp_tgt)
+			  struct bp_target_info *bp_tgt,
+			  enum remove_bp_reason reason)
 {
   return gdbarch_memory_remove_breakpoint (gdbarch, bp_tgt);
 }
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 992de29..cd715d2 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -1015,7 +1015,8 @@ procfs_insert_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
 
 static int
 procfs_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
-			  struct bp_target_info *bp_tgt)
+			  struct bp_target_info *bp_tgt,
+			  enum remove_bp_reason reason)
 {
   return procfs_breakpoint (bp_tgt->placed_address, _DEBUG_BREAK_EXEC, -1);
 }
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 77b5180..6a5ffa4 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1406,7 +1406,8 @@ record_btrace_insert_breakpoint (struct target_ops *ops,
 static int
 record_btrace_remove_breakpoint (struct target_ops *ops,
 				 struct gdbarch *gdbarch,
-				 struct bp_target_info *bp_tgt)
+				 struct bp_target_info *bp_tgt,
+				 enum remove_bp_reason reason)
 {
   const char *old;
   int ret;
@@ -1419,7 +1420,8 @@ record_btrace_remove_breakpoint (struct target_ops *ops,
   ret = 0;
   TRY
     {
-      ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, bp_tgt);
+      ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, bp_tgt,
+						reason);
     }
   CATCH (except, RETURN_MASK_ALL)
     {
diff --git a/gdb/record-full.c b/gdb/record-full.c
index f6023bf..418a346 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1684,7 +1684,8 @@ record_full_insert_breakpoint (struct target_ops *ops,
 static int
 record_full_remove_breakpoint (struct target_ops *ops,
 			       struct gdbarch *gdbarch,
-			       struct bp_target_info *bp_tgt)
+			       struct bp_target_info *bp_tgt,
+			       enum remove_bp_reason reason)
 {
   struct record_full_breakpoint *bp;
   int ix;
@@ -1704,7 +1705,7 @@ record_full_remove_breakpoint (struct target_ops *ops,
 
 	      old_cleanups = record_full_gdb_operation_disable_set ();
 	      ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch,
-							bp_tgt);
+							bp_tgt, reason);
 	      do_cleanups (old_cleanups);
 
 	      if (ret != 0)
@@ -2171,7 +2172,8 @@ record_full_core_insert_breakpoint (struct target_ops *ops,
 static int
 record_full_core_remove_breakpoint (struct target_ops *ops,
 				    struct gdbarch *gdbarch,
-				    struct bp_target_info *bp_tgt)
+				    struct bp_target_info *bp_tgt,
+				    enum remove_bp_reason reason)
 {
   return 0;
 }
diff --git a/gdb/remote-m32r-sdi.c b/gdb/remote-m32r-sdi.c
index c1bd858..25e51f0 100644
--- a/gdb/remote-m32r-sdi.c
+++ b/gdb/remote-m32r-sdi.c
@@ -1216,7 +1216,8 @@ m32r_insert_breakpoint (struct target_ops *ops,
 static int
 m32r_remove_breakpoint (struct target_ops *ops,
 			struct gdbarch *gdbarch,
-			struct bp_target_info *bp_tgt)
+			struct bp_target_info *bp_tgt,
+			enum remove_bp_reason reason)
 {
   CORE_ADDR addr = bp_tgt->placed_address;
   int i;
diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c
index 2f6663b..31aae9b 100644
--- a/gdb/remote-mips.c
+++ b/gdb/remote-mips.c
@@ -2387,13 +2387,14 @@ mips_insert_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
 
 static int
 mips_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
-			struct bp_target_info *bp_tgt)
+			struct bp_target_info *bp_tgt,
+			enum remove_bp_reason reason)
 {
   if (monitor_supports_breakpoints)
     return mips_clear_breakpoint (bp_tgt->placed_address, MIPS_INSN32_SIZE,
 				  BREAK_FETCH);
   else
-    return memory_remove_breakpoint (ops, gdbarch, bp_tgt);
+    return memory_remove_breakpoint (ops, gdbarch, bp_tgt, reason);
 }
 
 /* Tell whether this target can support a hardware breakpoint.  CNT
diff --git a/gdb/remote.c b/gdb/remote.c
index 6d56f19..404a5c6 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9339,7 +9339,8 @@ Target doesn't support breakpoints that have target side commands."));
 static int
 remote_remove_breakpoint (struct target_ops *ops,
 			  struct gdbarch *gdbarch,
-			  struct bp_target_info *bp_tgt)
+			  struct bp_target_info *bp_tgt,
+			  enum remove_bp_reason reason)
 {
   CORE_ADDR addr = bp_tgt->placed_address;
   struct remote_state *rs = get_remote_state ();
@@ -9368,7 +9369,7 @@ remote_remove_breakpoint (struct target_ops *ops,
       return (rs->buf[0] == 'E');
     }
 
-  return memory_remove_breakpoint (ops, gdbarch, bp_tgt);
+  return memory_remove_breakpoint (ops, gdbarch, bp_tgt, reason);
 }
 
 static enum Z_packet_type
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 5f44d11..ef7e14d 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -158,6 +158,8 @@
   target_debug_do_print (plongest (X))
 #define target_debug_print_struct_inferior_p(X)	\
   target_debug_do_print (host_address_to_string (X))
+#define target_debug_print_enum_remove_bp_reason(X) \
+  target_debug_do_print (plongest (X))
 
 static void
 target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index d23bc75..e21746f 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -262,24 +262,26 @@ debug_insert_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct b
 }
 
 static int
-delegate_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2)
+delegate_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2, enum remove_bp_reason arg3)
 {
   self = self->beneath;
-  return self->to_remove_breakpoint (self, arg1, arg2);
+  return self->to_remove_breakpoint (self, arg1, arg2, arg3);
 }
 
 static int
-debug_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2)
+debug_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2, enum remove_bp_reason arg3)
 {
   int result;
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_remove_breakpoint (...)\n", debug_target.to_shortname);
-  result = debug_target.to_remove_breakpoint (&debug_target, arg1, arg2);
+  result = debug_target.to_remove_breakpoint (&debug_target, arg1, arg2, arg3);
   fprintf_unfiltered (gdb_stdlog, "<- %s->to_remove_breakpoint (", debug_target.to_shortname);
   target_debug_print_struct_target_ops_p (&debug_target);
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_struct_gdbarch_p (arg1);
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_struct_bp_target_info_p (arg2);
+  fputs_unfiltered (", ", gdb_stdlog);
+  target_debug_print_enum_remove_bp_reason (arg3);
   fputs_unfiltered (") = ", gdb_stdlog);
   target_debug_print_int (result);
   fputs_unfiltered ("\n", gdb_stdlog);
diff --git a/gdb/target.c b/gdb/target.c
index ac66a3a..a0d5937 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2081,7 +2081,8 @@ target_insert_breakpoint (struct gdbarch *gdbarch,
 
 int
 target_remove_breakpoint (struct gdbarch *gdbarch,
-			  struct bp_target_info *bp_tgt)
+			  struct bp_target_info *bp_tgt,
+			  enum remove_bp_reason reason)
 {
   /* This is kind of a weird case to handle, but the permission might
      have been changed after breakpoints were inserted - in which case
@@ -2094,7 +2095,7 @@ target_remove_breakpoint (struct gdbarch *gdbarch,
     }
 
   return current_target.to_remove_breakpoint (&current_target,
-					      gdbarch, bp_tgt);
+					      gdbarch, bp_tgt, reason);
 }
 
 static void
diff --git a/gdb/target.h b/gdb/target.h
index e1419a9..7c8513a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -478,7 +478,8 @@ struct target_ops
 				 struct bp_target_info *)
       TARGET_DEFAULT_FUNC (memory_insert_breakpoint);
     int (*to_remove_breakpoint) (struct target_ops *, struct gdbarch *,
-				 struct bp_target_info *)
+				 struct bp_target_info *,
+				 enum remove_bp_reason)
       TARGET_DEFAULT_FUNC (memory_remove_breakpoint);
 
     /* Returns true if the target stopped because it executed a
@@ -1496,7 +1497,8 @@ extern int target_insert_breakpoint (struct gdbarch *gdbarch,
    machine.  Result is 0 for success, non-zero for error.  */
 
 extern int target_remove_breakpoint (struct gdbarch *gdbarch,
-				     struct bp_target_info *bp_tgt);
+				     struct bp_target_info *bp_tgt,
+				     enum remove_bp_reason reason);
 
 /* Returns true if the terminal settings of the inferior are in
    effect.  */
@@ -2356,7 +2358,8 @@ extern struct target_section_table *target_get_section_table
 /* From mem-break.c */
 
 extern int memory_remove_breakpoint (struct target_ops *, struct gdbarch *,
-				     struct bp_target_info *);
+				     struct bp_target_info *,
+				     enum remove_bp_reason);
 
 extern int memory_insert_breakpoint (struct target_ops *, struct gdbarch *,
 				     struct bp_target_info *);
-- 
1.9.3

Re: [PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error)

[Pedro Alves]

Yao reminded me about this series, and asked about putting it in 7.12.

I've done so now: pushed to master and 7.12 branch.

Thanks,
Pedro Alves

On 02/14/2016 06:37 PM, Pedro Alves wrote:
> This series fixes:
> 
>  (gdb) record
>  break marker2
>  Breakpoint 2 at 0x40067a: file /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.reverse/waitpid-reverse.c, line 30.
>  (gdb) PASS: gdb.reverse/waitpid-reverse.exp: set breakpoint at marker2
>  continue
>  Continuing.
>  /home/pedro/gdb/mygit/src/gdb/record-full.c:1720: internal-error: record_full_remove_breakpoint: removing unknown breakpoint
>  A problem internal to GDB has been detected,
>  further debugging may prove unreliable.
>  Quit this debugging session? (y or n)
>  KFAIL: gdb.reverse/waitpid-reverse.exp: continue to breakpoint: marker2 (GDB internal error) (PRMS: gdb/19187)
> 
> The problem is that the target side can't distinguish between deleting
> a breakpoint and detaching a breakpoint.
> 
> Right after a fork is detected, we detach breakpoints from the child
> (detach_breakpoints), which calls into target_remove_breakpoint with
> inferior_ptid pointing at the child process, but leaves the breakpoint
> marked inserted (in the parent).
> 
> Since record-full.c's target_remove_breakpoint implementation doesn't
> know we're detaching a breakpoint, it removes the breakpoint from its
> own breakpoint table.
> 
> Then when we later remove the breakpoint from the parent, we fail that
> assertion, since the breakpoint is unexpectedly not found in the
> record-full.c breakpoint table.
> 
> The first three patches pass down the reason for removing the
> breakpoint all the way to the target.
> 
> Patch 4 then simply makes record-full.c not delete the breakpoint from
> its breakpoint table when we're detaching the breakpoint from a fork
> child.
> 
> Pedro Alves (4):
>   Simplify remove_breakpoint interface
>   Introduce 'enum remove_bp_reason'
>   Plumb enum remove_bp_reason all the way to target_remove_breakpoint
>   Fix PR gdb/19187 (process record over a fork causes internal error)
> 
>  gdb/break-catch-sig.c                         |  3 +-
>  gdb/break-catch-syscall.c                     |  2 +-
>  gdb/breakpoint.c                              | 70 +++++++++++++--------------
>  gdb/breakpoint.h                              | 14 +++++-
>  gdb/corelow.c                                 | 10 +++-
>  gdb/exec.c                                    | 10 +++-
>  gdb/mem-break.c                               |  3 +-
>  gdb/record-btrace.c                           |  6 ++-
>  gdb/record-full.c                             | 15 ++++--
>  gdb/remote.c                                  |  5 +-
>  gdb/target-debug.h                            |  2 +
>  gdb/target-delegates.c                        | 10 ++--
>  gdb/target.c                                  |  5 +-
>  gdb/target.h                                  |  9 ++--
>  gdb/testsuite/gdb.reverse/waitpid-reverse.exp |  4 +-
>  15 files changed, 105 insertions(+), 63 deletions(-)
>