From: Simon Marchi <simon.marchi@ericsson.com>
To: <gdb-patches@sourceware.org>
Cc: Simon Marchi <simon.marchi@ericsson.com>
Subject: [PATCH 1/2] Introduce obstack_new, poison other "typed" obstack functions
Date: Wed, 25 Apr 2018 19:40:00 -0000 [thread overview]
Message-ID: <1524685183-5553-1-git-send-email-simon.marchi@ericsson.com> (raw)
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;
This patch introduces a utility to make this pattern simpler:
foo *f = obstack_new<foo> ();
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.
* dwarf2read.c (dwarf2_read_index): Use obstack_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/dwarf2read.c | 3 +--
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, 92 insertions(+), 33 deletions(-)
diff --git a/gdb/common/poison.h b/gdb/common/poison.h
index c98d2b3..ddab2c1 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 d9e6839..070ef15 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/dwarf2read.c b/gdb/dwarf2read.c
index 4207e4c..80185a9 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3601,8 +3601,7 @@ 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 = obstack_new<mapped_index> (&objfile->objfile_obstack);
*map = local_map;
dwarf2_per_objfile->index_table = map;
diff --git a/gdb/gdb_obstack.h b/gdb/gdb_obstack.h
index 1011008..29cad93 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 1359c2f..4b4ae0b 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 0084f19..d42e69c 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 4fc54cb..ed407cb 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 36ea4b1..61fb344 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.7.4
next reply other threads:[~2018-04-25 19:40 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-25 19:40 Simon Marchi [this message]
2018-04-25 19:40 ` [PATCH 2/2] Use XOBNEW when possible Simon Marchi
2018-04-25 22:27 ` Tom Tromey
2018-04-26 2:59 ` Simon Marchi
2018-04-25 22:36 ` [PATCH 1/2] Introduce obstack_new, poison other "typed" obstack functions Tom Tromey
2018-04-26 2:58 ` Simon Marchi
2018-04-26 17:42 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1524685183-5553-1-git-send-email-simon.marchi@ericsson.com \
--to=simon.marchi@ericsson.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).