diff mbox

[4/4] Poison XNEW and friends for types that should use new/delete

Message ID 1511368867-19365-5-git-send-email-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Nov. 22, 2017, 4:41 p.m. UTC 
  From: Simon Marchi <simon.marchi@polymtl.ca>

This patch (finally!) makes it so that trying to use XNEW with a type
that requires "new" will cause a compilation error.  The criterion I
initially used to allow a type to use XNEW (which calls malloc in the
end) was std::is_trivially_constructible, but then realized that gcc 4.8
did not have it.  Instead, I went with:

  using IsMallocatable = std::is_pod<T>;

which is just a bit more strict, which doesn't hurt.  A similar thing is
done for macros that free instead of allocated, the criterion is:

  using IsFreeable = gdb::Or<std::is_trivially_destructible<T>, std::is_void<T>>;

For simplicity, we could also do for std::is_pod for IsFreeable as well,
if you prefer.

I chose to put static_assert in the functions, instead of using
gdb::Requires in the template as SFINAE, because it allows to put a
message, which I think makes the compiler error more understandable.
With gdb::Requires, the error is:

    In file included from /home/simark/src/binutils-gdb/gdb/common/common-utils.h:26:0,
                     from /home/simark/src/binutils-gdb/gdb/common/common-defs.h:78,
                     from /home/simark/src/binutils-gdb/gdb/defs.h:28,
                     from /home/simark/src/binutils-gdb/gdb/lala.c:1:
    /home/simark/src/binutils-gdb/gdb/lala.c: In function ‘void foo()’:
    /home/simark/src/binutils-gdb/gdb/common/poison.h:108:25: error: no matching function for call to ‘xnew<bar>()’
     #define XNEW(T) xnew<T>()
                             ^
    /home/simark/src/binutils-gdb/gdb/lala.c:13:3: note: in expansion of macro ‘XNEW’
       XNEW(bar);
       ^~~~
    /home/simark/src/binutils-gdb/gdb/common/poison.h:101:1: note: candidate: template<class T, typename std::enable_if<std::is_trivially_constructible<_Tp>::value, void>::type <anonymous> > T* xnew()
     xnew ()
     ^~~~
    /home/simark/src/binutils-gdb/gdb/common/poison.h:101:1: note:   template argument deduction/substitution failed:
    /home/simark/src/binutils-gdb/gdb/common/poison.h:108:25: note:   couldn't deduce template parameter ‘<anonymous>’
     #define XNEW(T) xnew<T>()
                             ^
    /home/simark/src/binutils-gdb/gdb/lala.c:13:3: note: in expansion of macro ‘XNEW’
       XNEW(bar);
       ^~~~

and with static_assert:

    In file included from /home/simark/src/binutils-gdb/gdb/common/common-utils.h:26:0,
                     from /home/simark/src/binutils-gdb/gdb/common/common-defs.h:78,
                     from /home/simark/src/binutils-gdb/gdb/defs.h:28,
                     from /home/simark/src/binutils-gdb/gdb/lala.c:1:
    /home/simark/src/binutils-gdb/gdb/common/poison.h: In instantiation of ‘T* xnew() [with T = bar]’:
    /home/simark/src/binutils-gdb/gdb/lala.c:13:3:   required from here
    /home/simark/src/binutils-gdb/gdb/common/poison.h:103:3: error: static assertion failed: Trying to use XNEW with a non-POD data type.  Use operator new instead.
       static_assert (IsMallocatable<T>::value, "Trying to use XNEW with a non-POD\
       ^~~~~~~~~~~~~

I think the first one is more likely to just make people yell at their
screen, especially those less used to C++.

Generated-code-wise, it adds one more function call (xnew<T>) when using
XNEW and building with -O0, but it all goes away with optimizations
enabled.

gdb/ChangeLog:

	* common/common-utils.h: Include poison.h.
	(xfree): Remove declaration, add definition with static_assert.
	* common/common-utils.c (xfree): Remove.
	* common/poison.h (IsMallocatable): Define.
	(IsFreeable): Define.
	(free): Delete for non-freeable types.
	(xnew): New.
	(XNEW): Undef and redefine.
	(xcnew): New.
	(XCNEW): Undef and redefine.
	(xdelete): New.
	(XDELETE): Undef and redefine.
	(xnewvec): New.
	(XNEWVEC): Undef and redefine.
	(xcnewvec): New.
	(XCNEWVEC): Undef and redefine.
	(xresizevec): New.
	(XRESIZEVEC): Undef and redefine.
	(xdeletevec): New.
	(XDELETEVEC): Undef and redefine.
	(xnewvar): New.
	(XNEWVAR): Undef and redefine.
	(xcnewvar): New.
	(XCNEWVAR): Undef and redefine.
	(xresizevar): New.
	(XRESIZEVAR): Undef and redefine.
---
 gdb/common/common-utils.c |   7 ---
 gdb/common/common-utils.h |  14 ++++-
 gdb/common/poison.h       | 132 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 8 deletions(-)

Comments

Pedro Alves Nov. 23, 2017, 3:02 p.m. UTC | #1 
On 11/22/2017 04:41 PM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> This patch (finally!) makes it so that trying to use XNEW with a type
> that requires "new" will cause a compilation error.

Yay!

  The criterion I
> initially used to allow a type to use XNEW (which calls malloc in the
> end) was std::is_trivially_constructible, but then realized that gcc 4.8
> did not have it.  Instead, I went with:
> 

Yeah, in the memset poisoning I just disabled the poisoning on older
compilers.  This is the "#if HAVE_IS_TRIVIALLY_COPYABLE" check in
poison.h.

>   using IsMallocatable = std::is_pod<T>;
> 

Nit, I think "malloc-able" is more common than "malloc-atable".
At least, the latter sounds odd to me.  Or sounds like 
"m-allocatable", which is a different thing.  :-)


