RFC: don't set the pspace on ordinary breakpoints

[Tom Tromey <tromey at redhat dot com>]

I would appreciate comments on this patch.

This is the second of two patches in preparation for my "ambiguous
linespec" patch.  I think they are reasonably independent so I am
sending them separately.


This patch does two things.

First, it changes ordinary breakpoints to set leave their 'pspace' field
NULL.  The rationale for this is that, with the ambiguous linespec
patch, a linespec may apply to different program spaces, not just the
one which was selected when the breakpoint was created.  For these
breakpoints, we do not want breakpoint_program_space_exit to remove the
breakpoint.

I looked at removing the pspace field entirely.  Internal breakpoints
often set this field, though, and I chose not to touch all this code in
hopes of keeping the patch down to a reasonable size.  So, I left the
field and added a special meaning for NULL.


Second, this patch removes bp_startup_disabled.  Instead, I changed
should_be_inserted to directly examine the location's pspace, and I
changed one spot (update_global_location_list) to use should_be_inserted
to pick this up.

Built and regtested on x86-64 F15, but only when applied on top of the
previous patch.

Tom

2011-10-26  Tom Tromey  <tromey@redhat.com>

	* elfread.c (elf_gnu_ifunc_resolver_return_stop): Allow
	breakpoint's pspace to be NULL.
	* breakpoint.h (enum enable_state) <bp_startup_disabled>: Remove.
	(struct breakpoint) <pspace>: Update comment.
	* breakpoint.c (should_be_inserted): Explicitly check if program
	space is executing startup.
	(describe_other_breakpoints): Update.
	(init_raw_breakpoint): Conditionally set breakpoint's pspace.
	(disable_breakpoints_before_startup): Change executing_startup
	earlier.  Use location's pspace.
	(enable_breakpoints_after_startup): Likewise.
	(init_breakpoint_sal): Don't set breakpoint's pspace.  Don't use
	bp_startup_disabled.
	(create_breakpoint): Conditionally set breakpoint's pspace.  Don't
	use bp_startup_disabled.
	(update_global_location_list): Use should_be_inserted.
	(bkpt_re_set): Update.
	(prepare_re_set_context): Conditionally switch program space.

2011-10-26  Tom Tromey  <tromey@redhat.com>

>From b7cf18b3ee2b13ef0855fd196fa242d238b03949 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tromey@redhat.com>
Date: Fri, 7 Oct 2011 10:56:00 -0600
Subject: [PATCH 2/2] don't set the pspace on ordinary breakpoints also remove
 bp_startup_disabled

---
 gdb/ChangeLog    |   21 +++++++++++++++
 gdb/breakpoint.c |   76 ++++++++++++++++++++++--------------------------------
 gdb/breakpoint.h |   12 ++------
 gdb/elfread.c    |    2 +-
 4 files changed, 56 insertions(+), 55 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3ae7508..1c6a43b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1578,6 +1578,9 @@ should_be_inserted (struct bp_location *bl)
   if (!bl->enabled || bl->shlib_disabled || bl->duplicate)
     return 0;
 
+  if (bl->pspace->executing_startup)
+    return 0;
+
   /* This is set for example, when we're attached to the parent of a
      vfork, and have detached from the child.  The child is running
      free, and we expect it to do an exec or exit, at which point the
@@ -5323,8 +5326,7 @@ describe_other_breakpoints (struct gdbarch *gdbarch,
 	      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_startup_disabled)
+			       || b->enable_state == bp_call_disabled)
 			      ? " (disabled)"
 			      : b->enable_state == bp_permanent 
 			      ? " (permanent)"
@@ -5817,9 +5819,11 @@ init_raw_breakpoint (struct breakpoint *b, struct gdbarch *gdbarch,
   b->loc->address = adjusted_address;
   b->loc->pspace = sal.pspace;
 
-  /* Store the program space that was used to set the breakpoint, for
-     breakpoint resetting.  */
-  b->pspace = sal.pspace;
+  /* Store the program space that was used to set the breakpoint,
+     except for ordinary breakpoints, which are independent of the
+     program space.  */
+  if (bptype != bp_breakpoint && bptype != bp_hardware_breakpoint)
+    b->pspace = sal.pspace;
 
   if (sal.symtab == NULL)
     b->source_file = NULL;
