This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: don't set the pspace on ordinary breakpoints


>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> I *think* the should_be_inserted change is all that was really needed,
Tom> after re-reading all the messages in this thread.  Let me know what you
Tom> think.

Here is a refresh of this patch.  This one is relative to CVS HEAD.
I didn't attempt to test it in isolation.

Tom

2011-10-28  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.

>From 0315321273c5dad6ec98744439c72e1e00729be7 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tromey@redhat.com>
Date: Tue, 8 Nov 2011 13:50:12 -0700
Subject: [PATCH 1/2] remove bp_startup_disabled

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

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8c98bef..8ab09ba 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1568,6 +1568,9 @@ should_be_inserted (struct bp_location *bl)
   if (!bl->enabled || bl->shlib_disabled || bl->duplicate)
     return 0;
 
+  if (user_breakpoint_p (bl->owner) && 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
@@ -5313,8 +5316,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)"
@@ -5790,9 +5792,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;
@@ -6933,48 +6937,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;
 	}
     }
 
@@ -7215,7 +7219,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)
 	    {
@@ -7256,11 +7259,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
@@ -7980,14 +7978,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.  */
@@ -10607,13 +10601,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.  */
@@ -10891,10 +10880,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)
     {
@@ -11825,6 +11810,7 @@ addr_string_to_sals (struct breakpoint *b, char *addr_string, int *found)
       if (e.error == NOT_FOUND_ERROR
 	  && (b->condition_not_parsed 
 	      || (b->loc && b->loc->shlib_disabled)
+	      || (b->loc && b->loc->pspace->executing_startup)
 	      || b->enable_state == bp_disabled))
 	not_found_and_ok = 1;
 
@@ -11913,7 +11899,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 fe381df..94e324a 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]