Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
@ 2003-10-08 17:02 Daniel Jacobowitz
  2003-10-08 18:17 ` Jim Blandy
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2003-10-08 17:02 UTC (permalink / raw)
  To: gdb-patches

This patch adds a struct impl_breakpoint and the corresponding enum
impl_bptype.  Basic impl data is moved from the breakpoint to the
impl_breakpoint: address, shadow contents for sw breakpoints, inserted and
duplicate flags.  Then I mechanically update all of breakpoint.c to use the
new members.  No surprises here; everything in impl_breakpoint is
initialized just when it would have been before the move, except for the new
type field, which will be taken care of shortly.  The impl_breakpoint is
allocated and deallocated at the same time as the breakpoint.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-10-08  Daniel Jacobowitz  <drow@mvista.com>

	* breakpoint.h (enum impl_bptype, struct impl_breakpoint): New.
	(struct breakpoint): Remove address, shadow_contents, inserted,
	and duplicate.  Add a struct impl_breakpoint.
	* breakpoint.c (condition_command, read_memory_nobpt)
	(insert_breakpoints, remove_breakpoints, remove_hw_watchpoints)
	(reattach_breakpoints, update_breakpoints_after_exec)
	(detach_breakpoints, remove_breakpoint, mark_breakpoints_out)
	(breakpoint_init_inferior, breakpoint_here_p)
	(breakpoint_inserted_here_p, deprecated_frame_in_dummy)
	(breakpoint_thread_match, bpstat_stop_status)
	(bpstat_have_active_hw_watchpoints, print_one_breakpoint)
	(describe_other_breakpoints, check_duplicates)
	(make_breakpoint_permanent, create_thread_event_breakpoint)
	(disable_breakpoints_in_shlibs, re_enable_berakpoints_in_shlibs)
	(set_longjmp_resume_breakpoint, mention, create_breakpoints)
	(watch_command_1, print_one_exception_catchpoint)
	(clear_command, breakpoint_re_set_one): Adjust member accesses to
	use the breakpoint's ->impl.
	(set_raw_breakpoint): Likewise.  Initialize ->impl.
	(delete_breakpoint): Likewise.  Free ->impl.

Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2003-10-08 12:42:09.000000000 -0400
+++ gdb/breakpoint.c	2003-10-08 12:42:09.000000000 -0400
@@ -548,7 +548,7 @@ condition_command (char *arg, int from_t
 	  /* I don't know if it matters whether this is the string the user
 	     typed in or the decompiled expression.  */
 	  b->cond_string = savestring (arg, strlen (arg));
-	  b->cond = parse_exp_1 (&arg, block_for_pc (b->address), 0);
+	  b->cond = parse_exp_1 (&arg, block_for_pc (b->impl->address), 0);
 	  if (*arg)
 	    error ("Junk at end of expression");
 	}
@@ -631,7 +631,7 @@ read_memory_nobpt (CORE_ADDR memaddr, ch
 	|| b->type == bp_access_watchpoint)
       continue;
     /* bp in memory? */
-    if (!b->inserted)
+    if (!b->impl->inserted)
       continue;
     /* Addresses and length of the part of the breakpoint that
        we need to copy.  */
@@ -639,7 +639,7 @@ read_memory_nobpt (CORE_ADDR memaddr, ch
        breakpoint values.  BREAKPOINT_FROM_PC still manages to
        correctly determine the breakpoints memory address and size
        for these targets. */
-    bp_addr = b->address;
+    bp_addr = b->impl->address;
     bp_size = 0;
     if (BREAKPOINT_FROM_PC (&bp_addr, &bp_size) == NULL)
       continue;
@@ -675,7 +675,7 @@ read_memory_nobpt (CORE_ADDR memaddr, ch
 	}
 
       memcpy (myaddr + bp_addr - memaddr,
-	      b->shadow_contents + bptoffset, bp_size);
+	      b->impl->shadow_contents + bptoffset, bp_size);
 
       if (bp_addr > memaddr)
 	{
@@ -783,8 +783,8 @@ insert_breakpoints (void)
 	&& b->type != bp_catch_exec
 	&& b->type != bp_catch_throw
 	&& b->type != bp_catch_catch
-	&& !b->inserted
-	&& !b->duplicate)
+	&& !b->impl->inserted
+	&& !b->impl->duplicate)
       {
 	/* "Normal" instruction breakpoint: either the standard
 	   trap-instruction bp (bp_breakpoint), or a
@@ -798,10 +798,10 @@ insert_breakpoints (void)
 	    /* No overlay handling: just set the breakpoint.  */
 
 	    if (b->type == bp_hardware_breakpoint)
-	      val = target_insert_hw_breakpoint (b->address, 
-						 b->shadow_contents);
+	      val = target_insert_hw_breakpoint (b->impl->address, 
+						 b->impl->shadow_contents);
 	    else
-	      val = target_insert_breakpoint (b->address, b->shadow_contents);
+	      val = target_insert_breakpoint (b->impl->address, b->impl->shadow_contents);
 	  }
 	else
 	  {
@@ -817,10 +817,10 @@ insert_breakpoints (void)
 			   b->number);
 		else
 		  {
-		    CORE_ADDR addr = overlay_unmapped_address (b->address, 
+		    CORE_ADDR addr = overlay_unmapped_address (b->impl->address, 
 							       b->section);
 		    /* Set a software (trap) breakpoint at the LMA.  */
-		    val = target_insert_breakpoint (addr, b->shadow_contents);
+		    val = target_insert_breakpoint (addr, b->impl->shadow_contents);
 		    if (val != 0)
 		      fprintf_unfiltered (tmp_error_stream, 
 					  "Overlay breakpoint %d failed: in ROM?", 
@@ -832,11 +832,11 @@ insert_breakpoints (void)
 	      {
 		/* Yes.  This overlay section is mapped into memory.  */
 		if (b->type == bp_hardware_breakpoint)
-		  val = target_insert_hw_breakpoint (b->address, 
-						     b->shadow_contents);
+		  val = target_insert_hw_breakpoint (b->impl->address, 
+						     b->impl->shadow_contents);
 		else
-		  val = target_insert_breakpoint (b->address,
-						  b->shadow_contents);
+		  val = target_insert_breakpoint (b->impl->address,
+						  b->impl->shadow_contents);
 	      }
 	    else
 	      {
@@ -850,7 +850,7 @@ insert_breakpoints (void)
 	  {
 	    /* Can't set the breakpoint.  */
 #if defined (DISABLE_UNSETTABLE_BREAK)
-	    if (DISABLE_UNSETTABLE_BREAK (b->address))
+	    if (DISABLE_UNSETTABLE_BREAK (b->impl->address))
 	      {
 		/* See also: disable_breakpoints_in_shlibs. */
 		val = 0;
@@ -887,7 +887,7 @@ insert_breakpoints (void)
 					b->number);
 		    fprintf_filtered (tmp_error_stream, 
 				      "Error accessing memory address ");
-		    print_address_numeric (b->address, 1, tmp_error_stream);
+		    print_address_numeric (b->impl->address, 1, tmp_error_stream);
 		    fprintf_filtered (tmp_error_stream, ": %s.\n",
 				      safe_strerror (val));
 		  }
@@ -895,20 +895,20 @@ insert_breakpoints (void)
 	      }
 	  }
 	else
-	  b->inserted = 1;
+	  b->impl->inserted = 1;
 
 	if (val)
 	  return_val = val;	/* remember failure */
       }
     else if (ep_is_exception_catchpoint (b)
-	     && !b->inserted
-	     && !b->duplicate)
+	     && !b->impl->inserted
+	     && !b->impl->duplicate)
 
       {
 	/* If we get here, we must have a callback mechanism for exception
 	   events -- with g++ style embedded label support, we insert
 	   ordinary breakpoints and not catchpoints. */
-	val = target_insert_breakpoint (b->address, b->shadow_contents);
+	val = target_insert_breakpoint (b->impl->address, b->impl->shadow_contents);
 	if (val)
 	  {
 	    /* Couldn't set breakpoint for some reason */
@@ -917,7 +917,7 @@ insert_breakpoints (void)
 				b->number);
 	    fprintf_filtered (tmp_error_stream, 
 			      "Error accessing memory address ");
-	    print_address_numeric (b->address, 1, tmp_error_stream);
+	    print_address_numeric (b->impl->address, 1, tmp_error_stream);
 	    fprintf_filtered (tmp_error_stream, ": %s.\n",
 			      safe_strerror (val));
 	    b->enable_state = bp_disabled;
@@ -939,7 +939,7 @@ insert_breakpoints (void)
 	    do_cleanups (cleanups);
 	    if (val != 0 && val != -1)
 	      {
-		b->inserted = 1;
+		b->impl->inserted = 1;
 	      }
 	    /* Check if something went wrong; val == 0 can be ignored */
 	    if (val == -1)
@@ -960,8 +960,8 @@ insert_breakpoints (void)
 	      b->type == bp_read_watchpoint ||
 	      b->type == bp_access_watchpoint)
 	     && b->disposition != disp_del_at_next_stop
-	     && !b->inserted
-	     && !b->duplicate)
+	     && !b->impl->inserted
+	     && !b->impl->duplicate)
       {
 	struct frame_info *saved_frame;
 	int saved_level, within_current_scope;
@@ -998,7 +998,7 @@ insert_breakpoints (void)
 	    value_release_to_mark (mark);
 
 	    b->val_chain = v;
-	    b->inserted = 1;
+	    b->impl->inserted = 1;
 
 	    /* Look at each value on the value chain.  */
 	    for (; v; v = v->next)
@@ -1038,7 +1038,7 @@ insert_breakpoints (void)
 			       watches below, and removing a
 			       watchpoint we didn't insert could have
 			       adverse effects.  */
-			    b->inserted = 0;
+			    b->impl->inserted = 0;
 			  }
 			val = 0;
 		      }
@@ -1046,7 +1046,7 @@ insert_breakpoints (void)
 	      }
 	    /* Failure to insert a watchpoint on any memory value in the
 	       value chain brings us here.  */
-	    if (!b->inserted)
+	    if (!b->impl->inserted)
 	      {
 		remove_breakpoint (b, mark_uninserted);
 		hw_breakpoint_error = 1;
@@ -1077,8 +1077,8 @@ insert_breakpoints (void)
     else if ((b->type == bp_catch_fork
 	      || b->type == bp_catch_vfork
 	      || b->type == bp_catch_exec)
-	     && !b->inserted
-	     && !b->duplicate)
+	     && !b->impl->inserted
+	     && !b->impl->duplicate)
       {
 	char *prefix = xstrprintf ("warning: inserting catchpoint %d: ",
 				   b->number);
@@ -1089,7 +1089,7 @@ insert_breakpoints (void)
 	if (val < 0)
 	  b->enable_state = bp_disabled;
 	else
-	  b->inserted = 1;
+	  b->impl->inserted = 1;
       }
   }
   
@@ -1122,7 +1122,7 @@ remove_breakpoints (void)
 
   ALL_BREAKPOINTS (b)
   {
-    if (b->inserted)
+    if (b->impl->inserted)
       {
 	val = remove_breakpoint (b, mark_uninserted);
 	if (val != 0)
@@ -1140,7 +1140,7 @@ remove_hw_watchpoints (void)
 
   ALL_BREAKPOINTS (b)
   {
-    if (b->inserted
+    if (b->impl->inserted
 	&& (b->type == bp_hardware_watchpoint
 	    || b->type == bp_read_watchpoint
 	    || b->type == bp_access_watchpoint))
@@ -1164,13 +1164,13 @@ reattach_breakpoints (int pid)
   inferior_ptid = pid_to_ptid (pid);
   ALL_BREAKPOINTS (b)
   {
-    if (b->inserted)
+    if (b->impl->inserted)
       {
 	remove_breakpoint (b, mark_inserted);
 	if (b->type == bp_hardware_breakpoint)
-	  val = target_insert_hw_breakpoint (b->address, b->shadow_contents);
+	  val = target_insert_hw_breakpoint (b->impl->address, b->impl->shadow_contents);
 	else
-	  val = target_insert_breakpoint (b->address, b->shadow_contents);
+	  val = target_insert_breakpoint (b->impl->address, b->impl->shadow_contents);
 	if (val != 0)
 	  {
 	    do_cleanups (old_chain);
@@ -1257,7 +1257,7 @@ update_breakpoints_after_exec (void)
 	(b->type == bp_catch_vfork) ||
 	(b->type == bp_catch_fork))
       {
-	b->address = (CORE_ADDR) NULL;
+	b->impl->address = (CORE_ADDR) NULL;
 	continue;
       }
 
@@ -1310,7 +1310,7 @@ update_breakpoints_after_exec (void)
        unnecessary.  A call to breakpoint_re_set_one always recomputes
        the breakpoint's address from scratch, or deletes it if it can't.
        So I think this assignment could be deleted without effect.  */
-    b->address = (CORE_ADDR) NULL;
+    b->impl->address = (CORE_ADDR) NULL;
   }
   /* FIXME what about longjmp breakpoints?  Re-create them here?  */
   create_overlay_event_breakpoint ("_ovly_debug_event");
@@ -1330,7 +1330,7 @@ detach_breakpoints (int pid)
   inferior_ptid = pid_to_ptid (pid);
   ALL_BREAKPOINTS (b)
   {
-    if (b->inserted)
+    if (b->impl->inserted)
       {
 	val = remove_breakpoint (b, mark_inserted);
 	if (val != 0)
@@ -1379,10 +1379,10 @@ remove_breakpoint (struct breakpoint *b,
 	  /* No overlay handling: just remove the breakpoint.  */
 
 	  if (b->type == bp_hardware_breakpoint)
-	    val = target_remove_hw_breakpoint (b->address, 
-					       b->shadow_contents);
+	    val = target_remove_hw_breakpoint (b->impl->address, 
+					       b->impl->shadow_contents);
 	  else
-	    val = target_remove_breakpoint (b->address, b->shadow_contents);
+	    val = target_remove_breakpoint (b->impl->address, b->impl->shadow_contents);
 	}
       else
 	{
@@ -1393,29 +1393,29 @@ remove_breakpoint (struct breakpoint *b,
 		/* Yes -- overlay event support is not active, so we
 		   should have set a breakpoint at the LMA.  Remove it.  
 		*/
-		CORE_ADDR addr = overlay_unmapped_address (b->address, 
+		CORE_ADDR addr = overlay_unmapped_address (b->impl->address, 
 							   b->section);
 		/* Ignore any failures: if the LMA is in ROM, we will
 		   have already warned when we failed to insert it.  */
 		if (b->type != bp_hardware_breakpoint)
-		  target_remove_hw_breakpoint (addr, b->shadow_contents);
+		  target_remove_hw_breakpoint (addr, b->impl->shadow_contents);
 		else
-		  target_remove_breakpoint (addr, b->shadow_contents);
+		  target_remove_breakpoint (addr, b->impl->shadow_contents);
 	      }
 	  /* Did we set a breakpoint at the VMA? 
 	     If so, we will have marked the breakpoint 'inserted'.  */
-	  if (b->inserted)
+	  if (b->impl->inserted)
 	    {
 	      /* Yes -- remove it.  Previously we did not bother to
 		 remove the breakpoint if the section had been
 		 unmapped, but let's not rely on that being safe.  We
 		 don't know what the overlay manager might do.  */
 	      if (b->type == bp_hardware_breakpoint)
-		val = target_remove_hw_breakpoint (b->address, 
-						   b->shadow_contents);
+		val = target_remove_hw_breakpoint (b->impl->address, 
+						   b->impl->shadow_contents);
 	      else
-		val = target_remove_breakpoint (b->address,
-						b->shadow_contents);
+		val = target_remove_breakpoint (b->impl->address,
+						b->impl->shadow_contents);
 	    }
 	  else
 	    {
@@ -1425,18 +1425,18 @@ remove_breakpoint (struct breakpoint *b,
 	}
       if (val)
 	return val;
-      b->inserted = (is == mark_inserted);
+      b->impl->inserted = (is == mark_inserted);
     }
   else if ((b->type == bp_hardware_watchpoint ||
 	    b->type == bp_read_watchpoint ||
 	    b->type == bp_access_watchpoint)
 	   && b->enable_state == bp_enabled
-	   && !b->duplicate)
+	   && !b->impl->duplicate)
     {
       struct value *v;
       struct value *n;
 
-      b->inserted = (is == mark_inserted);
+      b->impl->inserted = (is == mark_inserted);
       /* Walk down the saved value chain.  */
       for (v = b->val_chain; v; v = v->next)
 	{
@@ -1464,13 +1464,13 @@ remove_breakpoint (struct breakpoint *b,
 
 		  val = target_remove_watchpoint (addr, len, type);
 		  if (val == -1)
-		    b->inserted = 1;
+		    b->impl->inserted = 1;
 		  val = 0;
 		}
 	    }
 	}
       /* Failure to remove any of the hardware watchpoints comes here.  */
-      if ((is == mark_uninserted) && (b->inserted))
+      if ((is == mark_uninserted) && (b->impl->inserted))
 	warning ("Could not remove hardware watchpoint %d.",
 		 b->number);
 
@@ -1487,7 +1487,7 @@ remove_breakpoint (struct breakpoint *b,
 	    b->type == bp_catch_vfork ||
 	    b->type == bp_catch_exec)
 	   && b->enable_state == bp_enabled
-	   && !b->duplicate)
+	   && !b->impl->duplicate)
     {
       val = -1;
       switch (b->type)
@@ -1507,30 +1507,30 @@ remove_breakpoint (struct breakpoint *b,
 	}
       if (val)
 	return val;
-      b->inserted = (is == mark_inserted);
+      b->impl->inserted = (is == mark_inserted);
     }
   else if ((b->type == bp_catch_catch ||
 	    b->type == bp_catch_throw)
 	   && b->enable_state == bp_enabled
-	   && !b->duplicate)
+	   && !b->impl->duplicate)
     {
 
-      val = target_remove_breakpoint (b->address, b->shadow_contents);
+      val = target_remove_breakpoint (b->impl->address, b->impl->shadow_contents);
       if (val)
 	return val;
-      b->inserted = (is == mark_inserted);
+      b->impl->inserted = (is == mark_inserted);
     }
   else if (ep_is_exception_catchpoint (b)
-	   && b->inserted	/* sometimes previous insert doesn't happen */
+	   && b->impl->inserted	/* sometimes previous insert doesn't happen */
 	   && b->enable_state == bp_enabled
-	   && !b->duplicate)
+	   && !b->impl->duplicate)
     {
 
-      val = target_remove_breakpoint (b->address, b->shadow_contents);
+      val = target_remove_breakpoint (b->impl->address, b->impl->shadow_contents);
       if (val)
 	return val;
 
-      b->inserted = (is == mark_inserted);
+      b->impl->inserted = (is == mark_inserted);
     }
 
   return 0;
@@ -1544,7 +1544,7 @@ mark_breakpoints_out (void)
   struct breakpoint *b;
 
   ALL_BREAKPOINTS (b)
-    b->inserted = 0;
+    b->impl->inserted = 0;
 }
 
 /* Clear the "inserted" flag in all breakpoints and delete any
@@ -1567,7 +1567,7 @@ breakpoint_init_inferior (enum inf_conte
 
   ALL_BREAKPOINTS_SAFE (b, temp)
   {
-    b->inserted = 0;
+    b->impl->inserted = 0;
 
     switch (b->type)
       {
@@ -1643,7 +1643,7 @@ breakpoint_here_p (CORE_ADDR pc)
   ALL_BREAKPOINTS (b)
     if ((b->enable_state == bp_enabled
 	 || b->enable_state == bp_permanent)
-	&& b->address == pc)	/* bp is enabled and matches pc */
+	&& b->impl->address == pc)	/* bp is enabled and matches pc */
       {
 	if (overlay_debugging 
 	    && section_is_overlay (b->section) 
@@ -1669,8 +1669,8 @@ breakpoint_inserted_here_p (CORE_ADDR pc
   struct breakpoint *b;
 
   ALL_BREAKPOINTS (b)
-    if (b->inserted
-	&& b->address == pc)	/* bp is inserted and matches pc */
+    if (b->impl->inserted
+	&& b->impl->address == pc)	/* bp is inserted and matches pc */
     {
       if (overlay_debugging 
 	  && section_is_overlay (b->section) 
@@ -1706,9 +1706,9 @@ deprecated_frame_in_dummy (struct frame_
     /* We need to check the PC as well as the frame on the sparc,
        for signals.exp in the testsuite.  */
 	&& (get_frame_pc (frame)
-	    >= (b->address
+	    >= (b->impl->address
 		- DEPRECATED_SIZEOF_CALL_DUMMY_WORDS / sizeof (LONGEST) * DEPRECATED_REGISTER_SIZE))
-	&& get_frame_pc (frame) <= b->address)
+	&& get_frame_pc (frame) <= b->impl->address)
       return 1;
   }
   return 0;
@@ -1729,7 +1729,7 @@ breakpoint_thread_match (CORE_ADDR pc, p
     if (b->enable_state != bp_disabled
 	&& b->enable_state != bp_shlib_disabled
 	&& b->enable_state != bp_call_disabled
-	&& b->address == pc
+	&& b->impl->address == pc
 	&& (b->thread == -1 || b->thread == thread))
     {
       if (overlay_debugging 
@@ -2540,7 +2540,7 @@ bpstat_stop_status (CORE_ADDR *pc, int n
 	&& b->type != bp_catch_catch
 	&& b->type != bp_catch_throw)	/* a non-watchpoint bp */
       {
-	if (b->address != bp_addr) 	/* address doesn't match */
+	if (b->impl->address != bp_addr) 	/* address doesn't match */
 	  continue;
 	if (overlay_debugging		/* unmapped overlay section */
 	    && section_is_overlay (b->section) 
@@ -2550,7 +2550,7 @@ bpstat_stop_status (CORE_ADDR *pc, int n
 
     if (b->type == bp_hardware_breakpoint)
       {
-	if (b->address != (*pc - DECR_PC_AFTER_HW_BREAK))
+	if (b->impl->address != (*pc - DECR_PC_AFTER_HW_BREAK))
 	  continue;
 	if (overlay_debugging		/* unmapped overlay section */
 	    && section_is_overlay (b->section) 
@@ -3141,7 +3141,7 @@ bpstat_have_active_hw_watchpoints (void)
   struct breakpoint *b;
   ALL_BREAKPOINTS (b)
     if ((b->enable_state == bp_enabled) &&
-	(b->inserted) &&
+	(b->impl->inserted) &&
 	((b->type == bp_hardware_watchpoint) ||
 	 (b->type == bp_read_watchpoint) ||
 	 (b->type == bp_access_watchpoint)))
@@ -3410,13 +3410,13 @@ print_one_breakpoint (struct breakpoint 
 	if (addressprint)
 	  {
 	    annotate_field (4);
-	    ui_out_field_core_addr (uiout, "addr", b->address);
+	    ui_out_field_core_addr (uiout, "addr", b->impl->address);
 	  }
 	annotate_field (5);
-	*last_addr = b->address;
+	*last_addr = b->impl->address;
 	if (b->source_file)
 	  {
-	    sym = find_pc_sect_function (b->address, b->section);
+	    sym = find_pc_sect_function (b->impl->address, b->section);
 	    if (sym)
 	      {
 		ui_out_text (uiout, "in ");
@@ -3431,7 +3431,7 @@ print_one_breakpoint (struct breakpoint 
 	  }
 	else
 	  {
-	    print_address_symbolic (b->address, stb->stream, demangle, "");
+	    print_address_symbolic (b->impl->address, stb->stream, demangle, "");
 	    ui_out_field_stream (uiout, "at", stb);
 	  }
 	break;
@@ -3695,14 +3695,14 @@ describe_other_breakpoints (CORE_ADDR pc
   struct breakpoint *b;
 
   ALL_BREAKPOINTS (b)
-    if (b->address == pc)	/* address match / overlay match */
+    if (b->impl->address == pc)	/* address match / overlay match */
       if (!overlay_debugging || b->section == section)
 	others++;
   if (others > 0)
     {
       printf_filtered ("Note: breakpoint%s ", (others > 1) ? "s" : "");
       ALL_BREAKPOINTS (b)
-	if (b->address == pc)	/* address match / overlay match */
+	if (b->impl->address == pc)	/* address match / overlay match */
 	  if (!overlay_debugging || b->section == section)
 	    {
 	      others--;
@@ -3783,7 +3783,7 @@ check_duplicates (struct breakpoint *bpt
   struct breakpoint *b;
   int count = 0;
   struct breakpoint *perm_bp = 0;
-  CORE_ADDR address = bpt->address;
+  CORE_ADDR address = bpt->impl->address;
   asection *section = bpt->section;
 
   if (! breakpoint_address_is_meaningful (bpt))
@@ -3793,7 +3793,7 @@ check_duplicates (struct breakpoint *bpt
     if (b->enable_state != bp_disabled
 	&& b->enable_state != bp_shlib_disabled
 	&& b->enable_state != bp_call_disabled
-	&& b->address == address	/* address / overlay match */
+	&& b->impl->address == address	/* address / overlay match */
 	&& (!overlay_debugging || b->section == section)
 	&& breakpoint_address_is_meaningful (b))
     {
@@ -3805,7 +3805,7 @@ check_duplicates (struct breakpoint *bpt
 	}
 	
       count++;
-      b->duplicate = count > 1;
+      b->impl->duplicate = count > 1;
     }
 
   /* If we found a permanent breakpoint at this address, go over the
@@ -3813,10 +3813,10 @@ check_duplicates (struct breakpoint *bpt
      duplicates.  */
   if (perm_bp)
     {
-      perm_bp->duplicate = 0;
+      perm_bp->impl->duplicate = 0;
 
       /* Permanent breakpoint should always be inserted.  */
-      if (! perm_bp->inserted)
+      if (! perm_bp->impl->inserted)
 	internal_error (__FILE__, __LINE__,
 			"allegedly permanent breakpoint is not "
 			"actually inserted");
@@ -3824,7 +3824,7 @@ check_duplicates (struct breakpoint *bpt
       ALL_BREAKPOINTS (b)
 	if (b != perm_bp)
 	  {
-	    if (b->inserted)
+	    if (b->impl->inserted)
 	      internal_error (__FILE__, __LINE__,
 			      "another breakpoint was inserted on top of "
 			      "a permanent breakpoint");
@@ -3832,10 +3832,10 @@ check_duplicates (struct breakpoint *bpt
 	    if (b->enable_state != bp_disabled
 		&& b->enable_state != bp_shlib_disabled
 		&& b->enable_state != bp_call_disabled
-		&& b->address == address	/* address / overlay match */
+		&& b->impl->address == address	/* address / overlay match */
 		&& (!overlay_debugging || b->section == section)
 		&& breakpoint_address_is_meaningful (b))
-	      b->duplicate = 1;
+	      b->impl->duplicate = 1;
 	  }
     }
 }
@@ -3862,7 +3862,9 @@ set_raw_breakpoint (struct symtab_and_li
 
   b = (struct breakpoint *) xmalloc (sizeof (struct breakpoint));
   memset (b, 0, sizeof (*b));
-  b->address = sal.pc;
+  b->impl = (struct impl_breakpoint *) xmalloc (sizeof (struct impl_breakpoint));
+  memset (b->impl, 0, sizeof (*b->impl));
+  b->impl->address = sal.pc;
   if (sal.symtab == NULL)
     b->source_file = NULL;
   else
@@ -3915,7 +3917,7 @@ make_breakpoint_permanent (struct breakp
   b->enable_state = bp_permanent;
 
   /* By definition, permanent breakpoints are already present in the code.  */
-  b->inserted = 1;
+  b->impl->inserted = 1;
 }
 
 static struct breakpoint *
@@ -4053,7 +4055,7 @@ create_thread_event_breakpoint (CORE_ADD
   
   b->enable_state = bp_enabled;
   /* addr_string has to be used or breakpoint_re_set will delete me.  */
-  xasprintf (&b->addr_string, "*0x%s", paddr (b->address));
+  xasprintf (&b->addr_string, "*0x%s", paddr (b->impl->address));
 
   return b;
 }
@@ -4104,8 +4106,8 @@ disable_breakpoints_in_shlibs (int silen
     if (((b->type == bp_breakpoint) ||
 	 (b->type == bp_hardware_breakpoint)) &&
 	b->enable_state == bp_enabled &&
-	!b->duplicate &&
-	PC_SOLIB (b->address))
+	!b->impl->duplicate &&
+	PC_SOLIB (b->impl->address))
       {
 	b->enable_state = bp_shlib_disabled;
 	if (!silent)
@@ -4136,7 +4138,7 @@ re_enable_breakpoints_in_shlibs (void)
 
       /* Do not reenable the breakpoint if the shared library
          is still not mapped in.  */
-      if (target_read_memory (b->address, buf, 1) == 0)
+      if (target_read_memory (b->impl->address, buf, 1) == 0)
 	b->enable_state = bp_enabled;
     }
 }
@@ -4353,7 +4355,7 @@ set_longjmp_resume_breakpoint (CORE_ADDR
   ALL_BREAKPOINTS (b)
     if (b->type == bp_longjmp_resume)
     {
-      b->address = pc;
+      b->impl->address = pc;
       b->enable_state = bp_enabled;
       b->frame_id = frame_id;
       check_duplicates (b);
@@ -4553,7 +4555,7 @@ mention (struct breakpoint *b)
       if (addressprint || b->source_file == NULL)
 	{
 	  printf_filtered (" at ");
-	  print_address_numeric (b->address, 1, gdb_stdout);
+	  print_address_numeric (b->impl->address, 1, gdb_stdout);
 	}
       if (b->source_file)
 	printf_filtered (": file %s, line %d.",
@@ -4615,7 +4617,7 @@ create_breakpoints (struct symtabs_and_l
 	else
 	  /* addr_string has to be used or breakpoint_re_set will delete
 	     me.  */
-	  xasprintf (&b->addr_string, "*0x%s", paddr (b->address));
+	  xasprintf (&b->addr_string, "*0x%s", paddr (b->impl->address));
 	b->cond_string = cond_string[i];
 	b->ignore_count = ignore_count;
 	b->enable_state = bp_enabled;
@@ -5468,7 +5470,7 @@ watch_command_1 (char *arg, int accessfl
 	  scope_breakpoint->frame_id = get_frame_id (prev_frame);
 
 	  /* Set the address at which we will stop.  */
-	  scope_breakpoint->address = get_frame_pc (prev_frame);
+	  scope_breakpoint->impl->address = get_frame_pc (prev_frame);
 
 	  /* The scope breakpoint is related to the watchpoint.  We
 	     will need to act on them together.  */
@@ -6060,10 +6062,10 @@ print_one_exception_catchpoint (struct b
   if (addressprint)
     {
       annotate_field (4);
-      ui_out_field_core_addr (uiout, "addr", b->address);
+      ui_out_field_core_addr (uiout, "addr", b->impl->address);
     }
   annotate_field (5);
-  *last_addr = b->address;
+  *last_addr = b->impl->address;
   if (strstr (b->addr_string, "throw") != NULL)
     ui_out_field_string (uiout, "what", "exception throw");
   else
@@ -6419,7 +6421,7 @@ clear_command (char *arg, int from_tty)
 	      && b->type != bp_read_watchpoint
 	      && b->type != bp_access_watchpoint
 	      /* Not if b is a watchpoint of any sort... */
-	      && (((sal.pc && (b->address == sal.pc)) 
+	      && (((sal.pc && (b->impl->address == sal.pc)) 
 		   && (!section_is_overlay (b->section)
 		       || b->section == sal.section))
 		  /* Yes, if sal.pc matches b (modulo overlays).  */
@@ -6531,7 +6533,7 @@ delete_breakpoint (struct breakpoint *bp
     delete_breakpoint_hook (bpt);
   breakpoint_delete_event (bpt->number);
 
-  if (bpt->inserted)
+  if (bpt->impl->inserted)
     remove_breakpoint (bpt, mark_inserted);
 
   if (breakpoint_chain == bpt)
@@ -6569,7 +6571,7 @@ delete_breakpoint (struct breakpoint *bp
   check_duplicates (bpt);
   /* If this breakpoint was inserted, and there is another breakpoint
      at the same address, we need to insert the other breakpoint.  */
-  if (bpt->inserted
+  if (bpt->impl->inserted
       && bpt->type != bp_hardware_watchpoint
       && bpt->type != bp_read_watchpoint
       && bpt->type != bp_access_watchpoint
@@ -6578,9 +6580,9 @@ delete_breakpoint (struct breakpoint *bp
       && bpt->type != bp_catch_exec)
     {
       ALL_BREAKPOINTS (b)
-	if (b->address == bpt->address
+	if (b->impl->address == bpt->impl->address
 	    && b->section == bpt->section
-	    && !b->duplicate
+	    && !b->impl->duplicate
 	    && b->enable_state != bp_disabled
 	    && b->enable_state != bp_shlib_disabled
 	    && b->enable_state != bp_call_disabled)
@@ -6597,9 +6599,9 @@ delete_breakpoint (struct breakpoint *bp
 			    "a permanent breakpoint");
 
 	  if (b->type == bp_hardware_breakpoint)
-	    val = target_insert_hw_breakpoint (b->address, b->shadow_contents);
+	    val = target_insert_hw_breakpoint (b->impl->address, b->impl->shadow_contents);
 	  else
-	    val = target_insert_breakpoint (b->address, b->shadow_contents);
+	    val = target_insert_breakpoint (b->impl->address, b->impl->shadow_contents);
 
 	  /* If there was an error in the insert, print a message, then stop execution.  */
 	  if (val != 0)
@@ -6619,7 +6621,7 @@ delete_breakpoint (struct breakpoint *bp
 		  {
 		    fprintf_unfiltered (tmp_error_stream, "Cannot insert breakpoint %d.\n", b->number);
 		    fprintf_filtered (tmp_error_stream, "Error accessing memory address ");
-		    print_address_numeric (b->address, 1, tmp_error_stream);
+		    print_address_numeric (b->impl->address, 1, tmp_error_stream);
 		    fprintf_filtered (tmp_error_stream, ": %s.\n",
 				      safe_strerror (val));
 		  }
@@ -6629,7 +6631,7 @@ delete_breakpoint (struct breakpoint *bp
 	      error_stream(tmp_error_stream); 
 	    }
 	  else
-	    b->inserted = 1;
+	    b->impl->inserted = 1;
 	}
     }
 
@@ -6669,6 +6671,7 @@ delete_breakpoint (struct breakpoint *bp
      bp, we mark it as deleted before freeing its storage. */
   bpt->type = bp_none;
 
+  xfree (bpt->impl);
   xfree (bpt);
 }
 
@@ -6810,7 +6813,7 @@ breakpoint_re_set_one (void *bint)
 	    }
 
 	  /* We need to re-set the breakpoint if the address changes... */
-	  if (b->address != sals.sals[i].pc
+	  if (b->impl->address != sals.sals[i].pc
 	  /* ...or new and old breakpoints both have source files, and
 	     the source file name or the line number changes...  */
 	      || (b->source_file != NULL
@@ -6832,7 +6835,7 @@ breakpoint_re_set_one (void *bint)
 		  savestring (sals.sals[i].symtab->filename,
 			      strlen (sals.sals[i].symtab->filename));
 	      b->line_number = sals.sals[i].line;
-	      b->address = sals.sals[i].pc;
+	      b->impl->address = sals.sals[i].pc;
 
 	      /* Used to check for duplicates here, but that can
 	         cause trouble, as it doesn't check for disabled
Index: gdb/breakpoint.h
===================================================================
--- gdb.orig/breakpoint.h	2003-10-08 12:42:09.000000000 -0400
+++ gdb/breakpoint.h	2003-10-08 12:42:09.000000000 -0400
@@ -184,6 +184,58 @@ enum target_hw_bp_type
     hw_execute = 3		/* Execute HW breakpoint */
   };
 
+/* GDB maintains two groups of breakpoints and related events.  One
+   group are the "implementation breakpoints"; these are minimal
+   structures used to manage stopping the program.  They map to a specific
+   stop reason (trap at a particular PC, for instance).  The other group
+   are "user breakpoints"; these carry higher-level information including
+   source locations and breakpoint conditions.  */
+
+enum impl_bptype
+{
+  impl_bp_software_breakpoint,
+  impl_bp_hardware_breakpoint,
+  impl_bp_hardware_watchpoint,
+  impl_bp_other			/* Miscellaneous...  */
+};
+
+struct impl_breakpoint
+{
+  /* Type of this implementation breakpoint.  */
+  enum impl_bptype type;
+
+  /* Each implementation breakpoint must belong to exactly one higher-level
+     breakpoint.  This and the DUPLICATE flag are more straightforward
+     than reference counting.  */
+  struct breakpoint *owner;
+
+  /* Nonzero if this breakpoint is now inserted.  */
+  char inserted;
+
+  /* Nonzero if this is not the first breakpoint in the list
+     for the given address.  */
+  char duplicate;
+
+  /* If we someday support real thread-specific breakpoints, then
+     the implementation breakpoint will need a thread identifier.  */
+
+  /* Data for specific breakpoint types.  These could be a union, but
+     simplicity is more important than memory usage for breakpoints.  */
+
+  /* Note that zero is a perfectly valid code address on some platforms
+     (for example, the mn10200 (OBSOLETE) and mn10300 simulators).  NULL
+     is not a special value for this field.  Valid for all types except
+     impl_bp_other.  */
+  CORE_ADDR address;
+
+  /* "Real" contents of byte where breakpoint has been inserted.
+     Valid only when breakpoints are in the program.  Under the complete
+     control of the target insert_breakpoint and remove_breakpoint routines.
+     No other code should assume anything about the value(s) here.
+     Valid only for impl_bp_software_breakpoint.  */
+  char shadow_contents[BREAKPOINT_MAX];
+};
+
 /* This structure is a collection of function pointers that, if available,
    will be called instead of the performing the default action for this
    bptype.  */
@@ -222,10 +274,8 @@ struct breakpoint
     /* Number assigned to distinguish breakpoints.  */
     int number;
 
-    /* Address to break at.  Note that zero is a perfectly valid code
-       address on some platforms (for example, the and mn10300
-       simulators).  NULL is not a special value for this field.  */
-    CORE_ADDR address;
+    /* Implementation breakpoint for this user-level breakpoint.  */
+    struct impl_breakpoint *impl;
 
     /* Line number of this address.  */
 
@@ -241,16 +291,6 @@ struct breakpoint
     /* Number of stops at this breakpoint that should
        be continued automatically before really stopping.  */
     int ignore_count;
-    /* "Real" contents of byte where breakpoint has been inserted.
-       Valid only when breakpoints are in the program.  Under the complete
-       control of the target insert_breakpoint and remove_breakpoint routines.
-       No other code should assume anything about the value(s) here.  */
-    char shadow_contents[BREAKPOINT_MAX];
-    /* Nonzero if this breakpoint is now inserted.  */
-    char inserted;
-    /* Nonzero if this is not the first breakpoint in the list
-       for the given address.  */
-    char duplicate;
     /* Chain of command lines to execute when this breakpoint is hit.  */
     struct command_line *commands;
     /* Stack depth (address of frame).  If nonzero, break only if fp


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-08 17:02 RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint Daniel Jacobowitz
@ 2003-10-08 18:17 ` Jim Blandy
  2003-10-14  1:30   ` Daniel Jacobowitz
  2003-10-14 16:03   ` Andrew Cagney
  2003-10-08 18:21 ` Eli Zaretskii
  2003-11-06 17:57 ` Daniel Jacobowitz
  2 siblings, 2 replies; 11+ messages in thread