@@ -6960,48 +6964,48 @@ enable_watchpoints_after_interactive_call_stop (void)
 void
 disable_breakpoints_before_startup (void)
 {
-  struct breakpoint *b;
+  struct bp_location *loc, **locp_tmp;
   int found = 0;
 
-  ALL_BREAKPOINTS (b)
+  current_program_space->executing_startup = 1;
+
+  ALL_BP_LOCATIONS (loc, locp_tmp)
     {
-      if (b->pspace != current_program_space)
+      if (loc->pspace != current_program_space)
 	continue;
 
-      if ((b->type == bp_breakpoint
-	   || b->type == bp_hardware_breakpoint)
-	  && breakpoint_enabled (b))
+      if ((loc->owner->type == bp_breakpoint
+	   || loc->owner->type == bp_hardware_breakpoint)
+	  && breakpoint_enabled (loc->owner))
 	{
-	  b->enable_state = bp_startup_disabled;
 	  found = 1;
+	  break;
 	}
     }
 
   if (found)
     update_global_location_list (0);
-
-  current_program_space->executing_startup = 1;
 }
 
 void
 enable_breakpoints_after_startup (void)
 {
-  struct breakpoint *b;
+  struct bp_location *loc, **locp_tmp;
   int found = 0;
 
   current_program_space->executing_startup = 0;
 
-  ALL_BREAKPOINTS (b)
+  ALL_BP_LOCATIONS (loc, locp_tmp)
     {
-      if (b->pspace != current_program_space)
+      if (loc->pspace != current_program_space)
 	continue;
 
-      if ((b->type == bp_breakpoint
-	   || b->type == bp_hardware_breakpoint)
-	  && b->enable_state == bp_startup_disabled)
+      if ((loc->owner->type == bp_breakpoint
+	   || loc->owner->type == bp_hardware_breakpoint)
+	  && breakpoint_enabled (loc->owner))
 	{
-	  b->enable_state = bp_enabled;
 	  found = 1;
+	  break;
 	}
     }
 
@@ -7241,7 +7245,6 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 	  b->ignore_count = ignore_count;
 	  b->enable_state = enabled ? bp_enabled : bp_disabled;
 	  b->disposition = disposition;
-	  b->pspace = sals.sals[0].pspace;
 
 	  if (type == bp_static_tracepoint)
 	    {
@@ -7282,11 +7285,6 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 			   "tracepoint marker to probe"));
 	    }
 
-	  if (enabled && b->pspace->executing_startup
-	      && (b->type == bp_breakpoint
-		  || b->type == bp_hardware_breakpoint))
-	    b->enable_state = bp_startup_disabled;
-
 	  loc = b->loc;
 	}
       else
@@ -8001,14 +7999,10 @@ create_breakpoint (struct gdbarch *gdbarch,
       b->disposition = tempflag ? disp_del : disp_donttouch;
       b->condition_not_parsed = 1;
       b->enable_state = enabled ? bp_enabled : bp_disabled;
-      b->pspace = current_program_space;
+      if (type_wanted != bp_breakpoint && type_wanted != bp_hardware_breakpoint)
+	b->pspace = current_program_space;
       b->py_bp_object = NULL;
 
-      if (enabled && b->pspace->executing_startup
-	  && (b->type == bp_breakpoint
-	      || b->type == bp_hardware_breakpoint))
-	b->enable_state = bp_startup_disabled;
-
       if (!internal)
         /* Do not mention breakpoints with a negative number, 
 	   but do notify observers.  */
@@ -10625,13 +10619,8 @@ update_global_location_list (int should_insert)
       struct breakpoint *b = loc->owner;
       struct bp_location **loc_first_p;
 
-      if (b->enable_state == bp_disabled
-	  || b->enable_state == bp_call_disabled
-	  || b->enable_state == bp_startup_disabled
-	  || !loc->enabled
-	  || loc->shlib_disabled
-	  || !breakpoint_address_is_meaningful (b)
-	  || is_tracepoint (b))
+      if (!should_be_inserted (loc)
+	  || !breakpoint_address_is_meaningful (b))
 	continue;
 
       /* Permanent breakpoint should always be inserted.  */
@@ -10909,10 +10898,6 @@ static struct breakpoint_ops base_breakpoint_ops =
 static void
 bkpt_re_set (struct breakpoint *b)
 {
-  /* Do not attempt to re-set breakpoints disabled during startup.  */
-  if (b->enable_state == bp_startup_disabled)
-    return;
-
   /* FIXME: is this still reachable?  */
   if (b->addr_string == NULL)
     {
@@ -11931,7 +11916,8 @@ prepare_re_set_context (struct breakpoint *b)
 
   input_radix = b->input_radix;
   cleanups = save_current_space_and_thread ();
-  switch_to_program_space_and_thread (b->pspace);
+  if (b->pspace != NULL)
+    switch_to_program_space_and_thread (b->pspace);
   set_language (b->language);
 
   return cleanups;
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index dba5392..e038999 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -186,14 +186,6 @@ 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
@@ -571,7 +563,9 @@ struct breakpoint
        equals this.  */
     struct frame_id frame_id;
 
-    /* The program space used to set the breakpoint.  */
+    /* The program space used to set the breakpoint.  This is only set
+       for breakpoints which are specific to a program space; for
+       ordinary breakpoints this is NULL.  */
     struct program_space *pspace;
 
     /* String we used to set the breakpoint (malloc'd).  */
diff --git a/gdb/elfread.c b/gdb/elfread.c
index a309a2c..067c77f 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1032,7 +1032,7 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
     }
   gdb_assert (b->type == bp_gnu_ifunc_resolver);
 
-  gdb_assert (current_program_space == b->pspace);
+  gdb_assert (current_program_space == b->pspace || b->pspace == NULL);
   elf_gnu_ifunc_record_cache (b->addr_string, resolved_pc);
 
   sal = find_pc_line (resolved_pc, 0);
-- 
1.7.6.4