[rfc] Infrastructure to disable breakpoints during inferior startup

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Hello,

in some cases, it can happen that the main executable of a process is
visible at different addresses once the inferior is running, as compared
to the addresses in the original objfile.  This is a problem for the Cell
combined debugger when running stand-alone SPU executables; but it should
also be a problem e.g. when running PIE executable on Linux.

In particular, there causes a problem with breakpoints.  GDB assumes that
whenever a process is running that all breakpoints (not in shared libraries)
can be inserted at their original addresses.  If a breakpoint was originally
set before the inferior was started, this address was computed using the
original objfile addresses -- but the breakpoint cannot be inserted there.

At some point during inferior startup, the relocation of the main
executable file is completed and reflected by GDB's solib handler,
causing the objfile to be relocated and breakpoint addresses to be
recomputed.  At this point, breakpoints can be installed without problem.

However, in the meantime we simply cannot insert breakpoints into the
inferior; every attempt to do so will fail, causing inferior startup
overall to fail.

The following patch attempts to address this by temporarily disabling all
user-installed breakpoints during the inferior startup phase, using a
mechanism similar to disable_watchpoints_before_interactive_call_start.

A new pair of functions:
  disable_breakpoints_before_startup
  enable_breakpoints_after_startup
is provided for this purpose.  The intent is that those functions are
called from a solib handler to disable breakpoints in the solib's
create_inferior hook, and re-enable them (e.g. in the current_sos hook)
once the inferior is ready and the final address of the main objfile
is known.

The disable function will bring all breakpoints into a new
bp_startup_disabled state, that is pretty much handled the same
as bp_call_disables is today.  In addition, a flag is set that
causes all newly user-created breakpoints in the period between
the disable and enable calls to start out in that same state.
(This is necessary because in some circumstances, e.g. when using
the remote target where you start out attached to the inferior at
the entry point, user interaction is possible in between those two
points in time.)

However, internal breakpoints are not affected (because e.g. solib
breakpoints must be installed in order for the solib handler to
regain control).  It's up to those GDB subsystems to handler such
internal breakpoints as necessary.

Tested on powerpc64-linux with no regression (note that the patch
is a no-op as is, because nobody calls the new functions).  Also
tested together with the Cell combined debugger patch, where the
functions *are* called and fix SPU stand-alone executable debugging.

At this point, the patch does not attempt to support PIE executables.
However, the infrastructure provided here should enable further
patches to provide that support on top.

Any comments on this approach -or other suggestions- are welcome.
If there are no objections, I'm planning to commit the patch
within a week or so.

Bye,
Ulrich


ChangeLog:

	* breakpoint.h (enum enable_state): Add bp_startup_disabled.
	(disable_breakpoints_before_startup): Add prototype.
	(enable_breakpoints_after_startup): Likewise.

	* breakpoint.c (executing_startup): New static variable.
	(describe_other_breakpoints): Handle bp_startup_disabled.
	(check_duplicates_for): Likewise.
	(disable_breakpoints_before_startup): New function.
	(enable_breakpoints_after_startup): New function.
	(create_breakpoint): Mark new breakpoints as bp_startup_disabled
	if executing_startup flag is true.
	(break_command_really): Likewise.
	(breakpoint_re_set_one): Skip bp_startup_disabled breakpoints.


Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c
+++ src/gdb/breakpoint.c
@@ -319,6 +319,9 @@ static int executing_breakpoint_commands
 /* Are overlay event breakpoints enabled? */
 static int overlay_events_enabled;
 
+/* Are we executing startup code?  */
+static int executing_startup;
+
 /* Walk the following statement or block through all breakpoints.
    ALL_BREAKPOINTS_SAFE does so even if the statment deletes the current
    breakpoint.  */
