[binutils-gdb] Don't memset non-POD types: struct bp_location

[Pedro Alves <palves at sourceware dot org>]

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

commit 5625a2864147f4d92b22edfeeab7600818988be2
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Apr 25 01:27:42 2017 +0100

    Don't memset non-POD types: struct bp_location
    
    struct bp_location is not a POD, so we shouldn't be using memset to
    initialize it.
    
    Caught like this:
    
      src/gdb/breakpoint.c: In function â??bp_location** get_first_locp_gte_addr(CORE_ADDR)â??:
      src/gdb/breakpoint.c:950:53: error: use of deleted function â??void* memset(T*, int, size_t) [with T = bp_location; <template-parameter-1-2> = void; size_t = long unsigned int]â??
         memset (&dummy_loc, 0, sizeof (struct bp_location));
    						       ^
      In file included from src/gdb/defs.h:28:0,
    		   from src/gdb/breakpoint.c:20:
      src/gdb/common/common-defs.h:126:7: note: declared here
       void *memset (T *s, int c, size_t n) = delete;
    	 ^
    
    gdb/ChangeLog:
    2017-04-25  Pedro Alves  <palves@redhat.com>
    
    	* ada-lang.c (ada_catchpoint_location): Now a "class".  Remove
    	"base" field and inherit from "bp_location" instead.  Add
    	non-default ctor.
    	(allocate_location_exception): Use new non-default ctor.
    	* breakpoint.c (get_first_locp_gte_addr): Remove memset call.
    	(init_bp_location): Convert to ...
    	(bp_location::bp_location): ... this new ctor, and remove memset
    	call.
    	(base_breakpoint_allocate_location): Use the new non-default ctor.
    	* breakpoint.h (bp_location): Now a class.  Declare default and
    	non-default ctors.  In-class initialize all members.
    	(init_bp_location): Remove declaration.

Diff:
---
 gdb/ChangeLog    | 15 +++++++++++++
 gdb/ada-lang.c   | 19 ++++++-----------
 gdb/breakpoint.c | 13 +++---------
 gdb/breakpoint.h | 65 ++++++++++++++++++++++++++++----------------------------
 4 files changed, 58 insertions(+), 54 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f5419db..5df4a96 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,20 @@
 2017-04-25  Pedro Alves  <palves@redhat.com>
 
+	* ada-lang.c (ada_catchpoint_location): Now a "class".  Remove
+	"base" field and inherit from "bp_location" instead.  Add
+	non-default ctor.
+	(allocate_location_exception): Use new non-default ctor.
+	* breakpoint.c (get_first_locp_gte_addr): Remove memset call.
+	(init_bp_location): Convert to ...
+	(bp_location::bp_location): ... this new ctor, and remove memset
+	call.
+	(base_breakpoint_allocate_location): Use the new non-default ctor.
+	* breakpoint.h (bp_location): Now a class.  Declare default and
+	non-default ctors.  In-class initialize all members.
+	(init_bp_location): Remove declaration.
+
+2017-04-25  Pedro Alves  <palves@redhat.com>
+
 	* common/enum-flags.h (enum_flags): Don't implement copy ctor and
 	assignment operator.
 
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 2e5643b..58c8a2e 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12224,14 +12224,14 @@ static char *ada_exception_catchpoint_cond_string (const char *excep_string);
    when symbols change.  */
 
 /* An instance of this type is used to represent an Ada catchpoint
-   breakpoint location.  It includes a "struct bp_location" as a kind
-   of base class; users downcast to "struct bp_location *" when
-   needed.  */
+   breakpoint location.  */
 
