From 284a0e3cbffa92ee5c94065fcdd5a528482344fc Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sun, 20 May 2018 21:06:03 -0400 Subject: [PATCH] Introduce obstack_new, poison other "typed" obstack functions 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 (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 | 24 ++++++++++++++++++++++++ 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 +------ 8 files changed, 115 insertions(+), 31 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b15d8d11a93..fb4d955b484 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,27 @@ +2018-05-20 Simon Marchi + + * 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 * stack.c (backtrace_command_1): Remove useless variable int i. diff --git a/gdb/common/poison.h b/gdb/common/poison.h index c98d2b362a0..ddab2c19b9c 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 -using IsMallocable = std::is_pod; +#if HAVE_IS_TRIVIALLY_CONSTRUCTIBLE +using IsMallocable = std::is_trivially_constructible; +#else +using IsMallocable = std::true_type; +#endif template using IsFreeable = gdb::Or, std::is_void>; @@ -216,4 +221,28 @@ non-POD data type."); #undef XRESIZEVAR #define XRESIZEVAR(T, P, S) xresizevar (P, S) +template +static T * +xobnew (obstack *ob) +{ + static_assert (IsMallocable::value, "Trying to use XOBNEW with a \ +non-POD data type."); + return XOBNEW (ob, T); +} + +#undef XOBNEW +#define XOBNEW(O, T) xobnew (O) + +template +static T * +xobnewvec (obstack *ob, size_t n) +{ + static_assert (IsMallocable::value, "Trying to use XOBNEWVEC with a \ +non-POD data type."); + return XOBNEWVEC (ob, T, n); +} + +#undef XOBNEWVEC +#define XOBNEWVEC(O, T, N) xobnewvec (O, N) + #endif /* COMMON_POISON_H */ diff --git a/gdb/common/traits.h b/gdb/common/traits.h index d9e68390112..070ef159e5b 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 1011008ffbd..29cad937e4e 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 +static inline T* +obstack_zalloc (struct obstack *ob) +{ + static_assert (IsMallocable::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 ((OBSTACK)) + +template +static inline T * +obstack_calloc (struct obstack *ob, size_t number) +{ + static_assert (IsMallocable::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 ((OBSTACK), (NUMBER)) + +/* Allocate an object on OB and call its constructor. */ + +template +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)...); + return object; +} /* Unless explicitly specified, GDB obstacks always use xmalloc() and xfree(). */ diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 82ac7519130..c430ebe52a4 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 b3a15c9898c..09edcd5eb28 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -38,6 +38,7 @@ #include #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 (gdbarch_obstack ((GDBARCH)), (NR)) + +#define GDBARCH_OBSTACK_ZALLOC(GDBARCH, TYPE) obstack_zalloc (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 bf12c684873..73304309374 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -1261,6 +1261,7 @@ cat < #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 (gdbarch_obstack ((GDBARCH)), (NR)) + +#define GDBARCH_OBSTACK_ZALLOC(GDBARCH, TYPE) \ + obstack_zalloc (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 <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 e20bf9b4698..83a6f6cafc3 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 (obstack); } /* Similar, but for the temporary copy used during architecture -- 2.30.2