From: Jim Blandy @ 2003-10-08 18:17 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches


Daniel Jacobowitz <drow@mvista.com> writes:
> +/* GDB maintains two groups of breakpoints and related events.  One
> +   group are the "implementation breakpoints"; these are minimal
> +   structures used to manage stopping the program.  They map to a specific
> +   stop reason (trap at a particular PC, for instance).  The other group
> +   are "user breakpoints"; these carry higher-level information including
> +   source locations and breakpoint conditions.  */

"minimal structures used to manage stopping the program" could be
almost anything.  How about:

    GDB maintains two groups of breakpoints and related events.  One
    group are the "implementation breakpoints" (struct
    impl_breakpoint); these represent specific machine-level
    mechanisms used to stop the program: trap instructions patched
    into the code ("software breakpoints"), hardware breakpoints,
    hardware watchpoint registers, and so on.

    The other group are "user breakpoints" (struct breakpoint); these
    are the breakpoints as seen and manipulated by the user.  They
    carry higher-level information like source locations and
    breakpoint conditions.

    A single user breakpoint may use several implementation
    breakpoints to get the right effect.  For example, the GNU C++
    compiler emits two copies of each constructor: the 'in-charge'
    constructor and the 'not-in-charge' constructor.  So a user
    breakpoint on the constructor would have two separate
    implementation breakpoints, one for each copy.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-08 17:02 RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint Daniel Jacobowitz
  2003-10-08 18:17 ` Jim Blandy
@ 2003-10-08 18:21 ` Eli Zaretskii
  2003-10-08 19:11   ` Daniel Jacobowitz
  2003-11-06 17:57 ` Daniel Jacobowitz
  2 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2003-10-08 18:21 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> Date: Wed, 8 Oct 2003 13:02:33 -0400