-struct ada_catchpoint_location
+class ada_catchpoint_location : public bp_location
 {
-  /* The base class.  */
-  struct bp_location base;
+public:
+  ada_catchpoint_location (const bp_location_ops *ops, breakpoint *owner)
+    : bp_location (ops, owner)
+  {}
 
   /* The condition that checks whether the exception that was raised
      is the specific exception the user specified on catchpoint
@@ -12347,12 +12347,7 @@ static struct bp_location *
 allocate_location_exception (enum ada_exception_catchpoint_kind ex,
 			     struct breakpoint *self)
 {
-  struct ada_catchpoint_location *loc;
-
-  loc = new ada_catchpoint_location ();
-  init_bp_location (&loc->base, &ada_catchpoint_location_ops, self);
-  loc->excep_cond_expr = NULL;
-  return &loc->base;
+  return new ada_catchpoint_location (&ada_catchpoint_location_ops, self);
 }
 
 /* Implement the RE_SET method in the breakpoint_ops structure for all
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b0f7cb5..4db32ab 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -947,7 +947,6 @@ get_first_locp_gte_addr (CORE_ADDR address)
   struct bp_location **locp_found = NULL;
 
   /* Initialize the dummy location's address field.  */
-  memset (&dummy_loc, 0, sizeof (struct bp_location));
   dummy_loc.address = address;
 
   /* Find a close match to the first location at ADDRESS.  */
@@ -7300,11 +7299,9 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
     }
 }
 
-void
-init_bp_location (struct bp_location *loc, const struct bp_location_ops *ops,
-		  struct breakpoint *owner)
+bp_location::bp_location (const bp_location_ops *ops, breakpoint *owner)
 {
-  memset (loc, 0, sizeof (*loc));
+  bp_location *loc = this;
 
   gdb_assert (ops != NULL);
 
@@ -12824,11 +12821,7 @@ base_breakpoint_dtor (struct breakpoint *self)
 static struct bp_location *
 base_breakpoint_allocate_location (struct breakpoint *self)
 {
-  struct bp_location *loc;
-
-  loc = new struct bp_location ();
-  init_bp_location (loc, &bp_location_ops, self);
-  return loc;
+  return new bp_location (&bp_location_ops, self);
 }
 
 static void
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 9b5dc3f..18b284f 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -310,20 +310,25 @@ struct bp_location_ops
   void (*dtor) (struct bp_location *self);
 };
 
