[PATCH v2 2/3] Introduce obstack_new, poison other "typed" obstack functions

[PATCH v2 2/3] Introduce obstack_new, poison other "typed" obstack functions

[Simon Marchi]

From: Simon Marchi <simon.marchi@ericsson.com>

Since we use obstacks with objects that are not default constructible,
we sometimes need to manually call the constructor by hand using
placement new:

  foo *f = obstack_alloc (obstack, sizeof (foo));
  f = new (f) foo;

It's possible to use allocate_on_obstack instead, but there are types
that we sometimes want to allcocate on an obstack, and sometimes on the
regular heap.  This patch introduces a utility to make this pattern
simpler if allocate_on_obstack is not an option:

  foo *f = obstack_new<foo> ();

Right now there's only one usage (in tdesc_data_init).

To help catch places where we would forget to call new when allocating
such an object on an obstack, this patch also poisons some other methods
of allocating an instance of a type on an obstack:

  - OBSTACK_ZALLOC/OBSTACK_CALLOC
  - XOBNEW/XOBNEW
  - GDBARCH_OBSTACK_ZALLOC/GDBARCH_OBSTACK_CALLOC

Unfortunately, there's no way to catch wrong usages of obstack_alloc.

By pulling on that string though, it tripped on allocating struct
template_symbol using OBSTACK_ZALLOC.  The criterion currently used to
know whether it's safe to "malloc" an instance of a struct is whether it
is a POD.  Because it inherits from struct symbol, template_symbol is
not a POD.  This criterion is a bit too strict however, it should still
safe to allocate memory for a template_symbol and memset it to 0.  We
didn't use is_trivially_constructible as the criterion in the first
place only because it is not available in gcc < 5.  So here I considered
two alternatives:

1. Relax that criterion to use std::is_trivially_constructible and add a
   bit more glue code to make it work with gcc < 5
2. Continue pulling on the string and change how the symbol structures
   are allocated and initialized

I managed to do both, but I decided to go with #1 to keep this patch
simpler and more focused.  When building with a compiler that does not
have is_trivially_constructible, the check will just not be enforced.

gdb/ChangeLog:

	* common/traits.h (HAVE_IS_TRIVIALLY_COPYABLE): Define if
	compiler supports std::is_trivially_constructible.
	* common/poison.h: Include obstack.h.
	(IsMallocable): Define to is_trivially_constructible if the
	compiler supports it, define to true_type otherwise.
	(xobnew): New.
	(XOBNEW): Redefine.
	(xobnewvec): New.
	(XOBNEWVEC): Redefine.
	* gdb_obstack.h (obstack_zalloc): New.
	(OBSTACK_ZALLOC): Redefine.
	(obstack_calloc): New.
	(OBSTACK_CALLOC): Redefine.
	(obstack_new): New.
	* gdbarch.sh: Include gdb_obstack in gdbarch.h.
	(gdbarch_obstack): New declaration in gdbarch.h, definition in
	gdbarch.c.
	(GDBARCH_OBSTACK_CALLOC, GDBARCH_OBSTACK_ZALLOC): Use
	obstack_calloc/obstack_zalloc.
	(gdbarch_obstack_zalloc): Remove.
	* target-descriptions.c (tdesc_data_init): Use obstack_new.
---
 gdb/common/poison.h       | 31 ++++++++++++++++++++++++++++++-
 gdb/common/traits.h       |  8 ++++++++
 gdb/gdb_obstack.h         | 36 ++++++++++++++++++++++++++++++++----
 gdb/gdbarch.c             |  9 ++-------
 gdb/gdbarch.h             | 10 +++++++---
 gdb/gdbarch.sh            | 21 +++++++++++----------
 gdb/target-descriptions.c |  7 +------
 7 files changed, 91 insertions(+), 31 deletions(-)

diff --git a/gdb/common/poison.h b/gdb/common/poison.h
index c98d2b362a0f..ddab2c19b9cb 100644
--- a/gdb/common/poison.h
+++ b/gdb/common/poison.h
@@ -21,6 +21,7 @@
 #define COMMON_POISON_H
 
 #include "traits.h"
+#include "obstack.h"
 
 /* Poison memset of non-POD types.  The idea is catching invalid
    initialization of non-POD structs that is easy to be introduced as
@@ -88,7 +89,11 @@ void *memmove (D *dest, const S *src, size_t n) = delete;
    objects that require new/delete.  */
 
 template<typename T>
-using IsMallocable = std::is_pod<T>;
+#if HAVE_IS_TRIVIALLY_CONSTRUCTIBLE
+using IsMallocable = std::is_trivially_constructible<T>;
+#else
+using IsMallocable = std::true_type;
+#endif
 
 template<typename T>
 using IsFreeable = gdb::Or<std::is_trivially_destructible<T>, std::is_void<T>>;
@@ -216,4 +221,28 @@ non-POD data type.");
 #undef XRESIZEVAR
 #define XRESIZEVAR(T, P, S) xresizevar<T> (P, S)
 