> From: Daniel Jacobowitz <drow@mvista.com>
> +
> +enum impl_bptype
> +{
> +  impl_bp_software_breakpoint,
> +  impl_bp_hardware_breakpoint,
> +  impl_bp_hardware_watchpoint,
> +  impl_bp_other			/* Miscellaneous...  */
> +};

Why did you decide to leave the subclasses of hardware watchpoints
(read, access, and write) in the parent structure, instead of moving
that distinction here?  That seems like you are spreading related
information between several places instead of having it in a single
place.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-08 18:21 ` Eli Zaretskii
@ 2003-10-08 19:11   ` Daniel Jacobowitz
  2003-10-09  6:04     ` Eli Zaretskii
  2003-10-09 19:16     ` Michael Snyder
  0 siblings, 2 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2003-10-08 19:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On Wed, Oct 08, 2003 at 07:58:35PM +0200, Eli Zaretskii wrote:
> > Date: Wed, 8 Oct 2003 13:02:33 -0400
> > From: Daniel Jacobowitz <drow@mvista.com>
> > +
> > +enum impl_bptype
> > +{
> > +  impl_bp_software_breakpoint,
> > +  impl_bp_hardware_breakpoint,
> > +  impl_bp_hardware_watchpoint,
> > +  impl_bp_other			/* Miscellaneous...  */
> > +};
> 
> Why did you decide to leave the subclasses of hardware watchpoints
> (read, access, and write) in the parent structure, instead of moving
> that distinction here?  That seems like you are spreading related
> information between several places instead of having it in a single
> place.

