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]

[PATCH] Make breakpoint subclasses inherit from breakpoint, add virtual destructor


From: Simon Marchi <simon.marchi@polymtl.ca>

Tom recently mentioned on IRC how breakpoint deallocation looked fishy.  A
syscall catchpoint, for example, is created with "new syscall_catchpoint", but
free'd using "delete bpt", where bpt is a breakpoint *.  I had this patch lying
around in a branch, so I decided to post it by itself.

---

I want to replace the vectors in the various breakpoint subclasses by
std::vector.  The problem right now is that while breakpoint
subclasses are constructed using new, they are not properly deleted.
The only place breakpoints are deleted is through a breakpoint pointer
in delete_breakpoint.  This means that even if I add a destructor in a
subclass (e.g. syscall_catchpoint), it's not going to be called, for two
reasons:

1. The destructor of breakpoint needs to be virtual if we want the
   destructors from the subclasses to be called.
2. The subclasses need to be actual subclasses, not just include the
   base class as a field.

It turns out at #2 generates a lot of small changes (removing "base."
everywhere), but it makes the code generally a bit nicer.

gdb/ChangeLog:

	* ada-lang.c (struct ada_catchpoint): Inherit from struct
	breakpoint.
	<base>: Remove.
	(create_excep_cond_exprs): Adjust.
	(create_ada_exception_catchpoint): Adjust.
	* break-catch-sig.c (struct signal_catchpoint): Inherit from
	struct breakpoint.
	<base>: Remove.
	(create_signal_catchpoint): Adjust.
	* break-catch-syscall.c (UNKNOWN): Adjust.
	(create_syscall_event_catchpoint): Adjust.
	* break-catch-throw.c (static): Adjust.
	(handle_gnu_v3_exceptions): Adjust.
	* breakpoint.c (is_watchpoint): Adjust.
	(watchpoint_in_thread_scope): Adjust.
	(update_watchpoint): Adjust.
	(watchpoint_check): Adjust.
	(bpstat_check_watchpoint): Adjust.
	(disable_breakpoints_in_freed_objfile): Adjust.
	(print_recreate_catch_vfork): Adjust.
	(breakpoint_hit_catch_solib): Adjust.
	(add_solib_catchpoint): Adjust.
	(create_fork_vfork_event_catchpoint): Adjust.
	(create_breakpoint_sal): Adjust.
	(create_breakpoint): Adjust.
	(static): Adjust.
	(watch_command_1): Adjust.
	(catch_exec_command_1): Adjust.
	(strace_marker_create_breakpoints_sal): Adjust.
	(create_tracepoint_from_upload): Adjust.
	(static): Adjust.
	* breakpoint.h (extern): Adjust.
	(struct breakpoint): Adjust.
	(extern): Adjust.
	* ctf.c (ctf_get_traceframe_address): Adjust.
	* mi/mi-cmd-break.c (mi_cmd_break_passcount): Adjust.
	* remote.c (remote_get_tracepoint_status): Adjust.
	* tracefile-tfile.c (tfile_get_traceframe_address): Adjust.
	* tracefile.c (tracefile_fetch_registers): Adjust.
	* tracepoint.c (actions_command): Adjust.
	(validate_actionline): Adjust.
	(tfind_1): Adjust.
	(get_traceframe_location): Adjust.
	(find_matching_tracepoint_location): Adjust.
	(merge_uploaded_tracepoints): Adjust.
	(parse_tracepoint_status): Adjust.
---
 gdb/ada-lang.c            |  21 ++----
 gdb/break-catch-sig.c     |  14 +---
 gdb/break-catch-syscall.c |  13 +---
 gdb/break-catch-throw.c   |  14 ++--
 gdb/breakpoint.c          | 187 ++++++++++++++++++++--------------------------
 gdb/breakpoint.h          |  22 +++---
 gdb/ctf.c                 |   4 +-
 gdb/mi/mi-cmd-break.c     |   2 +-
 gdb/remote.c              |   4 +-
 gdb/tracefile-tfile.c     |   4 +-
 gdb/tracefile.c           |  10 +--
 gdb/tracepoint.c          |  20 ++---
 12 files changed, 132 insertions(+), 183 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 58c8a2efc9..f4eb429dfe 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12257,15 +12257,10 @@ static const struct bp_location_ops ada_catchpoint_location_ops =
   ada_catchpoint_location_dtor
 };
 
-/* An instance of this type is used to represent an Ada catchpoint.
-   It includes a "struct breakpoint" as a kind of base class; users
-   downcast to "struct breakpoint *" when needed.  */
+/* An instance of this type is used to represent an Ada catchpoint.  */
 
-struct ada_catchpoint
+struct ada_catchpoint : public breakpoint
 {
-  /* The base class.  */
-  struct breakpoint base;
-
   /* The name of the specific exception the user specified.  */
   char *excep_string;
 };
@@ -12285,7 +12280,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
     return;
 
   /* Same if there are no locations... */
-  if (c->base.loc == NULL)
+  if (c->loc == NULL)
     return;
 
   /* Compute the condition expression in text form, from the specific
@@ -12295,7 +12290,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
 
   /* Iterate over all the catchpoint's locations, and parse an
      expression for each.  */
