Introduce obstack_new, poison other "typed" obstack functions
authorSimon Marchi <simon.marchi@ericsson.com>
Mon, 21 May 2018 01:06:03 +0000 (21:06 -0400)
committerSimon Marchi <simon.marchi@polymtl.ca>
Mon, 21 May 2018 01:06:36 +0000 (21:06 -0400)
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 allocate 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> (obstack);

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/ChangeLog
gdb/common/poison.h
gdb/common/traits.h
gdb/gdb_obstack.h
gdb/gdbarch.c
gdb/gdbarch.h
gdb/gdbarch.sh
gdb/target-descriptions.c

index b15d8d11a93b963b0ae45e86687b05e766fb0aee..fb4d955b484ca858e0a2358c9139f0c981d01a36 100644 (file)
@@ -1,3 +1,27 @@
+2018-05-20  Simon Marchi  <simon.marchi@ericsson.com>
+
+       * 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.
+
 2018-05-19  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
        * stack.c (backtrace_command_1): Remove useless variable int i.
index c98d2b362a0f1560a0e6e78af5f7da1dcd6b135e..ddab2c19b9cb59de45a6bc3f7f9c81929c8a4e66 100644 (file)
@@ -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 */
index d9e683901129ac4a800b37148938326856843647..070ef159e5b9da289bb7136bc3418111a2ace3f6 100644 (file)
 # 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
index 1011008ffbd3cb64291e8ab438bbbd43b82ad76c..29cad937e4ee7fc44229ebe52101e9c5c5330e50 100644 (file)
 
 /* 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().  */
index 82ac7519130d67bca780982fb8ccdd8ac0c4ec1b..c430ebe52a4f9808d95ef6e04e03488f02956ae7 100644 (file)
@@ -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.  */
index b3a15c9898c479f6a1c44659859c398f9088f366..09edcd5eb2820a8214f0ac3fd6275ca8267f7cd7 100644 (file)
@@ -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
index bf12c6848739bafac9b8174fc84053b18cc9b9c4..73304309374281df8e457d1045ffbccc915eba81 100755 (executable)
@@ -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.  */
index e20bf9b4698bd75f9063adf1d28632014bec4da8..83a6f6cafc32690b75724d35332bd268df7c423f 100644 (file)
@@ -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