I'm actually planning to move it to the impl_breakpoint.  I haven't
done it yet because I wanted to postpone watchpoints until the
one-to-many support was in place.

For instance, according to my interpretation, rwatch **foo should be:
  a read watchpoint on the address *foo
  a write watchpoint on the address foo, in case it is moved.
I don't know if that matches GDB's current interpreation of such
expressions, though - I haven't looked yet.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-08 19:11   ` Daniel Jacobowitz
@ 2003-10-09  6:04     ` Eli Zaretskii
  2003-10-09 19:16     ` Michael Snyder
  1 sibling, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2003-10-09  6:04 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> Date: Wed, 8 Oct 2003 15:11:56 -0400
> From: Daniel Jacobowitz <drow@mvista.com>
> > 
> > Why did you decide to leave the subclasses of hardware watchpoints
> > (read, access, and write) in the parent structure, instead of moving
> > that distinction here?  That seems like you are spreading related
> > information between several places instead of having it in a single
> > place.
> 
> I'm actually planning to move it to the impl_breakpoint.

Ah, okay then.

> For instance, according to my interpretation, rwatch **foo should be:
>   a read watchpoint on the address *foo
>   a write watchpoint on the address foo, in case it is moved.
> I don't know if that matches GDB's current interpreation of such
> expressions, though - I haven't looked yet.

GDB already does something similar, of course: that's why you see the
value chain being chased each time GDB inserts or removes watchpoints.
In this case, the value chain of **foo will include &foo as well.
Similar things happen with watching arrays and structs; the comments
in breakpoint.c should shed more light on this.

One other thing to keep in mind wrt watchpoints is that the
bp_hardware_watchpoint kind is handled differently from
bp_read_watchpoint and bp_access_watchpoint.  I believe the reasons
are histerical, but I think you will need to know this while hacking
that part of the code, especially if you decide to change it so that
all 3 kinds are handled the same way, as I think they should.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-08 19:11   ` Daniel Jacobowitz
  2003-10-09  6:04     ` Eli Zaretskii
@ 2003-10-09 19:16     ` Michael Snyder
  1 sibling, 0 replies; 11+ messages in thread
From: Michael Snyder @ 2003-10-09 19:16 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Eli Zaretskii, gdb-patches

Daniel Jacobowitz wrote:
> On Wed, Oct 08, 2003 at 07:58:35PM +0200, Eli Zaretskii wrote:
> 
>>>Date: Wed, 8 Oct 2003 13:02:33 -0400
>>>From: Daniel Jacobowitz <drow@mvista.com>
>>>+
>>>+enum impl_bptype
>>>+{
>>>+  impl_bp_software_breakpoint,
>>>+  impl_bp_hardware_breakpoint,
>>>+  impl_bp_hardware_watchpoint,
>>>+  impl_bp_other			/* Miscellaneous...  */
>>>+};
>>
>>Why did you decide to leave the subclasses of hardware watchpoints
>>(read, access, and write) in the parent structure, instead of moving
>>that distinction here?  That seems like you are spreading related
>>information between several places instead of having it in a single
>>place.
> 
> 
> I'm actually planning to move it to the impl_breakpoint.  I haven't
> done it yet because I wanted to postpone watchpoints until the
> one-to-many support was in place.
> 
> For instance, according to my interpretation, rwatch **foo should be:
>   a read watchpoint on the address *foo
>   a write watchpoint on the address foo, in case it is moved.
> I don't know if that matches GDB's current interpreation of such
> expressions, though - I haven't looked yet.

For software watchpoints, you'd probably just evaluate the expression
and see if the result changed.  For hardware watchpoints, you'd need
to watch every location that would be referenced in evaluating the
expression.  IMO...



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-08 18:17 ` Jim Blandy
@ 2003-10-14  1:30   ` Daniel Jacobowitz
  2003-10-14 15:31     ` Andrew Cagney
  2003-10-14 16:03   ` Andrew Cagney
  1 sibling, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2003-10-14  1:30 UTC (permalink / raw)
  To: gdb-patches

On Wed, Oct 08, 2003 at 01:16:03PM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > +/* GDB maintains two groups of breakpoints and related events.  One
> > +   group are the "implementation breakpoints"; these are minimal
> > +   structures used to manage stopping the program.  They map to a specific
> > +   stop reason (trap at a particular PC, for instance).  The other group
> > +   are "user breakpoints"; these carry higher-level information including
> > +   source locations and breakpoint conditions.  */
> 
> "minimal structures used to manage stopping the program" could be
> almost anything.  How about:
> 
>     GDB maintains two groups of breakpoints and related events.  One
>     group are the "implementation breakpoints" (struct
>     impl_breakpoint); these represent specific machine-level
>     mechanisms used to stop the program: trap instructions patched
>     into the code ("software breakpoints"), hardware breakpoints,
>     hardware watchpoint registers, and so on.
> 
>     The other group are "user breakpoints" (struct breakpoint); these
>     are the breakpoints as seen and manipulated by the user.  They
>     carry higher-level information like source locations and
>     breakpoint conditions.
> 
>     A single user breakpoint may use several implementation
>     breakpoints to get the right effect.  For example, the GNU C++
>     compiler emits two copies of each constructor: the 'in-charge'
>     constructor and the 'not-in-charge' constructor.  So a user
>     breakpoint on the constructor would have two separate
>     implementation breakpoints, one for each copy.

Thanks, I like it.  I'll adapt it into the next version.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-14  1:30   ` Daniel Jacobowitz
@ 2003-10-14 15:31     ` Andrew Cagney
  2003-10-14 15:36       ` Daniel Jacobowitz
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Cagney @ 2003-10-14 15:31 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> 
> Thanks, I like it.  I'll adapt it into the next version.

