[binutils-gdb] C++-ify bp_location

[Tom Tromey <tromey at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5f486660101ab09d50fa3bb9a10555f12722f39f

commit 5f486660101ab09d50fa3bb9a10555f12722f39f
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Feb 12 14:28:07 2019 -0700

    C++-ify bp_location
    
    Philippe noticed a memory leak coming from ada_catchpoint_location --
    it was not freeing the "function_name" member from its base class:
    
    ==14141== 114 bytes in 4 blocks are definitely lost in loss record 1,055 of 3,424
    ==14141==    at 0x4C2BE6D: malloc (vg_replace_malloc.c:309)
    ==14141==    by 0x405107: xmalloc (common-utils.c:44)
    ==14141==    by 0x7563F9: xstrdup (xstrdup.c:34)
    ==14141==    by 0x3B82B3: set_breakpoint_location_function(bp_location*, int) (breakpoint.c:7156)
    ==14141==    by 0x3C112B: add_location_to_breakpoint(breakpoint*, symtab_and_line const*) (breakpoint.c:8609)
    ==14141==    by 0x3C127A: init_raw_breakpoint(breakpoint*, gdbarch*, symtab_and_line, bptype, breakpoint_ops const*) (breakpoint.c:7187)
    ==14141==    by 0x3C1B52: init_ada_exception_breakpoint(breakpoint*, gdbarch*, symtab_and_line, char const*, breakpoint_ops const*, int, int, int) (breakpoint.c:11262)
    ==14141==    by 0x381C2E: create_ada_exception_catchpoint(gdbarch*, ada_exception_catchpoint_kind, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, int, int) (ada-lang.c:13255)
    
    This patch fixes the problem by further C++-ifying bp_location.  In
    particular, bp_location_ops is now removed, and the "dtor" function
    pointer is replaced with an ordinary destructor.
    
    gdb/ChangeLog
    2019-02-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
    	    Tom Tromey  <tromey@adacore.com>
    
    	* breakpoint.c (~bp_location): Rename from bp_location_dtor.
    	(bp_location_ops): Remove.
    	(base_breakpoint_allocate_location): Update.
    	(free_bp_location): Update.
    	* ada-lang.c (class ada_catchpoint_location)
    	<ada_catchpoint_location>: Remove ops parameter.
    	(ada_catchpoint_location_dtor): Remove.
    	(ada_catchpoint_location_ops): Remove.
    	(allocate_location_exception): Update.
    	* breakpoint.h (struct bp_location_ops): Remove.
    	(class bp_location) <bp_location>: Remove bp_location_ops
    	parameter.
    	<~bp_location>: Add destructor.
    	<ops>: Remove.

Diff:
---
 gdb/ChangeLog    | 18 ++++++++++++++++++
 gdb/ada-lang.c   | 24 +++---------------------
 gdb/breakpoint.c | 20 ++++----------------
 gdb/breakpoint.h | 18 +++---------------
 4 files changed, 28 insertions(+), 52 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index bb27f74..11667ae 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,21 @@
+2019-02-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+	    Tom Tromey  <tromey@adacore.com>
+
+	* breakpoint.c (~bp_location): Rename from bp_location_dtor.
+	(bp_location_ops): Remove.
+	(base_breakpoint_allocate_location): Update.
+	(free_bp_location): Update.
+	* ada-lang.c (class ada_catchpoint_location)
+	<ada_catchpoint_location>: Remove ops parameter.
+	(ada_catchpoint_location_dtor): Remove.
+	(ada_catchpoint_location_ops): Remove.
+	(allocate_location_exception): Update.
+	* breakpoint.h (struct bp_location_ops): Remove.
+	(class bp_location) <bp_location>: Remove bp_location_ops
+	parameter.
+	<~bp_location>: Add destructor.
+	<ops>: Remove.
+
 2019-02-14  Thomas Schwinge  <thomas@codesourcery.com>
 	    Pedro Alves  <palves@redhat.com>
 
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index a878d4d..602facb 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12396,8 +12396,8 @@ static std::string ada_exception_catchpoint_cond_string
 class ada_catchpoint_location : public bp_location
 {
 public:
-  ada_catchpoint_location (const bp_location_ops *ops, breakpoint *owner)
-    : bp_location (ops, owner)
+  ada_catchpoint_location (breakpoint *owner)
+    : bp_location (owner)
   {}
 
   /* The condition that checks whether the exception that was raised
@@ -12406,24 +12406,6 @@ public:
   expression_up excep_cond_expr;
 };
 
-/* Implement the DTOR method in the bp_location_ops structure for all
-   Ada exception catchpoint kinds.  */
-
-static void
-ada_catchpoint_location_dtor (struct bp_location *bl)
-{
-  struct ada_catchpoint_location *al = (struct ada_catchpoint_location *) bl;
-
-  al->excep_cond_expr.reset ();
-}
-
-/* The vtable to be used in Ada catchpoint locations.  */
-
-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.  */
 
 struct ada_catchpoint : public breakpoint
@@ -12493,7 +12475,7 @@ static struct bp_location *
 allocate_location_exception (enum ada_exception_catchpoint_kind ex,
 			     struct breakpoint *self)
 {
-  return new ada_catchpoint_location (&ada_catchpoint_location_ops, self);
+  return new ada_catchpoint_location (self);
 }
 
 /* Implement the RE_SET method in the breakpoint_ops structure for all
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 67d83e6..9be99ff 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6958,13 +6958,10 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
     }
 }
 
-bp_location::bp_location (const bp_location_ops *ops, breakpoint *owner)
+bp_location::bp_location (breakpoint *owner)
 {
   bp_location *loc = this;
 
-  gdb_assert (ops != NULL);
-
-  loc->ops = ops;
   loc->owner = owner;
   loc->cond_bytecode = NULL;
   loc->shlib_disabled = 0;
@@ -7033,7 +7030,6 @@ allocate_bp_location (struct breakpoint *bpt)
 static void
 free_bp_location (struct bp_location *loc)
 {
-  loc->ops->dtor (loc);
   delete loc;
 }
 
@@ -12166,19 +12162,11 @@ say_where (struct breakpoint *b)
     }
 }
 
-/* Default bp_location_ops methods.  */
-
-static void
-bp_location_dtor (struct bp_location *self)
+bp_location::~bp_location ()
 {
-  xfree (self->function_name);
+  xfree (function_name);
 }
 
-static const struct bp_location_ops bp_location_ops =
-{
-  bp_location_dtor
-};
-
 /* Destructor for the breakpoint base class.  */
 
 breakpoint::~breakpoint ()
@@ -12191,7 +12179,7 @@ breakpoint::~breakpoint ()
 static struct bp_location *
 base_breakpoint_allocate_location (struct breakpoint *self)
 {
-  return new bp_location (&bp_location_ops, self);
+  return new bp_location (self);
 }
 
 static void
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 8c8c66a..a91e3e3 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -301,31 +301,19 @@ enum bp_loc_type
   bp_loc_other			/* Miscellaneous...  */
 };
 
-/* This structure is a collection of function pointers that, if
-   available, will be called instead of performing the default action
-   for this bp_loc_type.  */
-
-struct bp_location_ops
-{
-  /* Destructor.  Releases everything from SELF (but not SELF
-     itself).  */
-  void (*dtor) (struct bp_location *self);
-};
-
 class bp_location
 {
 public:
   bp_location () = default;
 
-  bp_location (const bp_location_ops *ops, breakpoint *owner);
+  bp_location (breakpoint *owner);
+
+  virtual ~bp_location ();
 
   /* Chain pointer to the next breakpoint location for
      the same parent breakpoint.  */
   bp_location *next = NULL;
 
-  /* Methods associated with this location.  */
-  const bp_location_ops *ops = NULL;
-
   /* The reference count.  */
   int refc = 0;