> which is just a bit more strict, which doesn't hurt.  A similar thing is
> done for macros that free instead of allocated, the criterion is:
> 
>   using IsFreeable = gdb::Or<std::is_trivially_destructible<T>, std::is_void<T>>;
> 
> For simplicity, we could also do for std::is_pod for IsFreeable as well,
> if you prefer.
> 
> I chose to put static_assert in the functions, instead of using
> gdb::Requires in the template as SFINAE, because it allows to put a
> message, which I think makes the compiler error more understandable.
> With gdb::Requires, the error is:

Right, I think static_assert is what I had suggest initially too.
SFINAE alone wouldn't sound right to me here.

The right alternative to static_assert would be SFINAE+"=delete;",
which is what we do for the memcpy/memcpy poisoning, and what
you're doing to "free".

=delete keeps the function/overload in the overload set, serving
as "honeypot", so you get an error about trying to call a
deleted function.

You could do that to xfree instead
of the static_assert, but I guess inlining xfree ends up being
a good thing.

> I think the first one is more likely to just make people yell at their
> screen, especially those less used to C++.

Yes, don't do that.  :-)

> 
> Generated-code-wise, it adds one more function call (xnew<T>) when using
> XNEW and building with -O0, but it all goes away with optimizations
> enabled.

Good.

> 
> gdb/ChangeLog:
> 
> 	* common/common-utils.h: Include poison.h.
> 	(xfree): Remove declaration, add definition with static_assert.
> 	* common/common-utils.c (xfree): Remove.
> 	* common/poison.h (IsMallocatable): Define.
> 	(IsFreeable): Define.
> 	(free): Delete for non-freeable types.
> 	(xnew): New.
> 	(XNEW): Undef and redefine.
> 	(xcnew): New.
> 	(XCNEW): Undef and redefine.
> 	(xdelete): New.
> 	(XDELETE): Undef and redefine.
> 	(xnewvec): New.
> 	(XNEWVEC): Undef and redefine.
> 	(xcnewvec): New.
> 	(XCNEWVEC): Undef and redefine.
> 	(xresizevec): New.
> 	(XRESIZEVEC): Undef and redefine.
> 	(xdeletevec): New.
> 	(XDELETEVEC): Undef and redefine.
> 	(xnewvar): New.
> 	(XNEWVAR): Undef and redefine.
> 	(xcnewvar): New.
> 	(XCNEWVAR): Undef and redefine.
> 	(xresizevar): New.
> 	(XRESIZEVAR): Undef and redefine.
> ---
>  gdb/common/common-utils.c |   7 ---
>  gdb/common/common-utils.h |  14 ++++-
>  gdb/common/poison.h       | 132 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 145 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
> index 7139302..66d6161 100644
> --- a/gdb/common/common-utils.c
> +++ b/gdb/common/common-utils.c
> @@ -95,13 +95,6 @@ xzalloc (size_t size)
>  }
>  
>  void
> -xfree (void *ptr)
> -{
> -  if (ptr != NULL)
> -    free (ptr);		/* ARI: free */
> -}
> -
> -void
>  xmalloc_failed (size_t size)
>  {
>    malloc_failure (size);
> diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
> index 4926a32..feb4790 100644
> --- a/gdb/common/common-utils.h
> +++ b/gdb/common/common-utils.h
> @@ -23,6 +23,8 @@
>  #include <string>
>  #include <vector>
>  
> +#include "poison.h"
> +
>  /* If possible, define FUNCTION_NAME, a macro containing the name of
>     the function being defined.  Since this macro may not always be
>     defined, all uses must be protected by appropriate macro definition
> @@ -47,7 +49,17 @@
>  /* Like xmalloc, but zero the memory.  */
>  void *xzalloc (size_t);
>  
> -void xfree (void *);
> +template <typename T>
> +static void
> +xfree (T *ptr)
> +{
> +  static_assert (IsFreeable<T>::value, "Trying to use xfree with a non-POD \
> +data type.  Use operator delete instead.");
> +
> +  if (ptr != NULL)
> +    free (ptr);		/* ARI: free */
> +}
> +
>  
>  /* Like asprintf and vasprintf, but return the string, throw an error
>     if no memory.  */
> diff --git a/gdb/common/poison.h b/gdb/common/poison.h
> index 37dd35e..de4cefa 100644
> --- a/gdb/common/poison.h
> +++ b/gdb/common/poison.h
> @@ -84,4 +84,136 @@ void *memmove (D *dest, const S *src, size_t n) = delete;
>  
>  #endif /* HAVE_IS_TRIVIALLY_COPYABLE */
>  
> +/* Poison XNEW and friends to catch usages of malloc-style allocations on
> +   objects that require new/delete.  */
> +
> +template<typename T>
> +using IsMallocatable = std::is_pod<T>;
> +
> +template<typename T>
> +using IsFreeable = gdb::Or<std::is_trivially_destructible<T>, std::is_void<T>>;
> +
> +template <typename T, typename = gdb::Requires<gdb::Not<IsFreeable<T>>>>
> +void free (T *ptr) = delete;
> +
> +template<typename T>
> +static T *
> +xnew ()
> +{
> +  static_assert (IsMallocatable<T>::value, "Trying to use XNEW with a non-POD \
> +data type.  Use operator new instead.");
> +  return XNEW (T);
> +}
> +
> +#undef XNEW
> +#define XNEW(T) xnew<T>()
> +
> +template<typename T>
> +static T *
> +xcnew ()
> +{
> +  static_assert (IsMallocatable<T>::value, "Trying to use XCNEW with a non-POD \
> +data type.  Use operator new instead.");
> +  return XCNEW (T);
> +}
> +
> +#undef XCNEW
> +#define XCNEW(T) xcnew<T>()
> +
> +template<typename T>
> +static void
> +xdelete (T *p)
> +{
> +  static_assert (IsFreeable<T>::value, "Trying to use XDELETE with a non-POD \
> +data type.  Use operator delete instead.");
> +  XDELETE (p);
> +}
> +
> +#undef XDELETE
> +#define XDELETE(P) xdelete (p)
> +
> +template<typename T>
> +static T*

Missing space after T in several of these functions.

> +xnewvec (size_t n)
> +{
> +  static_assert (IsMallocatable<T>::value, "Trying to use XNEWVEC with a \
> +non-POD data type.  Use operator new[] (or std::vector) instead.");
> +  return XNEWVEC (T, n);
> +}
> +

Other than the formatting, this LGTM.  Thanks a lot for doing this.

Thanks,
Pedro Alves
Simon Marchi Nov. 23, 2017, 5:27 p.m. UTC | #2 
On 2017-11-23 10:02, Pedro Alves wrote:
> On 11/22/2017 04:41 PM, Simon Marchi wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> 
>> This patch (finally!) makes it so that trying to use XNEW with a type
>> that requires "new" will cause a compilation error.
> 
> Yay!
> 
>   The criterion I
>> initially used to allow a type to use XNEW (which calls malloc in the
>> end) was std::is_trivially_constructible, but then realized that gcc 
>> 4.8
>> did not have it.  Instead, I went with:
>> 
> 
> Yeah, in the memset poisoning I just disabled the poisoning on older
> compilers.  This is the "#if HAVE_IS_TRIVIALLY_COPYABLE" check in
> poison.h.

I'm not sure if you are suggesting adding 
HAVE_IS_TRIVIALLY_CONSTRUCTIBLE, but since we can use std::is_pod 
easily, I think it's simpler just to use that (even if it's a bit more 
strict than needed).

>>   using IsMallocatable = std::is_pod<T>;
>> 
> 
> Nit, I think "malloc-able" is more common than "malloc-atable".
> At least, the latter sounds odd to me.  Or sounds like
> "m-allocatable", which is a different thing.  :-)

Agreed.
> 
>> which is just a bit more strict, which doesn't hurt.  A similar thing 
>> is
>> done for macros that free instead of allocated, the criterion is:
>> 
>>   using IsFreeable = gdb::Or<std::is_trivially_destructible<T>, 
>> std::is_void<T>>;
>> 
>> For simplicity, we could also do for std::is_pod for IsFreeable as 
>> well,
>> if you prefer.
>> 
>> I chose to put static_assert in the functions, instead of using
>> gdb::Requires in the template as SFINAE, because it allows to put a
>> message, which I think makes the compiler error more understandable.
>> With gdb::Requires, the error is:
> 
> Right, I think static_assert is what I had suggest initially too.
> SFINAE alone wouldn't sound right to me here.
> 
> The right alternative to static_assert would be SFINAE+"=delete;",
> which is what we do for the memcpy/memcpy poisoning, and what
> you're doing to "free".
> 
> =delete keeps the function/overload in the overload set, serving
> as "honeypot", so you get an error about trying to call a
> deleted function.
> 
> You could do that to xfree instead
> of the static_assert, but I guess inlining xfree ends up being
> a good thing.

Ah yeah, it would probably make the message a bit clearer.  Instead of 
saying "I can't find a function that matches", it would say "I found a 
function that matches, but you can't use it".  I'll just keep 
static_assert for xfree as well, since it ends up clearer anyway.

>> I think the first one is more likely to just make people yell at their
>> screen, especially those less used to C++.
> 
> Yes, don't do that.  :-)
> 
>> 
>> Generated-code-wise, it adds one more function call (xnew<T>) when 
>> using
>> XNEW and building with -O0, but it all goes away with optimizations
>> enabled.
> 
> Good.
> 
>> 
>> gdb/ChangeLog:
>> 
>> 	* common/common-utils.h: Include poison.h.
>> 	(xfree): Remove declaration, add definition with static_assert.
>> 	* common/common-utils.c (xfree): Remove.
>> 	* common/poison.h (IsMallocatable): Define.
>> 	(IsFreeable): Define.
>> 	(free): Delete for non-freeable types.
>> 	(xnew): New.
>> 	(XNEW): Undef and redefine.
>> 	(xcnew): New.
>> 	(XCNEW): Undef and redefine.
>> 	(xdelete): New.
>> 	(XDELETE): Undef and redefine.
>> 	(xnewvec): New.
>> 	(XNEWVEC): Undef and redefine.
>> 	(xcnewvec): New.
>> 	(XCNEWVEC): Undef and redefine.
>> 	(xresizevec): New.
>> 	(XRESIZEVEC): Undef and redefine.
>> 	(xdeletevec): New.
>> 	(XDELETEVEC): Undef and redefine.
>> 	(xnewvar): New.
>> 	(XNEWVAR): Undef and redefine.
>> 	(xcnewvar): New.
>> 	(XCNEWVAR): Undef and redefine.
>> 	(xresizevar): New.
>> 	(XRESIZEVAR): Undef and redefine.
>> ---
>>  gdb/common/common-utils.c |   7 ---
>>  gdb/common/common-utils.h |  14 ++++-
>>  gdb/common/poison.h       | 132 
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 145 insertions(+), 8 deletions(-)
>> 
>> diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
>> index 7139302..66d6161 100644
>> --- a/gdb/common/common-utils.c
>> +++ b/gdb/common/common-utils.c
>> @@ -95,13 +95,6 @@ xzalloc (size_t size)
>>  }
>> 
>>  void
>> -xfree (void *ptr)
>> -{
>> -  if (ptr != NULL)
>> -    free (ptr);		/* ARI: free */
>> -}
>> -
>> -void
>>  xmalloc_failed (size_t size)
>>  {
>>    malloc_failure (size);
>> diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
>> index 4926a32..feb4790 100644
>> --- a/gdb/common/common-utils.h
>> +++ b/gdb/common/common-utils.h
>> @@ -23,6 +23,8 @@
>>  #include <string>
>>  #include <vector>
>> 
>> +#include "poison.h"
>> +
>>  /* If possible, define FUNCTION_NAME, a macro containing the name of
>>     the function being defined.  Since this macro may not always be
>>     defined, all uses must be protected by appropriate macro 
>> definition
>> @@ -47,7 +49,17 @@
>>  /* Like xmalloc, but zero the memory.  */
>>  void *xzalloc (size_t);
>> 
>> -void xfree (void *);
>> +template <typename T>
>> +static void
>> +xfree (T *ptr)
>> +{
>> +  static_assert (IsFreeable<T>::value, "Trying to use xfree with a 
>> non-POD \
>> +data type.  Use operator delete instead.");
>> +
>> +  if (ptr != NULL)
>> +    free (ptr);		/* ARI: free */
>> +}
>> +
>> 
>>  /* Like asprintf and vasprintf, but return the string, throw an error
>>     if no memory.  */
>> diff --git a/gdb/common/poison.h b/gdb/common/poison.h
>> index 37dd35e..de4cefa 100644
>> --- a/gdb/common/poison.h
>> +++ b/gdb/common/poison.h
>> @@ -84,4 +84,136 @@ void *memmove (D *dest, const S *src, size_t n) = 
>> delete;
>> 
>>  #endif /* HAVE_IS_TRIVIALLY_COPYABLE */
>> 
>> +/* Poison XNEW and friends to catch usages of malloc-style 
>> allocations on
>> +   objects that require new/delete.  */
>> +
>> +template<typename T>
>> +using IsMallocatable = std::is_pod<T>;
>> +
>> +template<typename T>
>> +using IsFreeable = gdb::Or<std::is_trivially_destructible<T>, 
>> std::is_void<T>>;
>> +
>> +template <typename T, typename = 
>> gdb::Requires<gdb::Not<IsFreeable<T>>>>
>> +void free (T *ptr) = delete;
>> +
>> +template<typename T>
>> +static T *
>> +xnew ()
>> +{
>> +  static_assert (IsMallocatable<T>::value, "Trying to use XNEW with a 
>> non-POD \
>> +data type.  Use operator new instead.");
>> +  return XNEW (T);
>> +}
>> +
>> +#undef XNEW
>> +#define XNEW(T) xnew<T>()
>> +
>> +template<typename T>
>> +static T *
>> +xcnew ()
>> +{
>> +  static_assert (IsMallocatable<T>::value, "Trying to use XCNEW with 
>> a non-POD \
>> +data type.  Use operator new instead.");
>> +  return XCNEW (T);
>> +}
>> +
>> +#undef XCNEW
>> +#define XCNEW(T) xcnew<T>()
>> +
>> +template<typename T>
>> +static void
>> +xdelete (T *p)
>> +{
>> +  static_assert (IsFreeable<T>::value, "Trying to use XDELETE with a 
>> non-POD \
>> +data type.  Use operator delete instead.");
>> +  XDELETE (p);
>> +}
>> +
>> +#undef XDELETE
>> +#define XDELETE(P) xdelete (p)
>> +
>> +template<typename T>
>> +static T*
> 
> Missing space after T in several of these functions.

Oops, thanks.

>> +xnewvec (size_t n)
>> +{
>> +  static_assert (IsMallocatable<T>::value, "Trying to use XNEWVEC 
>> with a \
>> +non-POD data type.  Use operator new[] (or std::vector) instead.");
>> +  return XNEWVEC (T, n);
>> +}
>> +
> 
> Other than the formatting, this LGTM.  Thanks a lot for doing this.

Thanks, I'm glad that I won't have to carry that branch on the side 
anymore :)

Simon
Pedro Alves Nov. 23, 2017, 5:31 p.m. UTC | #3 
On 11/23/2017 05:27 PM, Simon Marchi wrote:
> On 2017-11-23 10:02, Pedro Alves wrote:
>> On 11/22/2017 04:41 PM, Simon Marchi wrote:
>>> From: Simon Marchi <simon.marchi@polymtl.ca>

>>   The criterion I
>>> initially used to allow a type to use XNEW (which calls malloc in the
>>> end) was std::is_trivially_constructible, but then realized that gcc 4.8
>>> did not have it.  Instead, I went with:
>>>
>>
>> Yeah, in the memset poisoning I just disabled the poisoning on older
>> compilers.  This is the "#if HAVE_IS_TRIVIALLY_COPYABLE" check in
>> poison.h.
> 
> I'm not sure if you are suggesting adding
> HAVE_IS_TRIVIALLY_CONSTRUCTIBLE, but since we can use std::is_pod
> easily, I think it's simpler just to use that (even if it's a bit more
> strict than needed).

Nope, I was really just stating a fact.

>>> which is just a bit more strict, which doesn't hurt.  A similar thing is
>>> done for macros that free instead of allocated, the criterion is:
>>>
>>>   using IsFreeable = gdb::Or<std::is_trivially_destructible<T>,
>>> std::is_void<T>>;
>>>
>>> For simplicity, we could also do for std::is_pod for IsFreeable as well,
>>> if you prefer.
>>>
>>> I chose to put static_assert in the functions, instead of using
>>> gdb::Requires in the template as SFINAE, because it allows to put a
>>> message, which I think makes the compiler error more understandable.
>>> With gdb::Requires, the error is:
>>
>> Right, I think static_assert is what I had suggest initially too.
>> SFINAE alone wouldn't sound right to me here.
>>
>> The right alternative to static_assert would be SFINAE+"=delete;",
>> which is what we do for the memcpy/memcpy poisoning, and what
>> you're doing to "free".
>>
>> =delete keeps the function/overload in the overload set, serving
>> as "honeypot", so you get an error about trying to call a
>> deleted function.
>>
>> You could do that to xfree instead
>> of the static_assert, but I guess inlining xfree ends up being
>> a good thing.
> 
> Ah yeah, it would probably make the message a bit clearer.  Instead of
> saying "I can't find a function that matches", it would say "I found a
> function that matches, but you can't use it".  

Exactly.

> I'll just keep
> static_assert for xfree as well, since it ends up clearer anyway.

*nod*

>> Other than the formatting, this LGTM.  Thanks a lot for doing this.
> 
> Thanks, I'm glad that I won't have to carry that branch on the side
> anymore :)
> 

:-)

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
index 7139302..66d6161 100644
--- a/gdb/common/common-utils.c
+++ b/gdb/common/common-utils.c
@@ -95,13 +95,6 @@  xzalloc (size_t size)
 }
 
 void
-xfree (void *ptr)
-{
-  if (ptr != NULL)
-    free (ptr);		/* ARI: free */
-}
-
-void
 xmalloc_failed (size_t size)
 {
   malloc_failure (size);
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 4926a32..feb4790 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -23,6 +23,8 @@ 
 #include <string>
 #include <vector>
 
+#include "poison.h"
+
 /* If possible, define FUNCTION_NAME, a macro containing the name of
    the function being defined.  Since this macro may not always be
    defined, all uses must be protected by appropriate macro definition
@@ -47,7 +49,17 @@ 
 /* Like xmalloc, but zero the memory.  */
 void *xzalloc (size_t);
 
-void xfree (void *);
+template <typename T>
+static void
+xfree (T *ptr)
+{
+  static_assert (IsFreeable<T>::value, "Trying to use xfree with a non-POD \
+data type.  Use operator delete instead.");
+
+  if (ptr != NULL)
+    free (ptr);		/* ARI: free */
+}
+
 
 /* Like asprintf and vasprintf, but return the string, throw an error
    if no memory.  */
diff --git a/gdb/common/poison.h b/gdb/common/poison.h
index 37dd35e..de4cefa 100644
--- a/gdb/common/poison.h
+++ b/gdb/common/poison.h
@@ -84,4 +84,136 @@  void *memmove (D *dest, const S *src, size_t n) = delete;
 
 #endif /* HAVE_IS_TRIVIALLY_COPYABLE */
 
+/* Poison XNEW and friends to catch usages of malloc-style allocations on
+   objects that require new/delete.  */
+
+template<typename T>
+using IsMallocatable = std::is_pod<T>;
+
+template<typename T>
+using IsFreeable = gdb::Or<std::is_trivially_destructible<T>, std::is_void<T>>;
+
+template <typename T, typename = gdb::Requires<gdb::Not<IsFreeable<T>>>>
+void free (T *ptr) = delete;
+
+template<typename T>
+static T *
+xnew ()
+{
+  static_assert (IsMallocatable<T>::value, "Trying to use XNEW with a non-POD \
+data type.  Use operator new instead.");
+  return XNEW (T);
+}
+
+#undef XNEW
+#define XNEW(T) xnew<T>()
+
+template<typename T>
+static T *
+xcnew ()
+{
+  static_assert (IsMallocatable<T>::value, "Trying to use XCNEW with a non-POD \
+data type.  Use operator new instead.");
+  return XCNEW (T);
+}
+
+#undef XCNEW
+#define XCNEW(T) xcnew<T>()
+
+template<typename T>
+static void
+xdelete (T *p)
+{
+  static_assert (IsFreeable<T>::value, "Trying to use XDELETE with a non-POD \
+data type.  Use operator delete instead.");
+  XDELETE (p);
+}
+
+#undef XDELETE
+#define XDELETE(P) xdelete (p)
+
+template<typename T>
+static T*
+xnewvec (size_t n)
+{
+  static_assert (IsMallocatable<T>::value, "Trying to use XNEWVEC with a \
+non-POD data type.  Use operator new[] (or std::vector) instead.");
+  return XNEWVEC (T, n);
+}
+
+#undef XNEWVEC
+#define XNEWVEC(T, N) xnewvec<T> (N)
+
+template<typename T>
+static T*
+xcnewvec (size_t n)
+{
+  static_assert (IsMallocatable<T>::value, "Trying to use XCNEWVEC with a \
+non-POD data type.  Use operator new[] (or std::vector) instead.");
+  return XCNEWVEC (T, n);
+}
+
+#undef XCNEWVEC
+#define XCNEWVEC(T, N) xcnewvec<T> (N)
+
+template<typename T>
+static T*
+xresizevec (T *p, size_t n)
+{
+  static_assert (IsMallocatable<T>::value, "Trying to use XRESIZEVEC with a \
+non-POD data type.");
+  return XRESIZEVEC (T, p, n);
+}
+
+#undef XRESIZEVEC
+#define XRESIZEVEC(T, P, N) xresizevec<T> (P, N)
+
+template<typename T>
+static void
+xdeletevec (T *p)
+{
+  static_assert (IsFreeable<T>::value, "Trying to use XDELETEVEC with a \
+non-POD data type.  Use operator delete[] (or std::vector) instead.");
+  XDELETEVEC (p);
+}
+
+#undef XDELETEVEC
+#define XDELETEVEC(P) xdeletevec (P)
+
+template<typename T>
+static T*
+xnewvar (size_t s)
+{
+  static_assert (IsMallocatable<T>::value, "Trying to use XNEWVAR with a \
+non-POD data type.");
+  return XNEWVAR (T, s);;
+}
+
+#undef XNEWVAR
+#define XNEWVAR(T, S) xnewvar<T> (S)
+
+template<typename T>
+static T*
+xcnewvar (size_t s)
+{
+  static_assert (IsMallocatable<T>::value, "Trying to use XCNEWVAR with a \
+non-POD data type.");
+  return XCNEWVAR (T, s);
+}
+
+#undef XCNEWVAR
+#define XCNEWVAR(T, S) xcnewvar<T> (S)
+
+template<typename T>
+static T*
+xresizevar (T *p, size_t s)
+{
+  static_assert (IsMallocatable<T>::value, "Trying to use XRESIZEVAR with a \
+non-POD data type.");
+  return XRESIZEVAR (T, p, s);
+}
+
+#undef XRESIZEVAR
+#define XRESIZEVAR(T, P, S) xresizevar<T> (P, S)
+
 #endif /* COMMON_POISON_H */