Daniel,

This is like coding standards - if there is an existing convention (even 
if you personally find it a bit sucky) adopt it.  Especially when there 
there's a preference towards it by a number of developers.  It saves 
hassles trying to explain this over and over and over later.

Andrew



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-14 15:31     ` Andrew Cagney
@ 2003-10-14 15:36       ` Daniel Jacobowitz
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2003-10-14 15:36 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Tue, Oct 14, 2003 at 11:31:22AM -0400, Andrew Cagney wrote:
> >
> >Thanks, I like it.  I'll adapt it into the next version.
> 
> Daniel,
> 
> This is like coding standards - if there is an existing convention (even 
> if you personally find it a bit sucky) adopt it.  Especially when there 
> there's a preference towards it by a number of developers.  It saves 
> hassles trying to explain this over and over and over later.

I have very little idea what you're talking about.  I was thanking Jim
for some explanatory text of what the new kind of breakpoint is.  The
text is appropriate whatever we call it - forcing someone to know the
existing conventions isn't OK for this terminology especially when the
"existing conventions" are so unclear.

Are you complaining about my choice of what to call the two kinds of
breakpoints?  If so, I disagree strongly that the one book constitutes
existing convention, especially when its choice is unclear to half of
the GDB developers who have responded.  And relatively few people who
will read this code will have read said book; and (ooh, my personal
opinion is showing again, gasp) I'm not all that impressed with said
book anyway, and it seems many others aren't either from the reviews
I've read.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-08 18:17 ` Jim Blandy
  2003-10-14  1:30   ` Daniel Jacobowitz
@ 2003-10-14 16:03   ` Andrew Cagney
  1 sibling, 0 replies; 11+ messages in thread
From: Andrew Cagney @ 2003-10-14 16:03 UTC (permalink / raw)
  To: Jim Blandy, Daniel Jacobowitz; +Cc: gdb-patches

> Daniel Jacobowitz <drow@mvista.com> writes:
> 
>> +/* GDB maintains two groups of breakpoints and related events.  One
>> +   group are the "implementation breakpoints"; these are minimal
>> +   structures used to manage stopping the program.  They map to a specific
>> +   stop reason (trap at a particular PC, for instance).  The other group
>> +   are "user breakpoints"; these carry higher-level information including
>> +   source locations and breakpoint conditions.  */

GDB users are going to need to know that breakpoints are implemented at 
two levels, and both of those levels will need to be visible.  That's 
why people are suggesting pairs such as logical/physical, 
virtual/actual, ...  Calling one "user" and the other "impl" or 
"machine" does not convey that implicit relationship.  In fact calling 
one "user" is saying "hey users, just you stick to this user stuff".

``specific machine-level mechanisms used to stop the program: trap 
instructions patched into the code ("software breakpoints"), hardware 
breakpoints, hardware watchpoint registers, and so on.''

This isn't correct.  The low-level breakpoints correspond to 
target-specific and not machine-specific mechanisms.   For instance, the 
target can directly implement relatively abstract breakpoint/watchpoint 
mechanisms.  Eg: per-thread breakpoints where the the underlying 
machine-specific mechanisms are hidden from GDB.  This abstract model is 
extreemly common in 'debug agent' code, I've seen at least three people 
being forced to hack around GDB's bogus machine-level assumptions.

Andrew



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint
  2003-10-08 17:02 RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint Daniel Jacobowitz
  2003-10-08 18:17 ` Jim Blandy
  2003-10-08 18:21 ` Eli Zaretskii