-  for (bl = c->base.loc; bl != NULL; bl = bl->next)
+  for (bl = c->loc; bl != NULL; bl = bl->next)
     {
       struct ada_catchpoint_location *ada_loc
 	= (struct ada_catchpoint_location *) bl;
@@ -12316,7 +12311,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
 	    {
 	      warning (_("failed to reevaluate internal exception condition "
 			 "for catchpoint %d: %s"),
-		       c->base.number, e.message);
+		       c->number, e.message);
 	    }
 	  END_CATCH
 	}
@@ -13060,13 +13055,13 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
     = ada_exception_sal (ex_kind, excep_string, &addr_string, &ops);
 
   c = new ada_catchpoint ();
-  init_ada_exception_breakpoint (&c->base, gdbarch, sal, addr_string,
+  init_ada_exception_breakpoint (c, gdbarch, sal, addr_string,
 				 ops, tempflag, disabled, from_tty);
   c->excep_string = excep_string;
   create_excep_cond_exprs (c);
   if (cond_string != NULL)
-    set_breakpoint_condition (&c->base, cond_string, from_tty);
-  install_breakpoint (0, &c->base, 1);
+    set_breakpoint_condition (c, cond_string, from_tty);
+  install_breakpoint (0, c, 1);
 }
 
 /* Implement the "catch exception" command.  */
diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index 5f02fe6230..03bf319ae6 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -38,17 +38,11 @@ typedef enum gdb_signal gdb_signal_type;
 DEF_VEC_I (gdb_signal_type);
 
 /* An instance of this type is used to represent a signal catchpoint.
-   It includes a "struct breakpoint" as a kind of base class; users
-   downcast to "struct breakpoint *" when needed.  A breakpoint is
-   really of this type iff its ops pointer points to
+   A breakpoint is really of this type iff its ops pointer points to
    SIGNAL_CATCHPOINT_OPS.  */
 
-struct signal_catchpoint
+struct signal_catchpoint : public breakpoint
 {
-  /* The base class.  */
-
-  struct breakpoint base;
-
   /* Signal numbers used for the 'catch signal' feature.  If no signal
      has been specified for filtering, its value is NULL.  Otherwise,
      it holds a list of all signals to be caught.  */
@@ -372,11 +366,11 @@ create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
   struct gdbarch *gdbarch = get_current_arch ();
 
   c = new signal_catchpoint ();
-  init_catchpoint (&c->base, gdbarch, tempflag, NULL, &signal_catchpoint_ops);
+  init_catchpoint (c, gdbarch, tempflag, NULL, &signal_catchpoint_ops);
   c->signals_to_be_caught = filter;
   c->catch_all = catch_all;
 
-  install_breakpoint (0, &c->base, 1);
+  install_breakpoint (0, c, 1);
 }
 
 
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index 322f680ad8..3d0f77bb8e 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -31,16 +31,11 @@
 #include "xml-syscall.h"
 
 /* An instance of this type is used to represent a syscall catchpoint.
-   It includes a "struct breakpoint" as a kind of base class; users
-   downcast to "struct breakpoint *" when needed.  A breakpoint is
-   really of this type iff its ops pointer points to
+   A breakpoint is really of this type iff its ops pointer points to
    CATCH_SYSCALL_BREAKPOINT_OPS.  */
 
-struct syscall_catchpoint
+struct syscall_catchpoint : public breakpoint
 {
-  /* The base class.  */
-  struct breakpoint base;
-
   /* Syscall numbers used for the 'catch syscall' feature.  If no
      syscall has been specified for filtering, its value is NULL.
      Otherwise, it holds a list of all syscalls to be caught.  The
@@ -434,10 +429,10 @@ create_syscall_event_catchpoint (int tempflag, VEC(int) *filter,
   struct gdbarch *gdbarch = get_current_arch ();
 
   c = new syscall_catchpoint ();
-  init_catchpoint (&c->base, gdbarch, tempflag, NULL, ops);
+  init_catchpoint (c, gdbarch, tempflag, NULL, ops);
   c->syscalls_to_be_caught = filter;
 
-  install_breakpoint (0, &c->base, 1);
+  install_breakpoint (0, c, 1);
 }
 
 /* Splits the argument using space as delimiter.  Returns an xmalloc'd
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index 2714bbf26b..441e2d0c13 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -74,12 +74,8 @@ static struct breakpoint_ops gnu_v3_exception_catchpoint_ops;
 
 /* The type of an exception catchpoint.  */
 
-struct exception_catchpoint
+struct exception_catchpoint : public breakpoint
 {
-  /* The base class.  */
-
-  struct breakpoint base;
-
   /* The kind of exception catchpoint.  */
 
   enum exception_event_kind kind;
@@ -396,18 +392,18 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx,
 
   std::unique_ptr<exception_catchpoint> cp (new exception_catchpoint ());
 
-  init_catchpoint (&cp->base, get_current_arch (), tempflag, cond_string,
+  init_catchpoint (cp.get (), get_current_arch (), tempflag, cond_string,
 		   &gnu_v3_exception_catchpoint_ops);
   /* We need to reset 'type' in order for code in breakpoint.c to do
      the right thing.  */
-  cp->base.type = bp_breakpoint;
+  cp->type = bp_breakpoint;
   cp->kind = ex_event;
   cp->exception_rx = except_rx;
   cp->pattern = pattern;
 
-  re_set_exception_catchpoint (&cp->base);
+  re_set_exception_catchpoint (cp.get ());
 
-  install_breakpoint (0, &cp->base, 1);
+  install_breakpoint (0, cp.get (), 1);
   cp.release ();
 }
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index e8d8d09df5..1459be64a7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1706,7 +1706,7 @@ is_watchpoint (const struct breakpoint *bpt)
 static int
 watchpoint_in_thread_scope (struct watchpoint *b)
 {
-  return (b->base.pspace == current_program_space
+  return (b->pspace == current_program_space
 	  && (ptid_equal (b->watchpoint_thread, null_ptid)
 	      || (ptid_equal (inferior_ptid, b->watchpoint_thread)
 		  && !is_executing (inferior_ptid))));
@@ -1718,17 +1718,15 @@ watchpoint_in_thread_scope (struct watchpoint *b)
 static void
 watchpoint_del_at_next_stop (struct watchpoint *w)
 {
-  struct breakpoint *b = &w->base;
-
-  if (b->related_breakpoint != b)
+  if (w->related_breakpoint != w)
     {
-      gdb_assert (b->related_breakpoint->type == bp_watchpoint_scope);
-      gdb_assert (b->related_breakpoint->related_breakpoint == b);
-      b->related_breakpoint->disposition = disp_del_at_next_stop;
-      b->related_breakpoint->related_breakpoint = b->related_breakpoint;
-      b->related_breakpoint = b;
+      gdb_assert (w->related_breakpoint->type == bp_watchpoint_scope);
+      gdb_assert (w->related_breakpoint->related_breakpoint == w);
+      w->related_breakpoint->disposition = disp_del_at_next_stop;
+      w->related_breakpoint->related_breakpoint = w->related_breakpoint;
+      w->related_breakpoint = w;
     }
-  b->disposition = disp_del_at_next_stop;
+  w->disposition = disp_del_at_next_stop;
 }
 
 /* Extract a bitfield value from value VAL using the bit parameters contained in
@@ -1849,7 +1847,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
   if (!watchpoint_in_thread_scope (b))
     return;
 
-  if (b->base.disposition == disp_del_at_next_stop)
+  if (b->disposition == disp_del_at_next_stop)
     return;
  
   frame_saved = 0;
@@ -1887,7 +1885,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
   /* We don't free locations.  They are stored in the bp_location array
      and update_global_location_list will eventually delete them and
      remove breakpoints if needed.  */
-  b->base.loc = NULL;
+  b->loc = NULL;
 
   if (within_current_scope && reparse)
     {
@@ -1907,11 +1905,11 @@ update_watchpoint (struct watchpoint *b, int reparse)
       /* Note that unlike with breakpoints, the watchpoint's condition
 	 expression is stored in the breakpoint object, not in the
 	 locations (re)created below.  */
-      if (b->base.cond_string != NULL)
+      if (b->cond_string != NULL)
 	{
 	  b->cond_exp.reset ();
 
-	  s = b->base.cond_string;
+	  s = b->cond_string;
 	  b->cond_exp = parse_exp_1 (&s, 0, b->cond_exp_valid_block, 0);
 	}
     }
@@ -1928,8 +1926,8 @@ update_watchpoint (struct watchpoint *b, int reparse)
 	 the target gains execution, through breakpoint_re_set.  */
       if (!can_use_hw_watchpoints)
 	{
-	  if (b->base.ops->works_in_software_mode (&b->base))
-	    b->base.type = bp_watchpoint;
+	  if (b->ops->works_in_software_mode (b))
+	    b->type = bp_watchpoint;
 	  else
 	    error (_("Can't set read/access watchpoint when "
 		     "hardware watchpoints are disabled."));
@@ -1949,7 +1947,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
 	 happens, the code that reports it updates b->val directly.
 	 We don't keep track of the memory value for masked
 	 watchpoints.  */
-      if (!b->val_valid && !is_masked_watchpoint (&b->base))
+      if (!b->val_valid && !is_masked_watchpoint (b))
 	{
 	  if (b->val_bitsize != 0)
 	    {
@@ -2012,13 +2010,13 @@ update_watchpoint (struct watchpoint *b, int reparse)
 		    }
 
 		  type = hw_write;
-		  if (b->base.type == bp_read_watchpoint)
+		  if (b->type == bp_read_watchpoint)
 		    type = hw_read;
-		  else if (b->base.type == bp_access_watchpoint)
+		  else if (b->type == bp_access_watchpoint)
 		    type = hw_access;
 
-		  loc = allocate_bp_location (&b->base);
-		  for (tmp = &(b->base.loc); *tmp != NULL; tmp = &((*tmp)->next))
+		  loc = allocate_bp_location (b);
+		  for (tmp = &(b->loc); *tmp != NULL; tmp = &((*tmp)->next))
 		    ;
 		  *tmp = loc;
 		  loc->gdbarch = get_type_arch (value_type (v));
@@ -2068,7 +2066,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
 	      /* If this is a software watchpoint, we try to turn it
 		 to a hardware one -- count resources as if B was of
 		 hardware watchpoint type.  */
-	      type = b->base.type;
+	      type = b->type;
 	      if (type == bp_watchpoint)
 		type = bp_hardware_watchpoint;
 
@@ -2079,16 +2077,16 @@ update_watchpoint (struct watchpoint *b, int reparse)
 		 manually.  */
 
 	      /* Count resources used by all watchpoints except B.  */
-	      i = hw_watchpoint_used_count_others (&b->base, type, &other_type_used);
+	      i = hw_watchpoint_used_count_others (b, type, &other_type_used);
 
 	      /* Add in the resources needed for B.  */
-	      i += hw_watchpoint_use_count (&b->base);
+	      i += hw_watchpoint_use_count (b);
 
 	      target_resources_ok
 		= target_can_use_hardware_watchpoint (type, i, other_type_used);
 	      if (target_resources_ok <= 0)
 		{
-		  int sw_mode = b->base.ops->works_in_software_mode (&b->base);
+		  int sw_mode = b->ops->works_in_software_mode (b);
 
 		  if (target_resources_ok == 0 && !sw_mode)
 		    error (_("Target does not support this type of "
@@ -2098,7 +2096,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
 			     "resources for this watchpoint."));
 
 		  /* Downgrade to software watchpoint.  */
-		  b->base.type = bp_watchpoint;
+		  b->type = bp_watchpoint;
 		}
 	      else
 		{
@@ -2106,10 +2104,10 @@ update_watchpoint (struct watchpoint *b, int reparse)
 		     found we have enough resources to turn it to a
 		     hardware watchpoint.  Otherwise, this is a
 		     nop.  */
-		  b->base.type = type;
+		  b->type = type;
 		}
 	    }
-	  else if (!b->base.ops->works_in_software_mode (&b->base))
+	  else if (!b->ops->works_in_software_mode (b))
 	    {
 	      if (!can_use_hw_watchpoints)
 		error (_("Can't set read/access watchpoint when "
@@ -2119,11 +2117,11 @@ update_watchpoint (struct watchpoint *b, int reparse)
 			 "read/access watchpoint."));
 	    }
 	  else
-	    b->base.type = bp_watchpoint;
+	    b->type = bp_watchpoint;
 
-	  loc_type = (b->base.type == bp_watchpoint? bp_loc_other
+	  loc_type = (b->type == bp_watchpoint? bp_loc_other
 		      : bp_loc_hardware_watchpoint);
-	  for (bl = b->base.loc; bl; bl = bl->next)
+	  for (bl = b->loc; bl; bl = bl->next)
 	    bl->loc_type = loc_type;
 	}
 
@@ -2138,15 +2136,15 @@ update_watchpoint (struct watchpoint *b, int reparse)
 	 above left it without any location set up.  But,
 	 bpstat_stop_status requires a location to be able to report
 	 stops, so make sure there's at least a dummy one.  */
-      if (b->base.type == bp_watchpoint && b->base.loc == NULL)
-	software_watchpoint_add_no_memory_location (&b->base, frame_pspace);
+      if (b->type == bp_watchpoint && b->loc == NULL)
+	software_watchpoint_add_no_memory_location (b, frame_pspace);
     }
   else if (!within_current_scope)
     {
       printf_filtered (_("\
 Watchpoint %d deleted because the program has left the block\n\
 in which its expression is valid.\n"),
-		       b->base.number);
+		       b->number);
       watchpoint_del_at_next_stop (b);
     }
 
@@ -5182,7 +5180,7 @@ watchpoint_check (void *p)
       struct value *mark;
       struct value *new_val;
 
-      if (is_masked_watchpoint (&b->base))
+      if (is_masked_watchpoint (b))
 	/* Since we don't know the exact trigger address (from
 	   stopped_data_address), just tell the user we've triggered
 	   a mask watchpoint.  */
@@ -5242,13 +5240,13 @@ watchpoint_check (void *p)
 	    uiout->field_string
 	      ("reason", async_reason_lookup (EXEC_ASYNC_WATCHPOINT_SCOPE));
 	  uiout->text ("\nWatchpoint ");
-	  uiout->field_int ("wpnum", b->base.number);
+	  uiout->field_int ("wpnum", b->number);
 	  uiout->text (" deleted because the program has left the block in\n"
 		       "which its expression is valid.\n");
 	}
 
       /* Make sure the watchpoint's commands aren't executed.  */
-      decref_counted_command_line (&b->base.commands);
+      decref_counted_command_line (&b->commands);
       watchpoint_del_at_next_stop (b);
 
       return WP_DELETED;
@@ -5290,7 +5288,7 @@ bpstat_check_watchpoint (bpstat bs)
     {
       int must_check_value = 0;
       
-      if (b->base.type == bp_watchpoint)
+      if (b->type == bp_watchpoint)
 	/* For a software watchpoint, we must always check the
 	   watched value.  */
 	must_check_value = 1;
@@ -5300,7 +5298,7 @@ bpstat_check_watchpoint (bpstat bs)
 	   this watchpoint.  */
 	must_check_value = 1;
       else if (b->watchpoint_triggered == watch_triggered_unknown
-	       && b->base.type == bp_hardware_watchpoint)
+	       && b->type == bp_hardware_watchpoint)
 	/* We were stopped by a hardware watchpoint, but the target could
 	   not report the data address.  We must check the watchpoint's
 	   value.  Access and read watchpoints are out of luck; without
@@ -5311,7 +5309,7 @@ bpstat_check_watchpoint (bpstat bs)
 	{
 	  char *message
 	    = xstrprintf ("Error evaluating expression for watchpoint %d\n",
-			  b->base.number);
+			  b->number);
 	  struct cleanup *cleanups = make_cleanup (xfree, message);
 	  int e = catch_errors (watchpoint_check, bs, message,
 				RETURN_MASK_ALL);
@@ -5328,7 +5326,7 @@ bpstat_check_watchpoint (bpstat bs)
 	      bs->stop = 0;
 	      break;
 	    case WP_VALUE_CHANGED:
-	      if (b->base.type == bp_read_watchpoint)
+	      if (b->type == bp_read_watchpoint)
 		{
 		  /* There are two cases to consider here:
 
@@ -5399,8 +5397,8 @@ bpstat_check_watchpoint (bpstat bs)
 		}
 	      break;
 	    case WP_VALUE_NOT_CHANGED:
-	      if (b->base.type == bp_hardware_watchpoint
-		  || b->base.type == bp_watchpoint)
+	      if (b->type == bp_hardware_watchpoint
+		  || b->type == bp_watchpoint)
 		{
 		  /* Don't stop: write watchpoints shouldn't fire if
 		     the value hasn't changed.  */
@@ -5417,7 +5415,7 @@ bpstat_check_watchpoint (bpstat bs)
 		SWITCH_THRU_ALL_UIS ()
 	          {
 		    printf_filtered (_("Watchpoint %d deleted.\n"),
-				     b->base.number);
+				     b->number);
 		  }
 		watchpoint_del_at_next_stop (b);
 		/* We've already printed what needs to be printed.  */
@@ -8016,16 +8014,11 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile)
 /* FORK & VFORK catchpoints.  */
 
 /* An instance of this type is used to represent a fork or vfork
-   catchpoint.  It includes a "struct breakpoint" as a kind of base
-   class; users downcast to "struct breakpoint *" when needed.  A
-   breakpoint is really of this type iff its ops pointer points to
-   CATCH_FORK_BREAKPOINT_OPS.  */
+   catchpoint.  A breakpoint is really of this type iff its ops pointer points
+   to CATCH_FORK_BREAKPOINT_OPS.  */
 
-struct fork_catchpoint
+struct fork_catchpoint : public breakpoint
 {
-  /* The base class.  */
-  struct breakpoint base;
-
   /* Process id of a child process whose forking triggered this
      catchpoint.  This field is only valid immediately after this
      catchpoint has triggered.  */
@@ -8264,16 +8257,11 @@ print_recreate_catch_vfork (struct breakpoint *b, struct ui_file *fp)
 static struct breakpoint_ops catch_vfork_breakpoint_ops;
 
 /* An instance of this type is used to represent an solib catchpoint.
-   It includes a "struct breakpoint" as a kind of base class; users
-   downcast to "struct breakpoint *" when needed.  A breakpoint is
-   really of this type iff its ops pointer points to
+   A breakpoint is really of this type iff its ops pointer points to
    CATCH_SOLIB_BREAKPOINT_OPS.  */
 
-struct solib_catchpoint
+struct solib_catchpoint : public breakpoint
 {
-  /* The base class.  */
-  struct breakpoint base;
-
   /* True for "catch load", false for "catch unload".  */
   unsigned char is_load;
 
@@ -8329,7 +8317,7 @@ breakpoint_hit_catch_solib (const struct bp_location *bl,
     if (other->type != bp_shlib_event)
       continue;
 
-    if (self->base.pspace != NULL && other->pspace != self->base.pspace)
+    if (self->pspace != NULL && other->pspace != self->pspace)
       continue;
 
     for (other_bl = other->loc; other_bl != NULL; other_bl = other_bl->next)
@@ -8503,13 +8491,13 @@ add_solib_catchpoint (const char *arg, int is_load, int is_temp, int enabled)
     }
 
   c->is_load = is_load;
-  init_catchpoint (&c->base, gdbarch, is_temp, NULL,
+  init_catchpoint (c, gdbarch, is_temp, NULL,
 		   &catch_solib_breakpoint_ops);
 
-  c->base.enable_state = enabled ? bp_enabled : bp_disabled;
+  c->enable_state = enabled ? bp_enabled : bp_disabled;
 
   discard_cleanups (cleanup);
-  install_breakpoint (0, &c->base, 1);
+  install_breakpoint (0, c, 1);
 }
 
 /* A helper function that does all the work for "catch load" and
@@ -8585,26 +8573,21 @@ create_fork_vfork_event_catchpoint (struct gdbarch *gdbarch,
 {
   struct fork_catchpoint *c = new fork_catchpoint ();
 
-  init_catchpoint (&c->base, gdbarch, tempflag, cond_string, ops);
+  init_catchpoint (c, gdbarch, tempflag, cond_string, ops);
 
   c->forked_inferior_pid = null_ptid;
 
-  install_breakpoint (0, &c->base, 1);
+  install_breakpoint (0, c, 1);
 }
 
 /* Exec catchpoints.  */
 
 /* An instance of this type is used to represent an exec catchpoint.
-   It includes a "struct breakpoint" as a kind of base class; users
-   downcast to "struct breakpoint *" when needed.  A breakpoint is
-   really of this type iff its ops pointer points to
+   A breakpoint is really of this type iff its ops pointer points to
    CATCH_EXEC_BREAKPOINT_OPS.  */
 
-struct exec_catchpoint
+struct exec_catchpoint : public breakpoint
 {
-  /* The base class.  */
-  struct breakpoint base;
-
   /* Filename of a program whose exec triggered this catchpoint.
      This field is only valid immediately after this catchpoint has
      triggered.  */
@@ -9338,12 +9321,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
   struct cleanup *old_chain;
 
   if (is_tracepoint_type (type))
-    {
-      struct tracepoint *t;
-
-      t = new tracepoint ();
-      b = &t->base;
-    }
+    b = new tracepoint ();
   else
     b = new breakpoint ();
 
@@ -9831,12 +9809,7 @@ create_breakpoint (struct gdbarch *gdbarch,
       struct breakpoint *b;
 
       if (is_tracepoint_type (type_wanted))
-	{
-	  struct tracepoint *t;
-
-	  t = new tracepoint ();
-	  b = &t->base;
-	}
+	b = new tracepoint ();
       else
 	b = new breakpoint ();
 
@@ -11027,7 +11000,7 @@ static void
 watch_command_1 (const char *arg, int accessflag, int from_tty,
 		 int just_location, int internal)
 {
-  struct breakpoint *b, *scope_breakpoint = NULL;
+  struct breakpoint *scope_breakpoint = NULL;
   const struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL;
   struct value *val, *mark, *result;
   int saved_bitpos = 0, saved_bitsize = 0;
@@ -11266,16 +11239,16 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
     bp_type = bp_hardware_watchpoint;
 
   w = new watchpoint ();
-  b = &w->base;
+
   if (use_mask)
-    init_raw_breakpoint_without_location (b, NULL, bp_type,
+    init_raw_breakpoint_without_location (w, NULL, bp_type,
 					  &masked_watchpoint_breakpoint_ops);
   else
-    init_raw_breakpoint_without_location (b, NULL, bp_type,
+    init_raw_breakpoint_without_location (w, NULL, bp_type,
 					  &watchpoint_breakpoint_ops);
-  b->thread = thread;
-  b->disposition = disp_donttouch;
-  b->pspace = current_program_space;
+  w->thread = thread;
+  w->disposition = disp_donttouch;
+  w->pspace = current_program_space;
   w->exp = std::move (exp);
   w->exp_valid_block = exp_valid_block;
   w->cond_exp_valid_block = cond_exp_valid_block;
@@ -11295,7 +11268,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
 				  (int) (exp_end - exp_start), exp_start);
 
       /* The above expression is in C.  */
-      b->language = language_c;
+      w->language = language_c;
     }
   else
     w->exp_string = savestring (exp_start, exp_end - exp_start);
@@ -11313,9 +11286,9 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
     }
 
   if (cond_start)
-    b->cond_string = savestring (cond_start, cond_end - cond_start);
+    w->cond_string = savestring (cond_start, cond_end - cond_start);
   else
-    b->cond_string = 0;
+    w->cond_string = 0;
 
   if (frame_id_p (watchpoint_frame))
     {
@@ -11332,8 +11305,8 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
     {
       /* The scope breakpoint is related to the watchpoint.  We will
 	 need to act on them together.  */
-      b->related_breakpoint = scope_breakpoint;
-      scope_breakpoint->related_breakpoint = b;
+      w->related_breakpoint = scope_breakpoint;
+      scope_breakpoint->related_breakpoint = w;
     }
 
   if (!just_location)
@@ -11347,12 +11320,12 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
     }
   CATCH (e, RETURN_MASK_ALL)
     {
-      delete_breakpoint (b);
+      delete_breakpoint (w);
       throw_exception (e);
     }
   END_CATCH
 
-  install_breakpoint (internal, b, 1);
+  install_breakpoint (internal, w, 1);
   do_cleanups (back_to);
 }
 
@@ -11822,11 +11795,11 @@ catch_exec_command_1 (char *arg_entry, int from_tty,
     error (_("Junk at end of arguments."));
 
   c = new exec_catchpoint ();
-  init_catchpoint (&c->base, gdbarch, tempflag, cond_string,
+  init_catchpoint (c, gdbarch, tempflag, cond_string,
 		   &catch_exec_breakpoint_ops);
   c->exec_pathname = NULL;
 
-  install_breakpoint (0, &c->base, 1);
+  install_breakpoint (0, c, 1);
 }
 
 void
@@ -13665,7 +13638,7 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
       location = copy_event_location (canonical->location.get ());
 
       tp = new tracepoint ();
-      init_breakpoint_sal (&tp->base, gdbarch, expanded,
+      init_breakpoint_sal (tp, gdbarch, expanded,
 			   std::move (location), NULL,
 			   cond_string, extra_string,
 			   type_wanted, disposition,
@@ -13680,7 +13653,7 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
 	 corresponds to this one  */
       tp->static_trace_marker_id_idx = i;
 
-      install_breakpoint (internal, &tp->base, 0);
+      install_breakpoint (internal, tp, 0);
     }
 }
 
@@ -15296,7 +15269,7 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
   if (utp->pass > 0)
     {
       xsnprintf (small_buf, sizeof (small_buf), "%d %d", utp->pass,
-		 tp->base.number);
+		 tp->number);
 
       trace_pass_command (small_buf, 0);
     }
@@ -15314,7 +15287,7 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
 
       cmd_list = read_command_lines_1 (read_uploaded_action, 1, NULL, NULL);
 
-      breakpoint_set_commands (&tp->base, std::move (cmd_list));
+      breakpoint_set_commands (tp, std::move (cmd_list));
     }
   else if (!VEC_empty (char_ptr, utp->actions)
 	   || !VEC_empty (char_ptr, utp->step_actions))
@@ -15323,7 +15296,7 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
 	     utp->number);
 
   /* Copy any status information that might be available.  */
-  tp->base.hit_count = utp->hit_count;
+  tp->hit_count = utp->hit_count;
   tp->traceframe_usage = utp->traceframe_usage;
 
   return tp;
@@ -15409,10 +15382,10 @@ static void
 trace_pass_set_count (struct tracepoint *tp, int count, int from_tty)
 {
   tp->pass_count = count;
-  observer_notify_breakpoint_modified (&tp->base);
+  observer_notify_breakpoint_modified (tp);
   if (from_tty)
     printf_filtered (_("Setting tracepoint %d's passcount to %d\n"),
-		     tp->base.number, count);
+		     tp->number, count);
 }
 
 /* Set passcount for tracepoint.
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 26b0aa53a8..bc8cfc416a 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -680,6 +680,11 @@ extern int target_exact_watchpoints;
 
 struct breakpoint
 {
+  /* We need a virtual destructor, because we delete all kinds of breakpoints
+     through a pointer to breakpoint.  */
+
+  virtual ~breakpoint () = default;
+
   /* Methods associated with this breakpoint.  */
   const breakpoint_ops *ops = NULL;
 
@@ -783,15 +788,10 @@ struct breakpoint
   gdbscm_breakpoint_object *scm_bp_object = NULL;
 };
 
-/* An instance of this type is used to represent a watchpoint.  It
-   includes a "struct breakpoint" as a kind of base class; users
-   downcast to "struct breakpoint *" when needed.  */
+/* An instance of this type is used to represent a watchpoint.  */
 
-struct watchpoint
+struct watchpoint : public breakpoint
 {
-  /* The base class.  */
-  struct breakpoint base;
-
   /* String form of exp to use for displaying to the user (malloc'd),
      or NULL if none.  */
   char *exp_string;
@@ -867,14 +867,10 @@ extern int is_breakpoint (const struct breakpoint *bpt);
 extern int is_watchpoint (const struct breakpoint *bpt);
 
 /* An instance of this type is used to represent all kinds of
-   tracepoints.  It includes a "struct breakpoint" as a kind of base
-   class; users downcast to "struct breakpoint *" when needed.  */
+   tracepoints.  */
 
-struct tracepoint
+struct tracepoint : public breakpoint
 {
-  /* The base class.  */
-  struct breakpoint base;
-
   /* Number of times this tracepoint should single-step and collect
      additional data.  */
   long step_count;
diff --git a/gdb/ctf.c b/gdb/ctf.c
index 8716cadf97..82e7013f97 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -1522,8 +1522,8 @@ ctf_get_traceframe_address (void)
       struct tracepoint *tp
 	= get_tracepoint_by_number_on_target (tpnum);
 
-      if (tp && tp->base.loc)
-	addr = tp->base.loc->address;
+      if (tp && tp->loc)
+	addr = tp->loc->address;
     }
 
   /* Restore the position.  */
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index cfe2d34f23..9412af8e14 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -398,7 +398,7 @@ mi_cmd_break_passcount (const char *command, char **argv, int argc)
   if (t)
     {
       t->pass_count = p;
-      observer_notify_breakpoint_modified (&t->base);
+      observer_notify_breakpoint_modified (t);
     }
   else
     {
diff --git a/gdb/remote.c b/gdb/remote.c
index 2cd98502f7..de3951631b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12650,9 +12650,9 @@ remote_get_tracepoint_status (struct target_ops *self, struct breakpoint *bp,
 
   if (tp)
     {
-      tp->base.hit_count = 0;
+      tp->hit_count = 0;
       tp->traceframe_usage = 0;
-      for (loc = tp->base.loc; loc; loc = loc->next)
+      for (loc = tp->loc; loc; loc = loc->next)
 	{
 	  /* If the tracepoint was never downloaded, don't go asking for
 	     any status.  */
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 255bbc9ef9..d479cefffa 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -650,8 +650,8 @@ tfile_get_traceframe_address (off_t tframe_offset)
 
   tp = get_tracepoint_by_number_on_target (tpnum);
   /* FIXME this is a poor heuristic if multiple locations.  */
-  if (tp && tp->base.loc)
-    addr = tp->base.loc->address;
+  if (tp && tp->loc)
+    addr = tp->loc->address;
 
   /* Restore our seek position.  */
   cur_offset = saved_offset;
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index cc90945bb2..e208fc6d2b 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -398,15 +398,15 @@ tracefile_fetch_registers (struct regcache *regcache, int regno)
 
   /* We can often usefully guess that the PC is going to be the same
      as the address of the tracepoint.  */
-  if (tp == NULL || tp->base.loc == NULL)
+  if (tp == NULL || tp->loc == NULL)
     return;
 
   /* But don't try to guess if tracepoint is multi-location...  */
-  if (tp->base.loc->next)
+  if (tp->loc->next)
     {
       warning (_("Tracepoint %d has multiple "
 		 "locations, cannot infer $pc"),
-	       tp->base.number);
+	       tp->number);
       return;
     }
   /* ... or does while-stepping.  */
@@ -414,13 +414,13 @@ tracefile_fetch_registers (struct regcache *regcache, int regno)
     {
       warning (_("Tracepoint %d does while-stepping, "
 		 "cannot infer $pc"),
-	       tp->base.number);
+	       tp->number);
       return;
     }
 
   /* Guess what we can from the tracepoint location.  */
   gdbarch_guess_tracepoint_registers (gdbarch, regcache,
-				      tp->base.loc->address);
+				      tp->loc->address);
 }
 
 /* This is the implementation of target_ops method to_has_all_memory.  */
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 808afdeb68..bd70fb6afc 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -648,11 +648,11 @@ actions_command (char *args, int from_tty)
     {
       std::string tmpbuf =
 	string_printf ("Enter actions for tracepoint %d, one per line.",
-		       t->base.number);
+		       t->number);
 
       command_line_up l = read_command_lines (&tmpbuf[0], from_tty, 1,
 					      check_tracepoint_command, t);
-      breakpoint_set_commands (&t->base, std::move (l));
+      breakpoint_set_commands (t, std::move (l));
     }
   /* else just return */
 }