@@ -4140,7 +4143,8 @@ describe_other_breakpoints (struct gdbar
 	      printf_filtered (" (thread %d)", b->thread);
 	    printf_filtered ("%s%s ",
 			     ((b->enable_state == bp_disabled
-			       || b->enable_state == bp_call_disabled) 
+			       || b->enable_state == bp_call_disabled
+			       || b->enable_state == bp_startup_disabled)
 			      ? " (disabled)"
 			      : b->enable_state == bp_permanent 
 			      ? " (permanent)"
@@ -4211,6 +4215,7 @@ check_duplicates_for (CORE_ADDR address,
   ALL_BP_LOCATIONS (b)
     if (b->owner->enable_state != bp_disabled
 	&& b->owner->enable_state != bp_call_disabled
+	&& b->owner->enable_state != bp_startup_disabled
 	&& b->enabled
 	&& !b->shlib_disabled
 	&& b->address == address	/* address / overlay match */
@@ -4247,6 +4252,7 @@ check_duplicates_for (CORE_ADDR address,
 	    if (b->owner->enable_state != bp_permanent
 		&& b->owner->enable_state != bp_disabled
 		&& b->owner->enable_state != bp_call_disabled
+		&& b->owner->enable_state != bp_startup_disabled
 		&& b->enabled && !b->shlib_disabled		
 		&& b->address == address	/* address / overlay match */
 		&& (!overlay_debugging || b->section == section)
@@ -5095,6 +5101,52 @@ enable_watchpoints_after_interactive_cal
   }
 }
 
+void
+disable_breakpoints_before_startup (void)
+{
+  struct breakpoint *b;
+  int found = 0;
+
+  ALL_BREAKPOINTS (b)
+    {
+      if ((b->type == bp_breakpoint
+	   || b->type == bp_hardware_breakpoint)
+	  && breakpoint_enabled (b))
+	{
+	  b->enable_state = bp_startup_disabled;
+	  found = 1;
+	}
+    }
+
+  if (found)
+    update_global_location_list (0);
+
+  executing_startup = 1;
+}
+
+void
+enable_breakpoints_after_startup (void)
+{
+  struct breakpoint *b;
+  int found = 0;
+
+  executing_startup = 0;
+
+  ALL_BREAKPOINTS (b)
+    {
+      if ((b->type == bp_breakpoint
+	   || b->type == bp_hardware_breakpoint)
+	  && b->enable_state == bp_startup_disabled)
+	{
+	  b->enable_state = bp_enabled;
+	  found = 1;
+	}
+    }
+
+  if (found)
+    breakpoint_re_set ();
+}
+
 
 /* Set a breakpoint that will evaporate an end of command
    at address specified by SAL.
@@ -5438,6 +5490,11 @@ create_breakpoint (struct gdbarch *gdbar
 	  b->enable_state = enabled ? bp_enabled : bp_disabled;
 	  b->disposition = disposition;
 
+	  if (enabled && executing_startup
+	      && (b->type == bp_breakpoint
+		  || b->type == bp_hardware_breakpoint))
+	    b->enable_state = bp_startup_disabled;
+
 	  loc = b->loc;
 	}
       else
@@ -5993,6 +6050,11 @@ break_command_really (struct gdbarch *gd
       b->ops = ops;
       b->enable_state = enabled ? bp_enabled : bp_disabled;
 
+      if (enabled && executing_startup
+	  && (b->type == bp_breakpoint
+	      || b->type == bp_hardware_breakpoint))
+	b->enable_state = bp_startup_disabled;
+
       mention (b);
     }
   
@@ -7792,6 +7854,10 @@ breakpoint_re_set_one (void *bint)
     case bp_breakpoint:
     case bp_hardware_breakpoint:
     case bp_tracepoint:
+      /* Do not attempt to re-set breakpoints disabled during startup.  */
+      if (b->enable_state == bp_startup_disabled)
+	return 0;
+
       if (b->addr_string == NULL)
 	{
 	  /* Anything without a string can't be re-set. */
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h
+++ src/gdb/breakpoint.h
@@ -135,6 +135,12 @@ enum enable_state
 			   automatically enabled and reset when the call 
 			   "lands" (either completes, or stops at another 
 			   eventpoint). */
+    bp_startup_disabled,/* The eventpoint has been disabled during inferior
+			   startup.  This is necessary on some targets where
+			   the main executable will get relocated during
+			   startup, making breakpoint addresses invalid.
+			   The eventpoint will be automatically enabled and
+			   reset once inferior startup is complete.  */
     bp_permanent	/* There is a breakpoint instruction hard-wired into
 			   the target's code.  Don't try to write another
 			   breakpoint instruction on top of it, or restore
@@ -810,6 +816,19 @@ extern void disable_watchpoints_before_i
 
 extern void enable_watchpoints_after_interactive_call_stop (void);
 
+/* These functions disable and re-enable all breakpoints during
+   inferior startup.  They are intended to be called from solib
+   code where necessary.  This is needed on platforms where the
+   main executable is relocated at some point during startup
+   processing, making breakpoint addresses invalid.
+
+   If additional breakpoints are created after the routine
+   disable_breakpoints_before_startup but before the routine
+   enable_breakpoints_after_startup was called, they will also
+   be marked as disabled.  */
+extern void disable_breakpoints_before_startup (void);
+extern void enable_breakpoints_after_startup (void);
+
 /* For script interpreters that need to define breakpoint commands
    after they've already read the commands into a struct command_line.  */
 extern enum command_control_type commands_from_control_command
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com