@ 2003-11-06 17:57 ` Daniel Jacobowitz
  2 siblings, 0 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2003-11-06 17:57 UTC (permalink / raw)
  To: gdb-patches

On Wed, Oct 08, 2003 at 01:02:33PM -0400, Daniel Jacobowitz wrote:
> This patch adds a struct impl_breakpoint and the corresponding enum
> impl_bptype.  Basic impl data is moved from the breakpoint to the
> impl_breakpoint: address, shadow contents for sw breakpoints, inserted and
> duplicate flags.  Then I mechanically update all of breakpoint.c to use the
> new members.  No surprises here; everything in impl_breakpoint is
> initialized just when it would have been before the move, except for the new
> type field, which will be taken care of shortly.  The impl_breakpoint is
> allocated and deallocated at the same time as the breakpoint.

I'm checking in these patches, with the name changed to bp_location,
since no objections were raised.  I've made a couple of comments
changes, renamed one confusing field, and otherwise merged them to the
current tree.  Here's the first.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-11-06  Daniel Jacobowitz  <drow@mvista.com>

	* breakpoint.h (enum bp_loc_type, struct bp_location): New.
	(struct breakpoint): Remove address, shadow_contents, inserted,
	requested_address, and duplicate.  Add a struct bp_location.
	* breakpoint.c (condition_command, read_memory_nobpt)
	(insert_breakpoints, remove_breakpoints, remove_hw_watchpoints)
	(reattach_breakpoints, update_breakpoints_after_exec)
	(detach_breakpoints, remove_breakpoint, mark_breakpoints_out)
	(breakpoint_init_inferior, breakpoint_here_p)
	(breakpoint_inserted_here_p, deprecated_frame_in_dummy)
	(breakpoint_thread_match, bpstat_stop_status)
	(bpstat_have_active_hw_watchpoints, print_one_breakpoint)
	(describe_other_breakpoints, check_duplicates)
	(make_breakpoint_permanent, create_thread_event_breakpoint)
	(disable_breakpoints_in_shlibs, re_enable_berakpoints_in_shlibs)
	(set_longjmp_resume_breakpoint, mention, create_breakpoints)
	(watch_command_1, print_one_exception_catchpoint)
	(clear_command, breakpoint_re_set_one): Adjust member accesses to
	use the breakpoint's ->loc.
	(set_raw_breakpoint): Likewise.  Initialize ->loc.
	(delete_breakpoint): Likewise.  Free ->loc.

Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2003-11-05 11:53:25.000000000 -0500
+++ gdb/breakpoint.c	2003-11-05 11:53:26.000000000 -0500
@@ -552,7 +552,7 @@ condition_command (char *arg, int from_t
 	  /* I don't know if it matters whether this is the string the user
 	     typed in or the decompiled expression.  */
 	  b->cond_string = savestring (arg, strlen (arg));
-	  b->cond = parse_exp_1 (&arg, block_for_pc (b->address), 0);
+	  b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
 	  if (*arg)
 	    error ("Junk at end of expression");
 	}
@@ -635,7 +635,7 @@ read_memory_nobpt (CORE_ADDR memaddr, ch
 	|| b->type == bp_access_watchpoint)
       continue;
     /* bp in memory? */
-    if (!b->inserted)
+    if (!b->loc->inserted)
       continue;
     /* Addresses and length of the part of the breakpoint that
        we need to copy.  */
@@ -643,7 +643,7 @@ read_memory_nobpt (CORE_ADDR memaddr, ch
        breakpoint values.  BREAKPOINT_FROM_PC still manages to
        correctly determine the breakpoints memory address and size
        for these targets. */
-    bp_addr = b->address;
+    bp_addr = b->loc->address;
     bp_size = 0;
     if (BREAKPOINT_FROM_PC (&bp_addr, &bp_size) == NULL)
       continue;
@@ -679,7 +679,7 @@ read_memory_nobpt (CORE_ADDR memaddr, ch
 	}
 
       memcpy (myaddr + bp_addr - memaddr,
-	      b->shadow_contents + bptoffset, bp_size);
+	      b->loc->shadow_contents + bptoffset, bp_size);
 
       if (bp_addr > memaddr)
 	{
@@ -787,8 +787,8 @@ insert_breakpoints (void)
 	&& b->type != bp_catch_exec
 	&& b->type != bp_catch_throw
 	&& b->type != bp_catch_catch
-	&& !b->inserted
-	&& !b->duplicate)
+	&& !b->loc->inserted
+	&& !b->loc->duplicate)
       {
 	/* "Normal" instruction breakpoint: either the standard
 	   trap-instruction bp (bp_breakpoint), or a
@@ -802,10 +802,10 @@ insert_breakpoints (void)
 	    /* No overlay handling: just set the breakpoint.  */
 
 	    if (b->type == bp_hardware_breakpoint)
-	      val = target_insert_hw_breakpoint (b->address, 
-						 b->shadow_contents);
+	      val = target_insert_hw_breakpoint (b->loc->address, 
+						 b->loc->shadow_contents);
 	    else
-	      val = target_insert_breakpoint (b->address, b->shadow_contents);
+	      val = target_insert_breakpoint (b->loc->address, b->loc->shadow_contents);
 	  }
 	else
 	  {
@@ -821,10 +821,10 @@ insert_breakpoints (void)
 			   b->number);
 		else
 		  {
-		    CORE_ADDR addr = overlay_unmapped_address (b->address, 
+		    CORE_ADDR addr = overlay_unmapped_address (b->loc->address, 
 							       b->section);
 		    /* Set a software (trap) breakpoint at the LMA.  */
-		    val = target_insert_breakpoint (addr, b->shadow_contents);
+		    val = target_insert_breakpoint (addr, b->loc->shadow_contents);
 		    if (val != 0)
 		      fprintf_unfiltered (tmp_error_stream, 
 					  "Overlay breakpoint %d failed: in ROM?", 
@@ -836,11 +836,11 @@ insert_breakpoints (void)
 	      {
 		/* Yes.  This overlay section is mapped into memory.  */
 		if (b->type == bp_hardware_breakpoint)
-		  val = target_insert_hw_breakpoint (b->address, 
-						     b->shadow_contents);
+		  val = target_insert_hw_breakpoint (b->loc->address, 
+						     b->loc->shadow_contents);
 		else
-		  val = target_insert_breakpoint (b->address,
-						  b->shadow_contents);
+		  val = target_insert_breakpoint (b->loc->address,
+						  b->loc->shadow_contents);
 	      }
 	    else
 	      {
@@ -854,7 +854,7 @@ insert_breakpoints (void)
 	  {
 	    /* Can't set the breakpoint.  */
 #if defined (DISABLE_UNSETTABLE_BREAK)
-	    if (DISABLE_UNSETTABLE_BREAK (b->address))
+	    if (DISABLE_UNSETTABLE_BREAK (b->loc->address))
 	      {
 		/* See also: disable_breakpoints_in_shlibs. */
 		val = 0;
@@ -891,7 +891,7 @@ insert_breakpoints (void)
 					b->number);
 		    fprintf_filtered (tmp_error_stream, 
 				      "Error accessing memory address ");
-		    print_address_numeric (b->address, 1, tmp_error_stream);
+		    print_address_numeric (b->loc->address, 1, tmp_error_stream);
 		    fprintf_filtered (tmp_error_stream, ": %s.\n",
 				      safe_strerror (val));
 		  }
@@ -899,20 +899,20 @@ insert_breakpoints (void)
 	      }
 	  }
 	else
-	  b->inserted = 1;
+	  b->loc->inserted = 1;
 
 	if (val)
 	  return_val = val;	/* remember failure */
       }
     else if (ep_is_exception_catchpoint (b)
-	     && !b->inserted
-	     && !b->duplicate)
+	     && !b->loc->inserted
+	     && !b->loc->duplicate)
 
       {
 	/* If we get here, we must have a callback mechanism for exception
 	   events -- with g++ style embedded label support, we insert
 	   ordinary breakpoints and not catchpoints. */
-	val = target_insert_breakpoint (b->address, b->shadow_contents);
+	val = target_insert_breakpoint (b->loc->address, b->loc->shadow_contents);
 	if (val)
 	  {
 	    /* Couldn't set breakpoint for some reason */
@@ -921,7 +921,7 @@ insert_breakpoints (void)
 				b->number);
 	    fprintf_filtered (tmp_error_stream, 
 			      "Error accessing memory address ");
-	    print_address_numeric (b->address, 1, tmp_error_stream);
+	    print_address_numeric (b->loc->address, 1, tmp_error_stream);
 	    fprintf_filtered (tmp_error_stream, ": %s.\n",
 			      safe_strerror (val));
 	    b->enable_state = bp_disabled;
@@ -943,7 +943,7 @@ insert_breakpoints (void)
 	    do_cleanups (cleanups);
 	    if (val != 0 && val != -1)
 	      {
-		b->inserted = 1;
+		b->loc->inserted = 1;
 	      }
 	    /* Check if something went wrong; val == 0 can be ignored */
 	    if (val == -1)
@@ -964,8 +964,8 @@ insert_breakpoints (void)
 	      b->type == bp_read_watchpoint ||
 	      b->type == bp_access_watchpoint)
 	     && b->disposition != disp_del_at_next_stop
-	     && !b->inserted
-	     && !b->duplicate)
+	     && !b->loc->inserted
+	     && !b->loc->duplicate)
       {
 	struct frame_info *saved_frame;
 	int saved_level, within_current_scope;
@@ -1002,7 +1002,7 @@ insert_breakpoints (void)
 	    value_release_to_mark (mark);
 
 	    b->val_chain = v;
-	    b->inserted = 1;
+	    b->loc->inserted = 1;
 
 	    /* Look at each value on the value chain.  */
 	    for (; v; v = v->next)
@@ -1042,7 +1042,7 @@ insert_breakpoints (void)
 			       watches below, and removing a
 			       watchpoint we didn't insert could have
 			       adverse effects.  */
-			    b->inserted = 0;
+			    b->loc->inserted = 0;
 			  }
 			val = 0;
 		      }
@@ -1050,7 +1050,7 @@ insert_breakpoints (void)
 	      }
 	    /* Failure to insert a watchpoint on any memory value in the
 	       value chain brings us here.  */
-	    if (!b->inserted)
+	    if (!b->loc->inserted)
 	      {
 		remove_breakpoint (b, mark_uninserted);
 		hw_breakpoint_error = 1;
@@ -1081,8 +1081,8 @@ insert_breakpoints (void)
     else if ((b->type == bp_catch_fork
 	      || b->type == bp_catch_vfork
 	      || b->type == bp_catch_exec)
-	     && !b->inserted
-	     && !b->duplicate)
+	     && !b->loc->inserted
+	     && !b->loc->duplicate)
       {
 	char *prefix = xstrprintf ("warning: inserting catchpoint %d: ",
 				   b->number);
@@ -1093,7 +1093,7 @@ insert_breakpoints (void)
 	if (val < 0)
 	  b->enable_state = bp_disabled;
 	else
-	  b->inserted = 1;
+	  b->loc->inserted = 1;
       }
   }
   
@@ -1126,7 +1126,7 @@ remove_breakpoints (void)
 
   ALL_BREAKPOINTS (b)
   {
-    if (b->inserted)
+    if (b->loc->inserted)
       {
 	val = remove_breakpoint (b, mark_uninserted);
 	if (val != 0)
@@ -1144,7 +1144,7 @@ remove_hw_watchpoints (void)
 
   ALL_BREAKPOINTS (b)
   {
-    if (b->inserted
+    if (b->loc->inserted
 	&& (b->type == bp_hardware_watchpoint
 	    || b->type == bp_read_watchpoint
 	    || b->type == bp_access_watchpoint))
@@ -1168,13 +1168,13 @@ reattach_breakpoints (int pid)
   inferior_ptid = pid_to_ptid (pid);
   ALL_BREAKPOINTS (b)
   {
-    if (b->inserted)
+    if (b->loc->inserted)
       {
 	remove_breakpoint (b, mark_inserted);
 	if (b->type == bp_hardware_breakpoint)
-	  val = target_insert_hw_breakpoint (b->address, b->shadow_contents);
+	  val = target_insert_hw_breakpoint (b->loc->address, b->loc->shadow_contents);
 	else
-	  val = target_insert_breakpoint (b->address, b->shadow_contents);
+	  val = target_insert_breakpoint (b->loc->address, b->loc->shadow_contents);
 	if (val != 0)
 	  {
 	    do_cleanups (old_chain);
@@ -1261,7 +1261,7 @@ update_breakpoints_after_exec (void)
 	(b->type == bp_catch_vfork) ||
 	(b->type == bp_catch_fork))
       {
-	b->address = (CORE_ADDR) NULL;
+	b->loc->address = (CORE_ADDR) NULL;
 	continue;
       }
 
@@ -1314,7 +1314,7 @@ update_breakpoints_after_exec (void)
        unnecessary.  A call to breakpoint_re_set_one always recomputes
        the breakpoint's address from scratch, or deletes it if it can't.
        So I think this assignment could be deleted without effect.  */
-    b->address = (CORE_ADDR) NULL;
+    b->loc->address = (CORE_ADDR) NULL;
   }
   /* FIXME what about longjmp breakpoints?  Re-create them here?  */
   create_overlay_event_breakpoint ("_ovly_debug_event");
@@ -1334,7 +1334,7 @@ detach_breakpoints (int pid)
   inferior_ptid = pid_to_ptid (pid);
   ALL_BREAKPOINTS (b)
   {
-    if (b->inserted)
+    if (b->loc->inserted)
       {
 	val = remove_breakpoint (b, mark_inserted);
 	if (val != 0)
@@ -1383,10 +1383,10 @@ remove_breakpoint (struct breakpoint *b,
 	  /* No overlay handling: just remove the breakpoint.  */
 
 	  if (b->type == bp_hardware_breakpoint)
-	    val = target_remove_hw_breakpoint (b->address, 
-					       b->shadow_contents);
+	    val = target_remove_hw_breakpoint (b->loc->address, 
+					       b->loc->shadow_contents);
 	  else
-	    val = target_remove_breakpoint (b->address, b->shadow_contents);
+	    val = target_remove_breakpoint (b->loc->address, b->loc->shadow_contents);
 	}
       else
 	{
@@ -1397,29 +1397,29 @@ remove_breakpoint (struct breakpoint *b,
 		/* Yes -- overlay event support is not active, so we
 		   should have set a breakpoint at the LMA.  Remove it.  
 		*/
-		CORE_ADDR addr = overlay_unmapped_address (b->address, 
+		CORE_ADDR addr = overlay_unmapped_address (b->loc->address, 
 							   b->section);
 		/* Ignore any failures: if the LMA is in ROM, we will
 		   have already warned when we failed to insert it.  */
 		if (b->type != bp_hardware_breakpoint)
-		  target_remove_hw_breakpoint (addr, b->shadow_contents);
+		  target_remove_hw_breakpoint (addr, b->loc->shadow_contents);
 		else
-		  target_remove_breakpoint (addr, b->shadow_contents);
+		  target_remove_breakpoint (addr, b->loc->shadow_contents);
 	      }
 	  /* Did we set a breakpoint at the VMA? 
 	     If so, we will have marked the breakpoint 'inserted'.  */
-	  if (b->inserted)
+	  if (b->loc->inserted)
 	    {
 	      /* Yes -- remove it.  Previously we did not bother to
 		 remove the breakpoint if the section had been
 		 unmapped, but let's not rely on that being safe.  We
 		 don't know what the overlay manager might do.  */
 	      if (b->type == bp_hardware_breakpoint)
-		val = target_remove_hw_breakpoint (b->address, 
-						   b->shadow_contents);
+		val = target_remove_hw_breakpoint (b->loc->address, 
+						   b->loc->shadow_contents);
 	      else
-		val = target_remove_breakpoint (b->address,
-						b->shadow_contents);
+		val = target_remove_breakpoint (b->loc->address,
+						b->loc->shadow_contents);
 	    }
 	  else
 	    {
@@ -1429,18 +1429,18 @@ remove_breakpoint (struct breakpoint *b,
 	}
       if (val)
 	return val;
-      b->inserted = (is == mark_inserted);
+      b->loc->inserted = (is == mark_inserted);
     }
   else if ((b->type == bp_hardware_watchpoint ||
 	    b->type == bp_read_watchpoint ||
 	    b->type == bp_access_watchpoint)
 	   && b->enable_state == bp_enabled
-	   && !b->duplicate)
+	   && !b->loc->duplicate)
     {
       struct value *v;
       struct value *n;
 
-      b->inserted = (is == mark_inserted);
+      b->loc->inserted = (is == mark_inserted);
       /* Walk down the saved value chain.  */
       for (v = b->val_chain; v; v = v->next)
 	{
@@ -1468,13 +1468,13 @@ remove_breakpoint (struct breakpoint *b,
 
 		  val = target_remove_watchpoint (addr, len, type);
 		  if (val == -1)
-		    b->inserted = 1;
+		    b->loc->inserted = 1;
 		  val = 0;
 		}
 	    }
 	}
       /* Failure to remove any of the hardware watchpoints comes here.  */
-      if ((is == mark_uninserted) && (b->inserted))
+      if ((is == mark_uninserted) && (b->loc->inserted))
 	warning ("Could not remove hardware watchpoint %d.",
 		 b->number);
 
@@ -1491,7 +1491,7 @@ remove_breakpoint (struct breakpoint *b,
 	    b->type == bp_catch_vfork ||
 	    b->type == bp_catch_exec)
 	   && b->enable_state == bp_enabled
-	   && !b->duplicate)
+	   && !b->loc->duplicate)
     {
       val = -1;
       switch (b->type)
@@ -1511,30 +1511,30 @@ remove_breakpoint (struct breakpoint *b,
 	}
       if (val)
 	return val;
-      b->inserted = (is == mark_inserted);
+      b->loc->inserted = (is == mark_inserted);
     }
   else if ((b->type == bp_catch_catch ||
 	    b->type == bp_catch_throw)
 	   && b->enable_state == bp_enabled
-	   && !b->duplicate)
+	   && !b->loc->duplicate)
     {
 
-      val = target_remove_breakpoint (b->address, b->shadow_contents);
+      val = target_remove_breakpoint (b->loc->address, b->loc->shadow_contents);
       if (val)
 	return val;
-      b->inserted = (is == mark_inserted);
+      b->loc->inserted = (is == mark_inserted);
     }
   else if (ep_is_exception_catchpoint (b)
-	   && b->inserted	/* sometimes previous insert doesn't happen */
+	   && b->loc->inserted	/* sometimes previous insert doesn't happen */
 	   && b->enable_state == bp_enabled
-	   && !b->duplicate)
+	   && !b->loc->duplicate)
     {
 
-      val = target_remove_breakpoint (b->address, b->shadow_contents);
+      val = target_remove_breakpoint (b->loc->address, b->loc->shadow_contents);
       if (val)
 	return val;
 
-      b->inserted = (is == mark_inserted);
+      b->loc->inserted = (is == mark_inserted);
     }
 
   return 0;
@@ -1548,7 +1548,7 @@ mark_breakpoints_out (void)
   struct breakpoint *b;
 
   ALL_BREAKPOINTS (b)
-    b->inserted = 0;
+    b->loc->inserted = 0;
 }
 
 /* Clear the "inserted" flag in all breakpoints and delete any
@@ -1571,7 +1571,7 @@ breakpoint_init_inferior (enum inf_conte
 
   ALL_BREAKPOINTS_SAFE (b, temp)
   {
-    b->inserted = 0;
+    b->loc->inserted = 0;
 
     switch (b->type)
       {
@@ -1647,7 +1647,7 @@ breakpoint_here_p (CORE_ADDR pc)
   ALL_BREAKPOINTS (b)
     if ((b->enable_state == bp_enabled
 	 || b->enable_state == bp_permanent)
-	&& b->address == pc)	/* bp is enabled and matches pc */
+	&& b->loc->address == pc)	/* bp is enabled and matches pc */
       {
 	if (overlay_debugging 
 	    && section_is_overlay (b->section) 
@@ -1673,8 +1673,8 @@ breakpoint_inserted_here_p (CORE_ADDR pc
   struct breakpoint *b;
 
   ALL_BREAKPOINTS (b)
-    if (b->inserted
-	&& b->address == pc)	/* bp is inserted and matches pc */
+    if (b->loc->inserted
+	&& b->loc->address == pc)	/* bp is inserted and matches pc */
     {
       if (overlay_debugging 
 	  && section_is_overlay (b->section) 
@@ -1710,9 +1710,9 @@ deprecated_frame_in_dummy (struct frame_
     /* We need to check the PC as well as the frame on the sparc,
        for signals.exp in the testsuite.  */
 	&& (get_frame_pc (frame)
-	    >= (b->address
+	    >= (b->loc->address
 		- DEPRECATED_SIZEOF_CALL_DUMMY_WORDS / sizeof (LONGEST) * DEPRECATED_REGISTER_SIZE))
-	&& get_frame_pc (frame) <= b->address)
+	&& get_frame_pc (frame) <= b->loc->address)
       return 1;
   }
   return 0;
@@ -1733,7 +1733,7 @@ breakpoint_thread_match (CORE_ADDR pc, p
     if (b->enable_state != bp_disabled
 	&& b->enable_state != bp_shlib_disabled
 	&& b->enable_state != bp_call_disabled
-	&& b->address == pc
+	&& b->loc->address == pc
 	&& (b->thread == -1 || b->thread == thread))
     {
       if (overlay_debugging 
@@ -2027,9 +2027,9 @@ print_it_typical (bpstat bs)
     {
     case bp_breakpoint:
     case bp_hardware_breakpoint:
-      if (bs->breakpoint_at->address != bs->breakpoint_at->requested_address)
-	breakpoint_adjustment_warning (bs->breakpoint_at->requested_address,
-	                               bs->breakpoint_at->address,
+      if (bs->breakpoint_at->loc->address != bs->breakpoint_at->loc->requested_address)
+	breakpoint_adjustment_warning (bs->breakpoint_at->loc->requested_address,
+	                               bs->breakpoint_at->loc->address,
 				       bs->breakpoint_at->number, 1);
       annotate_breakpoint (bs->breakpoint_at->number);
       ui_out_text (uiout, "\nBreakpoint ");
@@ -2548,7 +2548,7 @@ bpstat_stop_status (CORE_ADDR *pc, int n
 	&& b->type != bp_catch_catch
 	&& b->type != bp_catch_throw)	/* a non-watchpoint bp */
       {
-	if (b->address != bp_addr) 	/* address doesn't match */
+	if (b->loc->address != bp_addr) 	/* address doesn't match */
 	  continue;
 	if (overlay_debugging		/* unmapped overlay section */
 	    && section_is_overlay (b->section) 
@@ -2558,7 +2558,7 @@ bpstat_stop_status (CORE_ADDR *pc, int n
 
     if (b->type == bp_hardware_breakpoint)
       {
-	if (b->address != (*pc - DECR_PC_AFTER_HW_BREAK))
+	if (b->loc->address != (*pc - DECR_PC_AFTER_HW_BREAK))
 	  continue;
 	if (overlay_debugging		/* unmapped overlay section */
 	    && section_is_overlay (b->section) 
@@ -3149,7 +3149,7 @@ bpstat_have_active_hw_watchpoints (void)
   struct breakpoint *b;
   ALL_BREAKPOINTS (b)
     if ((b->enable_state == bp_enabled) &&
-	(b->inserted) &&
+	(b->loc->inserted) &&
 	((b->type == bp_hardware_watchpoint) ||
 	 (b->type == bp_read_watchpoint) ||
 	 (b->type == bp_access_watchpoint)))
@@ -3418,13 +3418,13 @@ print_one_breakpoint (struct breakpoint 
 	if (addressprint)
 	  {
 	    annotate_field (4);
-	    ui_out_field_core_addr (uiout, "addr", b->address);
+	    ui_out_field_core_addr (uiout, "addr", b->loc->address);
 	  }
 	annotate_field (5);
-	*last_addr = b->address;
+	*last_addr = b->loc->address;
 	if (b->source_file)
 	  {
-	    sym = find_pc_sect_function (b->address, b->section);
+	    sym = find_pc_sect_function (b->loc->address, b->section);
 	    if (sym)
 	      {
 		ui_out_text (uiout, "in ");
@@ -3439,7 +3439,7 @@ print_one_breakpoint (struct breakpoint 
 	  }
 	else
 	  {
-	    print_address_symbolic (b->address, stb->stream, demangle, "");
+	    print_address_symbolic (b->loc->address, stb->stream, demangle, "");
 	    ui_out_field_stream (uiout, "at", stb);
 	  }
 	break;
@@ -3703,14 +3703,14 @@ describe_other_breakpoints (CORE_ADDR pc
   struct breakpoint *b;
 
   ALL_BREAKPOINTS (b)
-    if (b->address == pc)	/* address match / overlay match */
+    if (b->loc->address == pc)	/* address match / overlay match */
       if (!overlay_debugging || b->section == section)
 	others++;
   if (others > 0)
     {
       printf_filtered ("Note: breakpoint%s ", (others > 1) ? "s" : "");
       ALL_BREAKPOINTS (b)
-	if (b->address == pc)	/* address match / overlay match */
+	if (b->loc->address == pc)	/* address match / overlay match */
 	  if (!overlay_debugging || b->section == section)
 	    {
 	      others--;
@@ -3791,7 +3791,7 @@ check_duplicates (struct breakpoint *bpt
   struct breakpoint *b;
   int count = 0;
   struct breakpoint *perm_bp = 0;
-  CORE_ADDR address = bpt->address;
+  CORE_ADDR address = bpt->loc->address;
   asection *section = bpt->section;
 
   if (! breakpoint_address_is_meaningful (bpt))
@@ -3801,7 +3801,7 @@ check_duplicates (struct breakpoint *bpt
     if (b->enable_state != bp_disabled
 	&& b->enable_state != bp_shlib_disabled
 	&& b->enable_state != bp_call_disabled
-	&& b->address == address	/* address / overlay match */
+	&& b->loc->address == address	/* address / overlay match */
 	&& (!overlay_debugging || b->section == section)
 	&& breakpoint_address_is_meaningful (b))
     {
@@ -3813,7 +3813,7 @@ check_duplicates (struct breakpoint *bpt
 	}
 	
       count++;
-      b->duplicate = count > 1;
+      b->loc->duplicate = count > 1;
     }
 
   /* If we found a permanent breakpoint at this address, go over the
@@ -3821,10 +3821,10 @@ check_duplicates (struct breakpoint *bpt
      duplicates.  */
   if (perm_bp)
     {
-      perm_bp->duplicate = 0;
+      perm_bp->loc->duplicate = 0;
 
       /* Permanent breakpoint should always be inserted.  */
-      if (! perm_bp->inserted)
+      if (! perm_bp->loc->inserted)
 	internal_error (__FILE__, __LINE__,
 			"allegedly permanent breakpoint is not "
 			"actually inserted");
@@ -3832,7 +3832,7 @@ check_duplicates (struct breakpoint *bpt
       ALL_BREAKPOINTS (b)
 	if (b != perm_bp)
 	  {
-	    if (b->inserted)
+	    if (b->loc->inserted)
 	      internal_error (__FILE__, __LINE__,
 			      "another breakpoint was inserted on top of "
 			      "a permanent breakpoint");
@@ -3840,10 +3840,10 @@ check_duplicates (struct breakpoint *bpt
 	    if (b->enable_state != bp_disabled
 		&& b->enable_state != bp_shlib_disabled
 		&& b->enable_state != bp_call_disabled
-		&& b->address == address	/* address / overlay match */
+		&& b->loc->address == address	/* address / overlay match */
 		&& (!overlay_debugging || b->section == section)
 		&& breakpoint_address_is_meaningful (b))
-	      b->duplicate = 1;
+	      b->loc->duplicate = 1;
 	  }
     }
 }
@@ -3918,8 +3918,10 @@ set_raw_breakpoint (struct symtab_and_li
 
   b = (struct breakpoint *) xmalloc (sizeof (struct breakpoint));
   memset (b, 0, sizeof (*b));
-  b->requested_address = sal.pc;
-  b->address = adjust_breakpoint_address (b->requested_address);
+  b->loc = (struct bp_location *) xmalloc (sizeof (struct bp_location));
+  memset (b->loc, 0, sizeof (*b->loc));
+  b->loc->requested_address = sal.pc;
+  b->loc->address = adjust_breakpoint_address (b->loc->requested_address);
   if (sal.symtab == NULL)
     b->source_file = NULL;
   else
@@ -3972,7 +3974,7 @@ make_breakpoint_permanent (struct breakp
   b->enable_state = bp_permanent;
 
   /* By definition, permanent breakpoints are already present in the code.  */
-  b->inserted = 1;
+  b->loc->inserted = 1;
 }
 
 static struct breakpoint *
@@ -4110,7 +4112,7 @@ create_thread_event_breakpoint (CORE_ADD
   
   b->enable_state = bp_enabled;
   /* addr_string has to be used or breakpoint_re_set will delete me.  */
-  xasprintf (&b->addr_string, "*0x%s", paddr (b->address));
+  xasprintf (&b->addr_string, "*0x%s", paddr (b->loc->address));
 
   return b;
 }
@@ -4161,8 +4163,8 @@ disable_breakpoints_in_shlibs (int silen
     if (((b->type == bp_breakpoint) ||
 	 (b->type == bp_hardware_breakpoint)) &&
 	b->enable_state == bp_enabled &&
-	!b->duplicate &&
-	PC_SOLIB (b->address))
+	!b->loc->duplicate &&
+	PC_SOLIB (b->loc->address))
       {
 	b->enable_state = bp_shlib_disabled;
 	if (!silent)
@@ -4193,7 +4195,7 @@ re_enable_breakpoints_in_shlibs (void)
 
       /* Do not reenable the breakpoint if the shared library
          is still not mapped in.  */
-      if (target_read_memory (b->address, buf, 1) == 0)
+      if (target_read_memory (b->loc->address, buf, 1) == 0)
 	b->enable_state = bp_enabled;
     }
 }
@@ -4410,8 +4412,8 @@ set_longjmp_resume_breakpoint (CORE_ADDR
   ALL_BREAKPOINTS (b)
     if (b->type == bp_longjmp_resume)
     {
-      b->requested_address = pc;
-      b->address = adjust_breakpoint_address (b->requested_address);
+      b->loc->requested_address = pc;
+      b->loc->address = adjust_breakpoint_address (b->loc->requested_address);
       b->enable_state = bp_enabled;
       b->frame_id = frame_id;
       check_duplicates (b);
@@ -4611,7 +4613,7 @@ mention (struct breakpoint *b)
       if (addressprint || b->source_file == NULL)
 	{
 	  printf_filtered (" at ");
-	  print_address_numeric (b->address, 1, gdb_stdout);
+	  print_address_numeric (b->loc->address, 1, gdb_stdout);
 	}
       if (b->source_file)
 	printf_filtered (": file %s, line %d.",
@@ -4673,7 +4675,7 @@ create_breakpoints (struct symtabs_and_l
 	else
 	  /* addr_string has to be used or breakpoint_re_set will delete
 	     me.  */
-	  xasprintf (&b->addr_string, "*0x%s", paddr (b->address));
+	  xasprintf (&b->addr_string, "*0x%s", paddr (b->loc->address));
 	b->cond_string = cond_string[i];
 	b->ignore_count = ignore_count;
 	b->enable_state = bp_enabled;
@@ -5526,9 +5528,10 @@ watch_command_1 (char *arg, int accessfl
 	  scope_breakpoint->frame_id = get_frame_id (prev_frame);
 
 	  /* Set the address at which we will stop.  */
-	  scope_breakpoint->requested_address = get_frame_pc (prev_frame);
-	  scope_breakpoint->address =
-	    adjust_breakpoint_address (scope_breakpoint->requested_address);
+	  scope_breakpoint->loc->requested_address
+	    = get_frame_pc (prev_frame);
+	  scope_breakpoint->loc->address
+	    = adjust_breakpoint_address (scope_breakpoint->loc->requested_address);
 
 	  /* The scope breakpoint is related to the watchpoint.  We
 	     will need to act on them together.  */
@@ -6120,10 +6123,10 @@ print_one_exception_catchpoint (struct b
   if (addressprint)
     {
       annotate_field (4);
-      ui_out_field_core_addr (uiout, "addr", b->address);
+      ui_out_field_core_addr (uiout, "addr", b->loc->address);
     }
   annotate_field (5);
-  *last_addr = b->address;
+  *last_addr = b->loc->address;
   if (strstr (b->addr_string, "throw") != NULL)
     ui_out_field_string (uiout, "what", "exception throw");
   else
@@ -6479,7 +6482,7 @@ clear_command (char *arg, int from_tty)
 	      && b->type != bp_read_watchpoint
 	      && b->type != bp_access_watchpoint
 	      /* Not if b is a watchpoint of any sort... */
-	      && (((sal.pc && (b->address == sal.pc)) 
+	      && (((sal.pc && (b->loc->address == sal.pc)) 
 		   && (!section_is_overlay (b->section)
 		       || b->section == sal.section))
 		  /* Yes, if sal.pc matches b (modulo overlays).  */
@@ -6591,7 +6594,7 @@ delete_breakpoint (struct breakpoint *bp
     delete_breakpoint_hook (bpt);
   breakpoint_delete_event (bpt->number);
 
-  if (bpt->inserted)
+  if (bpt->loc->inserted)
     remove_breakpoint (bpt, mark_inserted);
 
   if (breakpoint_chain == bpt)
@@ -6629,7 +6632,7 @@ delete_breakpoint (struct breakpoint *bp
   check_duplicates (bpt);
   /* If this breakpoint was inserted, and there is another breakpoint
      at the same address, we need to insert the other breakpoint.  */
-  if (bpt->inserted
+  if (bpt->loc->inserted
       && bpt->type != bp_hardware_watchpoint
       && bpt->type != bp_read_watchpoint
       && bpt->type != bp_access_watchpoint
@@ -6638,9 +6641,9 @@ delete_breakpoint (struct breakpoint *bp
       && bpt->type != bp_catch_exec)
     {
       ALL_BREAKPOINTS (b)
-	if (b->address == bpt->address
+	if (b->loc->address == bpt->loc->address
 	    && b->section == bpt->section
-	    && !b->duplicate
+	    && !b->loc->duplicate
 	    && b->enable_state != bp_disabled
 	    && b->enable_state != bp_shlib_disabled
 	    && b->enable_state != bp_call_disabled)
@@ -6657,9 +6660,9 @@ delete_breakpoint (struct breakpoint *bp
 			    "a permanent breakpoint");
 
 	  if (b->type == bp_hardware_breakpoint)
-	    val = target_insert_hw_breakpoint (b->address, b->shadow_contents);
+	    val = target_insert_hw_breakpoint (b->loc->address, b->loc->shadow_contents);
 	  else
-	    val = target_insert_breakpoint (b->address, b->shadow_contents);
+	    val = target_insert_breakpoint (b->loc->address, b->loc->shadow_contents);
 
 	  /* If there was an error in the insert, print a message, then stop execution.  */
 	  if (val != 0)
@@ -6679,7 +6682,7 @@ delete_breakpoint (struct breakpoint *bp
 		  {
 		    fprintf_unfiltered (tmp_error_stream, "Cannot insert breakpoint %d.\n", b->number);
 		    fprintf_filtered (tmp_error_stream, "Error accessing memory address ");
-		    print_address_numeric (b->address, 1, tmp_error_stream);
+		    print_address_numeric (b->loc->address, 1, tmp_error_stream);
 		    fprintf_filtered (tmp_error_stream, ": %s.\n",
 				      safe_strerror (val));
 		  }
@@ -6689,7 +6692,7 @@ delete_breakpoint (struct breakpoint *bp
 	      error_stream(tmp_error_stream); 
 	    }
 	  else
-	    b->inserted = 1;
+	    b->loc->inserted = 1;
 	}
     }
 
@@ -6729,6 +6732,7 @@ delete_breakpoint (struct breakpoint *bp
      bp, we mark it as deleted before freeing its storage. */
   bpt->type = bp_none;
 
+  xfree (bpt->loc);
   xfree (bpt);
 }
 
@@ -6870,7 +6874,7 @@ breakpoint_re_set_one (void *bint)
 	    }
 
 	  /* We need to re-set the breakpoint if the address changes... */
-	  if (b->address != sals.sals[i].pc
+	  if (b->loc->address != sals.sals[i].pc
 	  /* ...or new and old breakpoints both have source files, and
 	     the source file name or the line number changes...  */
 	      || (b->source_file != NULL
@@ -6892,8 +6896,9 @@ breakpoint_re_set_one (void *bint)
 		  savestring (sals.sals[i].symtab->filename,
 			      strlen (sals.sals[i].symtab->filename));
 	      b->line_number = sals.sals[i].line;
-	      b->requested_address = sals.sals[i].pc;
-	      b->address = adjust_breakpoint_address (b->requested_address);
+	      b->loc->requested_address = sals.sals[i].pc;
+	      b->loc->address
+	        = adjust_breakpoint_address (b->loc->requested_address);
 
 	      /* Used to check for duplicates here, but that can
 	         cause trouble, as it doesn't check for disabled
Index: gdb/breakpoint.h
===================================================================
--- gdb.orig/breakpoint.h	2003-11-05 11:53:25.000000000 -0500
+++ gdb/breakpoint.h	2003-11-05 11:53:26.000000000 -0500
@@ -184,6 +184,72 @@ enum target_hw_bp_type
     hw_execute = 3		/* Execute HW breakpoint */
   };
 
+/* GDB maintains two types of information about each breakpoint (or
+   watchpoint, or other related event).  The first type corresponds
+   to struct breakpoint; this is a relatively high-level structure
+   which contains the source location(s), stopping conditions, user
+   commands to execute when the breakpoint is hit, and so forth.
+
+   The second type of information corresponds to struct bp_location.
+   Each breakpoint has one or (eventually) more locations associated
+   with it, which represent target-specific and machine-specific
+   mechanisms for stopping the program.  For instance, a watchpoint
+   expression may require multiple hardware watchpoints in order to
+   catch all changes in the value of the expression being watched.  */
+
+enum bp_loc_type
+{
+  bp_loc_software_breakpoint,
+  bp_loc_hardware_breakpoint,
+  bp_loc_hardware_watchpoint,
+  bp_loc_other			/* Miscellaneous...  */
+};
+
+struct bp_location
+{
+  /* Type of this breakpoint location.  */
+  enum bp_loc_type loc_type;
+
+  /* Each breakpoint location must belong to exactly one higher-level
+     breakpoint.  This and the DUPLICATE flag are more straightforward
+     than reference counting.  */
+  struct breakpoint *owner;
+
+  /* Nonzero if this breakpoint is now inserted.  */
+  char inserted;
+
+  /* Nonzero if this is not the first breakpoint in the list
+     for the given address.  */
+  char duplicate;
+
+  /* If we someday support real thread-specific breakpoints, then
+     the breakpoint location will need a thread identifier.  */
+
+  /* Data for specific breakpoint types.  These could be a union, but
+     simplicity is more important than memory usage for breakpoints.  */
+
+  /* Note that zero is a perfectly valid code address on some platforms
+     (for example, the mn10200 (OBSOLETE) and mn10300 simulators).  NULL
+     is not a special value for this field.  Valid for all types except
+     bp_loc_other.  */
+  CORE_ADDR address;
+
+  /* "Real" contents of byte where breakpoint has been inserted.
+     Valid only when breakpoints are in the program.  Under the complete
+     control of the target insert_breakpoint and remove_breakpoint routines.
+     No other code should assume anything about the value(s) here.
+     Valid only for bp_loc_software_breakpoint.  */
+  char shadow_contents[BREAKPOINT_MAX];
+
+  /* Address at which breakpoint was requested, either by the user or
+     by GDB for internal breakpoints.  This will usually be the same
+     as ``address'' (above) except for cases in which
+     ADJUST_BREAKPOINT_ADDRESS has computed a different address at
+     which to place the breakpoint in order to comply with a
+     processor's architectual constraints.  */
+  CORE_ADDR requested_address;
+};
+
 /* This structure is a collection of function pointers that, if available,
    will be called instead of the performing the default action for this
    bptype.  */
@@ -222,18 +288,8 @@ struct breakpoint
     /* Number assigned to distinguish breakpoints.  */
     int number;
 
-    /* Address to break at.  Note that zero is a perfectly valid code
-       address on some platforms (for example, the and mn10300
-       simulators).  NULL is not a special value for this field.  */
-    CORE_ADDR address;
-
-    /* Address at which breakpoint was requested, either by the user or
-       by GDB for internal breakpoints.  This will usually be the same
-       as ``address'' (above) except for cases in which
-       ADJUST_BREAKPOINT_ADDRESS has computed a different address at
-       which to place the breakpoint in order to comply with a
-       processor's architectual constraints.  */
-    CORE_ADDR requested_address;
+    /* Location(s) associated with this high-level breakpoint.  */
+    struct bp_location *loc;
 
     /* Line number of this address.  */
 
@@ -249,16 +305,6 @@ struct breakpoint
     /* Number of stops at this breakpoint that should
        be continued automatically before really stopping.  */
     int ignore_count;
-    /* "Real" contents of byte where breakpoint has been inserted.
-       Valid only when breakpoints are in the program.  Under the complete
-       control of the target insert_breakpoint and remove_breakpoint routines.
-       No other code should assume anything about the value(s) here.  */
-    char shadow_contents[BREAKPOINT_MAX];
-    /* Nonzero if this breakpoint is now inserted.  */
-    char inserted;
-    /* Nonzero if this is not the first breakpoint in the list
-       for the given address.  */
-    char duplicate;
     /* Chain of command lines to execute when this breakpoint is hit.  */
     struct command_line *commands;
     /* Stack depth (address of frame).  If nonzero, break only if fp


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2003-11-06 17:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-10-08 17:02 RFA: Breakpoint infrastructure cleanups [1/8] - define impl_breakpoint Daniel Jacobowitz
2003-10-08 18:17 ` Jim Blandy
2003-10-14  1:30   ` Daniel Jacobowitz
2003-10-14 15:31     ` Andrew Cagney
2003-10-14 15:36       ` Daniel Jacobowitz
2003-10-14 16:03   ` Andrew Cagney
2003-10-08 18:21 ` Eli Zaretskii
2003-10-08 19:11   ` Daniel Jacobowitz
2003-10-09  6:04     ` Eli Zaretskii
2003-10-09 19:16     ` Michael Snyder
2003-11-06 17:57 ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox