Re: [PATCH 1/5] Poison non-POD memset & non-trivially-copyable memcpy/memmove

[Pedro Alves <palves@redhat.com>]

Hi Simon,

Sorry for the delay.  Finally managed to get back to this.

On 04/24/2017 02:12 AM, Simon Marchi wrote:
> On 2017-04-12 22:27, Pedro Alves wrote:
>> This patch catches invalid initialization of non-POD types with
>> memset, at compile time.
> 
> Would it be possible to do something similar but to catch uses of
> XNEW/XCNEW with types that need new?  XNEW is defined as:
> 
> #define XNEW(T) ((T *) xmalloc (sizeof (T)))
> 
> I just tried this, and it seems to work well:
> 
> #define assert_pod(T) static_assert(std::is_pod<T>::value)
> 
> #undef XNEW
> #define XNEW(T) ({ assert_pod(T); (T *) xmalloc (sizeof (T)); })
> #undef XCNEW
> #define XCNEW(T)  ({ assert_pod(T); (T *) xcalloc (1, sizeof (T)); })
> 
> assuming the compiler knows about statement expressions.

I think that that's a great idea!  I tried that locally and see that
this already catches two bad cases (btrace_function and objfile).

We don't need to use non-standard statement expressions though.
Function templates should work just as well here:

template<typename T>
T *xnew ()
{
  static_assert (std::is_pod<T>::value, "use operator new instead");
  return (T *) xmalloc (sizeof (T));
}

template<typename T>
T *xcnew ()
{
  static_assert (std::is_pod<T>::value, "use operator new instead");
  return (T *) xcalloc (1, sizeof (T));
}

#undef XNEW
#define XNEW(T) xnew<T>()
#undef XCNEW
#define XCNEW(T) xcnew<T>()

As should lambdas:

#undef XNEW
#define XNEW(T) [] () -> T *						\
  {									\
    static_assert (std::is_pod<T>::value, "use operator new instead");	\
    return (T *) xmalloc (sizeof (T));					\
  } ()

#undef XCNEW
#define XCNEW(T) [] () -> T *						\
  {									\
    static_assert (std::is_pod<T>::value, "use operator new instead");	\
    return (T *) xcalloc (1, sizeof (T));				\
  } ()

I think the template version is likely a little bit easier
to understand and debug (e.g., easy to put a breakpoint on the function
template, not so easy to put a breakpoint on a lambda).  I'd just
confirm that the template/lambda is completely optimized out on an
optimized build (e.g., compare out of "$ size gdb" before and after
patch).

Thanks,
Pedro Alves