-struct bp_location
+class bp_location
 {
+public:
+  bp_location () = default;
+
+  bp_location (const bp_location_ops *ops, breakpoint *owner);
+
   /* Chain pointer to the next breakpoint location for
      the same parent breakpoint.  */
-  struct bp_location *next;
+  bp_location *next = NULL;
 
   /* Methods associated with this location.  */
-  const struct bp_location_ops *ops;
+  const bp_location_ops *ops = NULL;
 
   /* The reference count.  */
-  int refc;
+  int refc = 0;
 
   /* Type of this breakpoint location.  */
-  enum bp_loc_type loc_type;
+  bp_loc_type loc_type {};
 
   /* Each breakpoint location must belong to exactly one higher-level
      breakpoint.  This pointer is NULL iff this bp_location is no
@@ -331,7 +336,7 @@ struct bp_location
      is deleted, its locations may still be found in the
      moribund_locations list, or if we had stopped for it, in
      bpstats.  */
-  struct breakpoint *owner;
+  breakpoint *owner = NULL;
 
   /* Conditional.  Break only if this expression's value is nonzero.
      Unlike string form of condition, which is associated with
@@ -360,32 +365,32 @@ struct bp_location
      duplicates of this location and thus we don't need to call
      force_breakpoint_reinsertion (...) for this location.  */
 
-  enum condition_status condition_changed;
+  condition_status condition_changed {};
 
   agent_expr_up cmd_bytecode;
 
   /* Signals that breakpoint conditions and/or commands need to be
      re-synched with the target.  This has no use other than
      target-side breakpoints.  */
-  char needs_update;
+  bool needs_update = false;
 
   /* This location's address is in an unloaded solib, and so this
      location should not be inserted.  It will be automatically
      enabled when that solib is loaded.  */
-  char shlib_disabled; 
+  bool shlib_disabled = false;
 
   /* Is this particular location enabled.  */
-  char enabled;
+  bool enabled = false;
   
   /* Nonzero if this breakpoint is now inserted.  */
-  char inserted;
+  bool inserted = false;
 
   /* Nonzero if this is a permanent breakpoint.  There is a breakpoint
      instruction hard-wired into the target's code.  Don't try to
      write another breakpoint instruction on top of it, or restore its
      value.  Step over it using the architecture's
      gdbarch_skip_permanent_breakpoint method.  */
-  char permanent;
+  bool permanent = false;
 
   /* Nonzero if this is not the first breakpoint in the list
      for the given address.  location of tracepoint can _never_
@@ -393,7 +398,7 @@ struct bp_location
      kinds of breakpoints, because two locations at the same
      address may have different actions, so both of these locations
      should be downloaded and so that `tfind N' always works.  */
-  char duplicate;
+  bool duplicate = false;
 
   /* If we someday support real thread-specific breakpoints, then
      the breakpoint location will need a thread identifier.  */
@@ -403,7 +408,7 @@ struct bp_location
 
   /* Architecture associated with this location's address.  May be
      different from the breakpoint architecture.  */
-  struct gdbarch *gdbarch;
+  struct gdbarch *gdbarch = NULL;
 
   /* The program space associated with this breakpoint location
      address.  Note that an address space may be represented in more
@@ -411,26 +416,26 @@ struct bp_location
      its own program space, but there will only be one address space
      for all of them), but we must not insert more than one location
      at the same address in the same address space.  */
-  struct program_space *pspace;
+  program_space *pspace = NULL;
 
   /* 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;
+  CORE_ADDR address = 0;
 
   /* For hardware watchpoints, the size of the memory region being
      watched.  For hardware ranged breakpoints, the size of the
      breakpoint range.  */
-  int length;
+  int length = 0;
 
   /* Type of hardware watchpoint.  */
-  enum target_hw_bp_type watchpoint_type;
+  target_hw_bp_type watchpoint_type {};
 
   /* For any breakpoint type with an address, this is the section
      associated with the address.  Used primarily for overlay
      debugging.  */
-  struct obj_section *section;
+  obj_section *section = NULL;
 
   /* Address at which breakpoint was requested, either by the user or
      by GDB for internal breakpoints.  This will usually be the same
@@ -438,24 +443,24 @@ struct bp_location
      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;
+  CORE_ADDR requested_address = 0;
 
   /* An additional address assigned with this location.  This is currently
      only used by STT_GNU_IFUNC resolver breakpoints to hold the address
      of the resolver function.  */
-  CORE_ADDR related_address;
+  CORE_ADDR related_address = 0;
 
   /* If the location comes from a probe point, this is the probe associated
      with it.  */
-  struct bound_probe probe;
+  bound_probe probe {};
 
-  char *function_name;
+  char *function_name = NULL;
 
   /* Details of the placed breakpoint, when inserted.  */
-  struct bp_target_info target_info;
+  bp_target_info target_info {};
 
   /* Similarly, for the breakpoint at an overlay's LMA, if necessary.  */
-  struct bp_target_info overlay_target_info;
+  bp_target_info overlay_target_info {};
 
   /* In a non-stop mode, it's possible that we delete a breakpoint,
      but as we do that, some still running thread hits that breakpoint.
@@ -466,19 +471,19 @@ struct bp_location
      breakpoint was deleted, we retire all locations of that breakpoint.
      This variable keeps a number of events still to go, when
      it becomes 0 this location is retired.  */
-  int events_till_retirement;
+  int events_till_retirement = 0;
 
   /* Line number which was used to place this location.
 
      Breakpoint placed into a comment keeps it's user specified line number
      despite ADDRESS resolves into a different line number.  */
 
-  int line_number;
+  int line_number = 0;
 
   /* Symtab which was used to place this location.  This is used
      to find the corresponding source file name.  */
 
-  struct symtab *symtab;
+  struct symtab *symtab = NULL;
 };
 
 /* The possible return values for print_bpstat, print_it_normal,
@@ -1205,10 +1210,6 @@ extern void until_break_command (char *, int, int);
 
 /* Initialize a struct bp_location.  */
 
-extern void init_bp_location (struct bp_location *loc,
-			      const struct bp_location_ops *ops,
-			      struct breakpoint *owner);
-
 extern void update_breakpoint_locations (struct breakpoint *b,
 					 struct program_space *filter_pspace,
 					 struct symtabs_and_lines sals,