+template<typename T>
+static T *
+xobnew (obstack *ob)
+{
+  static_assert (IsMallocable<T>::value, "Trying to use XOBNEW with a \
+non-POD data type.");
+  return XOBNEW (ob, T);
+}
+
+#undef XOBNEW
+#define XOBNEW(O, T) xobnew<T> (O)
+
+template<typename T>
+static T *
+xobnewvec (obstack *ob, size_t n)
+{
+  static_assert (IsMallocable<T>::value, "Trying to use XOBNEWVEC with a \
+non-POD data type.");
+  return XOBNEWVEC (ob, T, n);
+}
+
+#undef XOBNEWVEC
+#define XOBNEWVEC(O, T, N) xobnewvec<T> (O, N)
+
 #endif /* COMMON_POISON_H */
diff --git a/gdb/common/traits.h b/gdb/common/traits.h
index d9e683901129..070ef159e5b9 100644
--- a/gdb/common/traits.h
+++ b/gdb/common/traits.h
@@ -33,6 +33,14 @@
 # define HAVE_IS_TRIVIALLY_COPYABLE 1
 #endif
 
+/* HAVE_IS_TRIVIALLY_CONSTRUCTIBLE is defined as 1 iff
+   std::is_trivially_constructible is available.  GCC only implemented it
+   in GCC 5.  */
+#if (__has_feature(is_trivially_constructible) \
+     || (defined __GNUC__ && __GNUC__ >= 5))
+# define HAVE_IS_TRIVIALLY_COPYABLE 1
+#endif
+
 namespace gdb {
 
 /* Pre C++14-safe (CWG 1558) version of C++17's std::void_t.  See
diff --git a/gdb/gdb_obstack.h b/gdb/gdb_obstack.h
index 1011008ffbd3..29cad937e4ee 100644
--- a/gdb/gdb_obstack.h
+++ b/gdb/gdb_obstack.h
@@ -24,12 +24,40 @@
 
 /* Utility macros - wrap obstack alloc into something more robust.  */
 
-#define OBSTACK_ZALLOC(OBSTACK,TYPE) \
-  ((TYPE *) memset (obstack_alloc ((OBSTACK), sizeof (TYPE)), 0, sizeof (TYPE)))
+template <typename T>
+static inline T*
+obstack_zalloc (struct obstack *ob)
+{
+  static_assert (IsMallocable<T>::value, "Trying to use OBSTACK_ZALLOC with a \
+non-POD data type.  Use obstack_new instead.");
+  return ((T *) memset (obstack_alloc (ob, sizeof (T)), 0, sizeof (T)));
+}
+
+#define OBSTACK_ZALLOC(OBSTACK,TYPE) obstack_zalloc<TYPE> ((OBSTACK))
+
+template <typename T>
+static inline T *
+obstack_calloc (struct obstack *ob, size_t number)
+{
+  static_assert (IsMallocable<T>::value, "Trying to use OBSTACK_CALLOC with a \
+non-POD data type.  Use obstack_new instead.");
+  return ((T *) memset (obstack_alloc (ob, number * sizeof (T)), 0,
+			number * sizeof (T)));
+}
 
 #define OBSTACK_CALLOC(OBSTACK,NUMBER,TYPE) \
-  ((TYPE *) memset (obstack_alloc ((OBSTACK), (NUMBER) * sizeof (TYPE)), \
-		    0, (NUMBER) * sizeof (TYPE)))
+  obstack_calloc<TYPE> ((OBSTACK), (NUMBER))
+
+/* Allocate an object on OB and call its constructor.  */
+
+template <typename T, typename... Args>
+static inline T*
+obstack_new (struct obstack *ob, Args&&... args)
+{
+  T* object = (T *) obstack_alloc (ob, sizeof (T));
+  object = new (object) T (std::forward<Args> (args)...);
+  return object;
+}
 
 /* Unless explicitly specified, GDB obstacks always use xmalloc() and
    xfree().  */
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 1359c2fb53f3..4b4ae0bc141e 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -471,15 +471,10 @@ gdbarch_alloc (const struct gdbarch_info *info,
 }
 
 
-/* Allocate extra space using the per-architecture obstack.  */
 
-void *
-gdbarch_obstack_zalloc (struct gdbarch *arch, long size)
+obstack *gdbarch_obstack (gdbarch *arch)
 {
-  void *data = obstack_alloc (arch->obstack, size);
-
-  memset (data, 0, size);
-  return data;
+  return arch->obstack;
 }
 
 /* See gdbarch.h.  */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 0084f199d7cc..d42e69ccc0fb 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -38,6 +38,7 @@
 #include <vector>
 #include "frame.h"
 #include "dis-asm.h"
+#include "gdb_obstack.h"
 
 struct floatformat;
 struct ui_file;
@@ -1705,14 +1706,17 @@ extern struct gdbarch *gdbarch_alloc (const struct gdbarch_info *info, struct gd
 
 extern void gdbarch_free (struct gdbarch *);
 
+/* Get the obstack owned by ARCH.  */
+
+extern obstack *gdbarch_obstack (gdbarch *arch);
 
 /* Helper function.  Allocate memory from the ``struct gdbarch''
    obstack.  The memory is freed when the corresponding architecture
    is also freed.  */
 
-extern void *gdbarch_obstack_zalloc (struct gdbarch *gdbarch, long size);
-#define GDBARCH_OBSTACK_CALLOC(GDBARCH, NR, TYPE) ((TYPE *) gdbarch_obstack_zalloc ((GDBARCH), (NR) * sizeof (TYPE)))
-#define GDBARCH_OBSTACK_ZALLOC(GDBARCH, TYPE) ((TYPE *) gdbarch_obstack_zalloc ((GDBARCH), sizeof (TYPE)))
+#define GDBARCH_OBSTACK_CALLOC(GDBARCH, NR, TYPE)   obstack_calloc<TYPE> (gdbarch_obstack ((GDBARCH)), (NR))
+
+#define GDBARCH_OBSTACK_ZALLOC(GDBARCH, TYPE)   obstack_zalloc<TYPE> (gdbarch_obstack((GDBARCH)))
 
 /* Duplicate STRING, returning an equivalent string that's allocated on the
    obstack associated with GDBARCH.  The string is freed when the corresponding
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 4fc54cba9c30..ed407cb28682 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1261,6 +1261,7 @@ cat <<EOF
 #include <vector>
 #include "frame.h"
 #include "dis-asm.h"
+#include "gdb_obstack.h"
 
 struct floatformat;
 struct ui_file;
@@ -1532,14 +1533,19 @@ extern struct gdbarch *gdbarch_alloc (const struct gdbarch_info *info, struct gd
 
 extern void gdbarch_free (struct gdbarch *);
 
+/* Get the obstack owned by ARCH.  */
+
+extern obstack *gdbarch_obstack (gdbarch *arch);
 
 /* Helper function.  Allocate memory from the \`\`struct gdbarch''
    obstack.  The memory is freed when the corresponding architecture
    is also freed.  */
 
-extern void *gdbarch_obstack_zalloc (struct gdbarch *gdbarch, long size);
-#define GDBARCH_OBSTACK_CALLOC(GDBARCH, NR, TYPE) ((TYPE *) gdbarch_obstack_zalloc ((GDBARCH), (NR) * sizeof (TYPE)))
-#define GDBARCH_OBSTACK_ZALLOC(GDBARCH, TYPE) ((TYPE *) gdbarch_obstack_zalloc ((GDBARCH), sizeof (TYPE)))
+#define GDBARCH_OBSTACK_CALLOC(GDBARCH, NR, TYPE) \
+  obstack_calloc<TYPE> (gdbarch_obstack ((GDBARCH)), (NR))
+
+#define GDBARCH_OBSTACK_ZALLOC(GDBARCH, TYPE) \
+  obstack_zalloc<TYPE> (gdbarch_obstack((GDBARCH)))
 
 /* Duplicate STRING, returning an equivalent string that's allocated on the
    obstack associated with GDBARCH.  The string is freed when the corresponding
@@ -1849,15 +1855,10 @@ EOF
 printf "\n"
 printf "\n"
 cat <<EOF
-/* Allocate extra space using the per-architecture obstack.  */
 
-void *
-gdbarch_obstack_zalloc (struct gdbarch *arch, long size)
+obstack *gdbarch_obstack (gdbarch *arch)
 {
-  void *data = obstack_alloc (arch->obstack, size);
-
-  memset (data, 0, size);
-  return data;
+  return arch->obstack;
 }
 
 /* See gdbarch.h.  */
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 36ea4b12f5f1..61fb344af844 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -723,12 +723,7 @@ tdesc_find_type (struct gdbarch *gdbarch, const char *id)
 static void *
 tdesc_data_init (struct obstack *obstack)
 {
-  struct tdesc_arch_data *data;
-
-  data = OBSTACK_ZALLOC (obstack, struct tdesc_arch_data);
-  new (data) tdesc_arch_data ();
-
-  return data;
+  return obstack_new<tdesc_arch_data> (obstack);
 }
 
 /* Similar, but for the temporary copy used during architecture
-- 
2.17.0

[PATCH v2 0/3] Poison obstack functions

[Simon Marchi]

v2 of https://sourceware.org/ml/gdb-patches/2018-04/msg00524.html

What's new:

- Don't allocate mapped_index on an obstack (patch 1/3)
- Use OBSTACK_ZALLOC instead of XOBNEW + memset when possible (patch
  3/3)

Simon Marchi (3):
  Don't allocate mapped_index on the objfile obstack
  Introduce obstack_new, poison other "typed" obstack functions
  Use XOBNEW/XOBNEWVEC/OBSTACK_ZALLOC when possible

 gdb/ada-lang.c            |  3 +--
 gdb/common/poison.h       | 31 +++++++++++++++++++++++-
 gdb/common/traits.h       |  8 ++++++
 gdb/dwarf2-frame.c        |  3 +--
 gdb/dwarf2read.c          | 14 +++--------
 gdb/dwarf2read.h          |  2 +-
 gdb/gdb_obstack.h         | 36 ++++++++++++++++++++++++---
 gdb/gdbarch.c             |  9 ++-----
 gdb/gdbarch.h             | 10 +++++---
 gdb/gdbarch.sh            | 21 ++++++++--------
 gdb/hppa-tdep.c           |  7 ++----
 gdb/mdebugread.c          | 51 +++++++++++++--------------------------
 gdb/minsyms.c             |  6 ++---
 gdb/objfiles.c            |  9 ++-----
 gdb/psymtab.c             |  4 +--
 gdb/stabsread.c           |  7 ++----
 gdb/target-descriptions.c |  7 +-----
 gdb/xcoffread.c           |  3 +--
 18 files changed, 125 insertions(+), 106 deletions(-)

-- 
2.17.0

[PATCH v2 3/3] Use XOBNEW/XOBNEWVEC/OBSTACK_ZALLOC when possible

[Simon Marchi]

From: Simon Marchi <simon.marchi@ericsson.com>

Since XOBNEW/XOBNEWVEC/OBSTACK_ZALLOC are now poisoned to prevent using
them with non-trivially-constructible objects, it is worth using them
over plain obstack_alloc.  This patch changes the locations I could find
where we can do that change easily.

gdb/ChangeLog:

	* ada-lang.c (cache_symbol): Use XOBNEW and/or XOBNEWVEC and/or
	OBSTACK_ZALLOC.
	* dwarf2-frame.c (dwarf2_build_frame_info): Likewise.
	* hppa-tdep.c (hppa_init_objfile_priv_data): Likewise.
	* mdebugread.c (mdebug_build_psymtabs): Likewise.
	(add_pending): Likewise.
	(parse_symbol): Likewise.
	(parse_partial_symbols): Likewise.
	(psymtab_to_symtab_1): Likewise.
	(new_psymtab): Likewise.
	(elfmdebug_build_psymtabs): Likewise.
	* minsyms.c (terminate_minimal_symbol_table): Likewise.
	* objfiles.c (get_objfile_bfd_data): Likewise.
	(objfile_register_static_link): Likewise.
	* psymtab.c (allocate_psymtab): Likewise.
	* stabsread.c (read_member_functions): Likewise.
	* xcoffread.c (xcoff_end_psymtab): Likewise.
---
 gdb/ada-lang.c     |  3 +--
 gdb/dwarf2-frame.c |  3 +--
 gdb/hppa-tdep.c    |  7 ++-----
 gdb/mdebugread.c   | 51 ++++++++++++++++------------------------------
 gdb/minsyms.c      |  6 ++----
 gdb/objfiles.c     |  9 ++------
 gdb/psymtab.c      |  4 +---
 gdb/stabsread.c    |  7 ++-----
 gdb/xcoffread.c    |  3 +--
 9 files changed, 29 insertions(+), 64 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index de20c43beddc..8145480d590c 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4744,8 +4744,7 @@ cache_symbol (const char *name, domain_enum domain, struct symbol *sym,
     return;
 
   h = msymbol_hash (name) % HASH_SIZE;
-  e = (struct cache_entry *) obstack_alloc (&sym_cache->cache_space,
-					    sizeof (*e));
+  e = XOBNEW (&sym_cache->cache_space, cache_entry);
   e->next = sym_cache->root[h];
   sym_cache->root[h] = e;
   e->name = copy
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 38367f7d73e4..aed6c7b1e6ca 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -2205,8 +2205,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
   fde_table.entries = NULL;
 
   /* Build a minimal decoding of the DWARF2 compilation unit.  */
-  unit = (struct comp_unit *) obstack_alloc (&objfile->objfile_obstack,
-					     sizeof (struct comp_unit));
+  unit = XOBNEW (&objfile->objfile_obstack, comp_unit);
   unit->abfd = objfile->obfd;
   unit->objfile = objfile;
   unit->dbase = 0;
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 84dbd6674859..9692b33e097b 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -205,13 +205,10 @@ hppa_symbol_address(const char *sym)
 static struct hppa_objfile_private *
 hppa_init_objfile_priv_data (struct objfile *objfile)
 {
-  struct hppa_objfile_private *priv;
+  hppa_objfile_private *priv
+    = OBSTACK_ZALLOC (&objfile->objfile_obstack, hppa_objfile_private);
 
-  priv = (struct hppa_objfile_private *)
-  	 obstack_alloc (&objfile->objfile_obstack,
-	 		sizeof (struct hppa_objfile_private));
   set_objfile_data (objfile, hppa_objfile_priv_data, priv);
-  memset (priv, 0, sizeof (*priv));
 
   return priv;
 }
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index c0bce551489d..fee8d430aa08 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -356,9 +356,8 @@ mdebug_build_psymtabs (minimal_symbol_reader &reader,
       char *fdr_end;
       FDR *fdr_ptr;
 
-      info->fdr = (FDR *) obstack_alloc (&objfile->objfile_obstack,
-					 (info->symbolic_header.ifdMax
-					  * sizeof (FDR)));
+      info->fdr = (FDR *) XOBNEWVEC (&objfile->objfile_obstack, FDR,
+				     info->symbolic_header.ifdMax);
       fdr_src = (char *) info->external_fdr;
       fdr_end = (fdr_src
 		 + info->symbolic_header.ifdMax * swap->external_fdr_size);
@@ -508,9 +507,7 @@ add_pending (FDR *fh, char *sh, struct type *t)
   /* Make sure we do not make duplicates.  */
   if (!p)
     {
-      p = ((struct mdebug_pending *)
-	   obstack_alloc (&mdebugread_objfile->objfile_obstack,
-			  sizeof (struct mdebug_pending)));
+      p = XOBNEW (&mdebugread_objfile->objfile_obstack, mdebug_pending);
       p->s = sh;
       p->t = t;
       p->next = pending_list[f_idx];
@@ -1174,10 +1171,8 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
 	  SYMBOL_DOMAIN (s) = LABEL_DOMAIN;
 	  SYMBOL_ACLASS_INDEX (s) = LOC_CONST;
 	  SYMBOL_TYPE (s) = objfile_type (mdebugread_objfile)->builtin_void;
-	  e = ((struct mdebug_extra_func_info *)
-	       obstack_alloc (&mdebugread_objfile->objfile_obstack,
-			      sizeof (struct mdebug_extra_func_info)));
-	  memset (e, 0, sizeof (struct mdebug_extra_func_info));
+	  e = OBSTACK_ZALLOC (&mdebugread_objfile->objfile_obstack,
+			      mdebug_extra_func_info);
 	  SYMBOL_VALUE_BYTES (s) = (gdb_byte *) e;
 	  e->numargs = top_stack->numargs;
 	  e->pdr.framereg = -1;
@@ -2372,8 +2367,7 @@ parse_partial_symbols (minimal_symbol_reader &reader,
       && (bfd_get_section_flags (cur_bfd, text_sect) & SEC_RELOC))
     relocatable = 1;
 
-  extern_tab = (EXTR *) obstack_alloc (&objfile->objfile_obstack,
-				       sizeof (EXTR) * hdr->iextMax);
+  extern_tab = XOBNEWVEC (&objfile->objfile_obstack, EXTR, hdr->iextMax);
 
   includes_allocated = 30;
   includes_used = 0;
@@ -2415,10 +2409,8 @@ parse_partial_symbols (minimal_symbol_reader &reader,
   }
 
   /* Allocate the global pending list.  */
-  pending_list =
-    ((struct mdebug_pending **)
-     obstack_alloc (&objfile->objfile_obstack,
-		    hdr->ifdMax * sizeof (struct mdebug_pending *)));
+  pending_list = XOBNEWVEC (&objfile->objfile_obstack, mdebug_pending *,
+			    hdr->ifdMax);
   memset (pending_list, 0,
 	  hdr->ifdMax * sizeof (struct mdebug_pending *));
 
@@ -2659,8 +2651,7 @@ parse_partial_symbols (minimal_symbol_reader &reader,
 				  textlow,
 				  objfile->global_psymbols,
 				  objfile->static_psymbols);
-      pst->read_symtab_private = obstack_alloc (&objfile->objfile_obstack,
-						sizeof (struct symloc));
+      pst->read_symtab_private = XOBNEW (&objfile->objfile_obstack, symloc);
       memset (pst->read_symtab_private, 0, sizeof (struct symloc));
 
       save_pst = pst;
@@ -3773,11 +3764,8 @@ parse_partial_symbols (minimal_symbol_reader &reader,
       /* Skip the first file indirect entry as it is a self dependency for
          source files or a reverse .h -> .c dependency for header files.  */
       pst->number_of_dependencies = 0;
-      pst->dependencies =
-	((struct partial_symtab **)
-	 obstack_alloc (&objfile->objfile_obstack,
-			((fh->crfd - 1)
-			 * sizeof (struct partial_symtab *))));
+      pst->dependencies = XOBNEWVEC (&objfile->objfile_obstack,
+				     partial_symtab *, (fh->crfd - 1));
       for (s_idx = 1; s_idx < fh->crfd; s_idx++)
 	{
 	  RFDT rh;
@@ -4064,13 +4052,11 @@ psymtab_to_symtab_1 (struct objfile *objfile,
 		{
 		  /* Make up special symbol to contain
 		     procedure specific info.  */
-		  struct mdebug_extra_func_info *e =
-		    ((struct mdebug_extra_func_info *)
-		     obstack_alloc (&mdebugread_objfile->objfile_obstack,
-				    sizeof (struct mdebug_extra_func_info)));
+		  mdebug_extra_func_info *e
+		    = OBSTACK_ZALLOC (&mdebugread_objfile->objfile_obstack,
+				      mdebug_extra_func_info);
 		  struct symbol *s = new_symbol (MDEBUG_EFI_SYMBOL_NAME);
 
-		  memset (e, 0, sizeof (struct mdebug_extra_func_info));
 		  SYMBOL_DOMAIN (s) = LABEL_DOMAIN;
 		  SYMBOL_ACLASS_INDEX (s) = LOC_CONST;
 		  SYMBOL_TYPE (s) = objfile_type (objfile)->builtin_void;
@@ -4750,9 +4736,8 @@ new_psymtab (const char *name, struct objfile *objfile)
 
   /* Keep a backpointer to the file's symbols.  */
 
-  psymtab->read_symtab_private = obstack_alloc (&objfile->objfile_obstack,
-						sizeof (struct symloc));
-  memset (psymtab->read_symtab_private, 0, sizeof (struct symloc));
+  psymtab->read_symtab_private
+    = OBSTACK_ZALLOC (&objfile->objfile_obstack, symloc);
   CUR_BFD (psymtab) = cur_bfd;
   DEBUG_SWAP (psymtab) = debug_swap;
   DEBUG_INFO (psymtab) = debug_info;
@@ -4877,9 +4862,7 @@ elfmdebug_build_psymtabs (struct objfile *objfile,
 
   minimal_symbol_reader reader (objfile);
 
-  info = ((struct ecoff_debug_info *)
-	  obstack_alloc (&objfile->objfile_obstack,
-			 sizeof (struct ecoff_debug_info)));
+  info = XOBNEW (&objfile->objfile_obstack, ecoff_debug_info);
 
   if (!(*swap->read_debug_info) (abfd, sec, info))
     error (_("Error reading ECOFF debugging information: %s"),
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 221404ee3ae7..eacc9d78a6a5 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1419,10 +1419,8 @@ void
 terminate_minimal_symbol_table (struct objfile *objfile)
 {
   if (! objfile->per_bfd->msymbols)
-    objfile->per_bfd->msymbols
-      = ((struct minimal_symbol *)
-	 obstack_alloc (&objfile->per_bfd->storage_obstack,
-			sizeof (struct minimal_symbol)));
+    objfile->per_bfd->msymbols = XOBNEW (&objfile->per_bfd->storage_obstack,
+					 minimal_symbol);
 
   {
     struct minimal_symbol *m
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 98e81c48c467..2ec358ad4dbb 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -147,11 +147,7 @@ get_objfile_bfd_data (struct objfile *objfile, struct bfd *abfd)
 	  set_bfd_data (abfd, objfiles_bfd_data, storage);
 	}
       else
-	{
-	  storage = (objfile_per_bfd_storage *)
-	    obstack_alloc (&objfile->objfile_obstack,
-			   sizeof (objfile_per_bfd_storage));
-	}
+	storage = XOBNEW (&objfile->objfile_obstack, objfile_per_bfd_storage);
 
       /* objfile_per_bfd_storage is not trivially constructible, must
 	 call the ctor manually.  */
@@ -269,8 +265,7 @@ objfile_register_static_link (struct objfile *objfile,
   slot = htab_find_slot (objfile->static_links, &lookup_entry, INSERT);
   gdb_assert (*slot == NULL);
 
-  entry = (struct static_link_htab_entry *) obstack_alloc
-	    (&objfile->objfile_obstack, sizeof (*entry));
+  entry = XOBNEW (&objfile->objfile_obstack, static_link_htab_entry);
   entry->block = block;
   entry->static_link = static_link;
   *slot = (void *) entry;
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index ac0ee0a5a64c..fa59ee2b0fb2 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1805,9 +1805,7 @@ allocate_psymtab (const char *filename, struct objfile *objfile)
       objfile->free_psymtabs = psymtab->next;
     }
   else
-    psymtab = (struct partial_symtab *)
-      obstack_alloc (&objfile->objfile_obstack,
-		     sizeof (struct partial_symtab));
+    psymtab = XOBNEW (&objfile->objfile_obstack, partial_symtab);
 
   memset (psymtab, 0, sizeof (struct partial_symtab));
   psymtab->filename
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 0017f18c35ac..49990df82181 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -2731,11 +2731,8 @@ read_member_functions (struct field_info *fip, const char **pp,
 	      xfree (main_fn_name);
 	    }
 
-	  new_fnlist->fn_fieldlist.fn_fields = (struct fn_field *)
-	    obstack_alloc (&objfile->objfile_obstack,
-			   sizeof (struct fn_field) * length);
-	  memset (new_fnlist->fn_fieldlist.fn_fields, 0,
-		  sizeof (struct fn_field) * length);
+	  new_fnlist->fn_fieldlist.fn_fields
+	    = OBSTACK_CALLOC (&objfile->objfile_obstack, length, fn_field);
 	  for (i = length; (i--, sublist); sublist = sublist->next)
 	    {
 	      new_fnlist->fn_fieldlist.fn_fields[i] = sublist->fn_field;
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 8c707aa8fe68..4f9b315eca52 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -2097,8 +2097,7 @@ xcoff_end_psymtab (struct objfile *objfile, struct partial_symtab *pst,
       struct partial_symtab *subpst =
 	allocate_psymtab (include_list[i], objfile);
 
-      subpst->read_symtab_private = obstack_alloc (&objfile->objfile_obstack,
-						   sizeof (struct symloc));
+      subpst->read_symtab_private = XOBNEW (&objfile->objfile_obstack, symloc);
       ((struct symloc *) subpst->read_symtab_private)->first_symnum = 0;
       ((struct symloc *) subpst->read_symtab_private)->numsyms = 0;
       subpst->textlow = 0;
-- 
2.17.0

[PATCH v2 1/3] Don't allocate mapped_index on the objfile obstack

[Simon Marchi]

To make things simpler, allocate it with new and manage its lifetime
using a unique_ptr (just like mapped_debug_names).  We can also
std::move the temporary local_map in the permanent map.

gdb/ChangeLog;

	* dwarf2read.h (struct dwarf2_per_objfile) <index_table>: Change
	type to std::unique_ptr.
	* dwarf2read.c (dwarf2_per_objfile::~dwarf2_per_objfile): Don't
	manually destroy mapped_index.
	(dwarf2_read_index): Adjust for unique_ptr, use std::move.
	(dw2_symtab_iter_init): Adjust for unique_ptr.
---
 gdb/dwarf2read.c | 14 ++++----------
 gdb/dwarf2read.h |  2 +-
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 9eb98b2eab8e..b3bcb43a519b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2153,9 +2153,6 @@ dwarf2_per_objfile::~dwarf2_per_objfile ()
   if (dwz_file != NULL && dwz_file->dwz_bfd)
     gdb_bfd_unref (dwz_file->dwz_bfd);
 
-  if (index_table != NULL)
-    index_table->~mapped_index ();
-
   /* Everything else should be on the objfile obstack.  */
 }
 
@@ -3541,7 +3538,7 @@ to use the section anyway."),
 static int
 dwarf2_read_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
 {
-  struct mapped_index local_map, *map;
+  struct mapped_index local_map;
   const gdb_byte *cu_list, *types_list, *dwz_list = NULL;
   offset_type cu_list_elements, types_list_elements, dwz_list_elements = 0;
   struct dwz_file *dwz;
@@ -3601,11 +3598,8 @@ dwarf2_read_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
 
   create_addrmap_from_index (dwarf2_per_objfile, &local_map);
 
-  map = XOBNEW (&objfile->objfile_obstack, struct mapped_index);
-  map = new (map) mapped_index ();
-  *map = local_map;
-
-  dwarf2_per_objfile->index_table = map;
+  dwarf2_per_objfile->index_table.reset (new mapped_index);
+  *dwarf2_per_objfile->index_table = std::move (local_map);
   dwarf2_per_objfile->using_index = 1;
   dwarf2_per_objfile->quick_file_names_table =
     create_quick_file_names_table (dwarf2_per_objfile->all_comp_units.size ());
@@ -3920,7 +3914,7 @@ dw2_symtab_iter_init (struct dw2_symtab_iterator *iter,
   iter->next = 0;
   iter->global_seen = 0;
 
-  mapped_index *index = dwarf2_per_objfile->index_table;
+  mapped_index *index = dwarf2_per_objfile->index_table.get ();
 
   /* index is NULL if OBJF_READNOW.  */
   if (index != NULL && find_slot_in_mapped_hash (index, name, &iter->vec))
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index 8e6c41dc09ef..f36d039e7bea 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -209,7 +209,7 @@ public:
   bool using_index = false;
 
   /* The mapped index, or NULL if .gdb_index is missing or not being used.  */
-  mapped_index *index_table = NULL;
+  std::unique_ptr<mapped_index> index_table;
 
   /* The mapped index, or NULL if .debug_names is missing or not being used.  */
   std::unique_ptr<mapped_debug_names> debug_names_table;
-- 
2.17.0

Re: [PATCH v2 0/3] Poison obstack functions

[Simon Marchi]

On 2018-04-27 11:31 PM, Simon Marchi wrote:
> v2 of https://sourceware.org/ml/gdb-patches/2018-04/msg00524.html
> 
> What's new:
> 
> - Don't allocate mapped_index on an obstack (patch 1/3)
> - Use OBSTACK_ZALLOC instead of XOBNEW + memset when possible (patch
>   3/3)
> 
> Simon Marchi (3):
>   Don't allocate mapped_index on the objfile obstack
>   Introduce obstack_new, poison other "typed" obstack functions
>   Use XOBNEW/XOBNEWVEC/OBSTACK_ZALLOC when possible
> 
>  gdb/ada-lang.c            |  3 +--
>  gdb/common/poison.h       | 31 +++++++++++++++++++++++-
>  gdb/common/traits.h       |  8 ++++++
>  gdb/dwarf2-frame.c        |  3 +--
>  gdb/dwarf2read.c          | 14 +++--------
>  gdb/dwarf2read.h          |  2 +-
>  gdb/gdb_obstack.h         | 36 ++++++++++++++++++++++++---
>  gdb/gdbarch.c             |  9 ++-----
>  gdb/gdbarch.h             | 10 +++++---
>  gdb/gdbarch.sh            | 21 ++++++++--------
>  gdb/hppa-tdep.c           |  7 ++----
>  gdb/mdebugread.c          | 51 +++++++++++++--------------------------
>  gdb/minsyms.c             |  6 ++---
>  gdb/objfiles.c            |  9 ++-----
>  gdb/psymtab.c             |  4 +--
>  gdb/stabsread.c           |  7 ++----
>  gdb/target-descriptions.c |  7 +-----
>  gdb/xcoffread.c           |  3 +--
>  18 files changed, 125 insertions(+), 106 deletions(-)
> 

Since Tom is playing with some things that are obstack allocated in dwarf2read, I
finally pushed this in case it helps.

Simon

Re: [PATCH v2 0/3] Poison obstack functions

[Simon Marchi]

On 2018-05-20 09:10 PM, Simon Marchi wrote:
> Since Tom is playing with some things that are obstack allocated in dwarf2read, I
> finally pushed this in case it helps.

I forgot to mention that I did not push patch 1 because it was obsoleted by a similar
patch from Tom.

Simon

[pushed] Fix copy-pasto, allocate objfile_per_bfd_storage with obstack_new

[Simon Marchi]

I realized after pushing that I made a copy-pasto, I had:

  # define HAVE_IS_TRIVIALLY_COPYABLE 1

instead of

  # define HAVE_IS_TRIVIALLY_CONSTRUCTIBLE 1

with the consequence that IsMallocable was always std::true_type (and
was therefore not enforcing anything).  Fixing that mistake triggered a
build failure:

/home/simark/src/binutils-gdb/gdb/objfiles.c:150:12:   required from here
/home/simark/src/binutils-gdb/gdb/common/poison.h:228:3: error: static assertion failed: Trying to use XOBNEW with a non-POD data type.

I am not sure why I did not see this when I originally wrote the patch
(but I saw and fixed other failures).  In any case, I swapped XOBNEW
with obstack_new to get rid of it.

Regtested on the buildbot.

gdb/ChangeLog:

	* common/traits.h (HAVE_IS_TRIVIALLY_COPYABLE): Rename the wrong
	instance to...
	(HAVE_IS_TRIVIALLY_CONSTRUCTIBLE): ... this.
	* objfiles.c (get_objfile_bfd_data): Allocate
	objfile_per_bfd_storage with obstack_new when allocating on
	obstack.
---
 gdb/ChangeLog       |  9 +++++++++
 gdb/common/traits.h |  2 +-
 gdb/objfiles.c      | 10 +++++-----
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5eb5eeaa9ad9..45e6ff719492 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2018-05-20  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* common/traits.h (HAVE_IS_TRIVIALLY_COPYABLE): Rename the wrong
+	instance to...
+	(HAVE_IS_TRIVIALLY_CONSTRUCTIBLE): ... this.
+	* objfiles.c (get_objfile_bfd_data): Allocate
+	objfile_per_bfd_storage with obstack_new when allocating on
+	obstack.
+
 2018-05-20  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* ada-lang.c (cache_symbol): Use XOBNEW and/or XOBNEWVEC and/or
diff --git a/gdb/common/traits.h b/gdb/common/traits.h
index 070ef159e5b9..e1066e0d97e5 100644
--- a/gdb/common/traits.h
+++ b/gdb/common/traits.h
@@ -38,7 +38,7 @@
    in GCC 5.  */
 #if (__has_feature(is_trivially_constructible) \
      || (defined __GNUC__ && __GNUC__ >= 5))
-# define HAVE_IS_TRIVIALLY_COPYABLE 1
+# define HAVE_IS_TRIVIALLY_CONSTRUCTIBLE 1
 #endif
 
 namespace gdb {
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 2ec358ad4dbb..f57f4f58b008 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -144,14 +144,14 @@ get_objfile_bfd_data (struct objfile *objfile, struct bfd *abfd)
 	  storage
 	    = ((struct objfile_per_bfd_storage *)
 	       bfd_alloc (abfd, sizeof (struct objfile_per_bfd_storage)));
+	  /* objfile_per_bfd_storage is not trivially constructible, must
+	     call the ctor manually.  */
+	  storage = new (storage) objfile_per_bfd_storage ();
 	  set_bfd_data (abfd, objfiles_bfd_data, storage);
 	}
       else
-	storage = XOBNEW (&objfile->objfile_obstack, objfile_per_bfd_storage);
-
-      /* objfile_per_bfd_storage is not trivially constructible, must
-	 call the ctor manually.  */
-      storage = new (storage) objfile_per_bfd_storage ();
+	storage
+	  = obstack_new<objfile_per_bfd_storage> (&objfile->objfile_obstack);
 
       /* Look up the gdbarch associated with the BFD.  */
       if (abfd != NULL)
-- 
2.17.0