@@ -738,7 +738,7 @@ validate_actionline (const char *line, struct breakpoint *b)
 	      /* else fall thru, treat p as an expression and parse it!  */
 	    }
 	  tmp_p = p;
-	  for (loc = t->base.loc; loc; loc = loc->next)
+	  for (loc = t->loc; loc; loc = loc->next)
 	    {
 	      p = tmp_p;
 	      expression_up exp = parse_exp_1 (&p, loc->address,
@@ -788,7 +788,7 @@ validate_actionline (const char *line, struct breakpoint *b)
 	  p = skip_spaces_const (p);
 
 	  tmp_p = p;
-	  for (loc = t->base.loc; loc; loc = loc->next)
+	  for (loc = t->loc; loc; loc = loc->next)
 	    {
 	      p = tmp_p;
 
@@ -2246,7 +2246,7 @@ tfind_1 (enum trace_find_type type, int num,
   reinit_frame_cache ();
   target_dcache_invalidate ();
 
-  set_tracepoint_num (tp ? tp->base.number : target_tracept);
+  set_tracepoint_num (tp ? tp->number : target_tracept);
 
   if (target_frameno != get_traceframe_number ())
     observer_notify_traceframe_changed (target_frameno, tracepoint_number);
@@ -2870,7 +2870,7 @@ get_traceframe_location (int *stepping_frame_p)
      locations, assume it is a direct hit rather than a while-stepping
      frame.  (FIXME this is not reliable, should record each frame's
      type.)  */
-  for (tloc = t->base.loc; tloc; tloc = tloc->next)
+  for (tloc = t->loc; tloc; tloc = tloc->next)
     if (tloc->address == regcache_read_pc (regcache))
       {
 	*stepping_frame_p = 0;
@@ -2880,7 +2880,7 @@ get_traceframe_location (int *stepping_frame_p)
   /* If this is a stepping frame, we don't know which location
      triggered.  The first is as good (or bad) a guess as any...  */
   *stepping_frame_p = 1;
-  return t->base.loc;
+  return t->loc;
 }
 
 /* Return all the actions, including default collect, of a tracepoint
@@ -3233,7 +3233,7 @@ find_matching_tracepoint_location (struct uploaded_tp *utp)
       if (b->type == utp->type
 	  && t->step_count == utp->step
 	  && t->pass_count == utp->pass
-	  && cond_string_is_same (t->base.cond_string, utp->cond_string)
+	  && cond_string_is_same (t->cond_string, utp->cond_string)
 	  /* FIXME also test actions.  */
 	  )
 	{
@@ -3302,7 +3302,7 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
 	  if (t)
 	    printf_filtered (_("Created tracepoint %d for "
 			       "target's tracepoint %d at %s.\n"),
-			     t->base.number, utp->number,
+			     t->number, utp->number,
 			     paddress (get_current_arch (), utp->addr));
 	  else
 	    printf_filtered (_("Failed to create tracepoint for target's "
@@ -3605,7 +3605,7 @@ parse_tracepoint_status (char *p, struct breakpoint *bp,
 
   p = unpack_varlen_hex (p, &uval);
   if (tp)
-    tp->base.hit_count += uval;
+    tp->hit_count += uval;
   else
     utp->hit_count += uval;
   p = unpack_varlen_hex (p + 1, &uval);
-- 
2.11.0


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