This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/5] Poison non-POD memset & non-trivially-copyable memcpy/memmove
On 04/24/2017 02:53 AM, Simon Marchi wrote:
> On 2017-04-23 21:12, 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.
>
> Actually, it should probably use std::is_trivially_constructible.
> And I
> suppose we could do the same with xfree, delete it when
> !std::is_trivially_destructible.
I think you wanted std::is_trivially_default_constructible
for XNEW. I think that we want _both_ conditions (*constructible
and *destructible) on both XNEW and xfree. For example, it'll be
good to catch the mismatching new/delete that could sneak in otherwise:
// type with trivial constructor
struct A
{
// A() = default;
~A() { /* do something with side effects */ } // not trivial
};
// type with trivial destructor
struct B
{
B() { /* do something with side effects */ } // not trivial
//~B() = default;
};
void foo ()
{
A *a = XNEW (struct A);
delete a;
B *b = new B;
xfree (b);
}
Calling delete on a pointer not allocated with new is undefined behavior.
These mismatches are also flagged by -fsanitize=address, but
making them compile-time errors would be even better.
This wouldn't catch allocating types that are both trivially
default constructible and trivially destructible, and which _also_
have non-default ctors, like this, for example:
struct C
{
C() = default;
explicit C(int) { /* some side effects */ }
};
static_assert (std::is_trivially_default_constructible<C>::value, "");
static_assert (std::is_trivially_destructible<C>::value, "");
C *b = new C(1);
xfree (b); // whoops, technically undefined. -fsanitify=address likely complains.
but std::is_pod wouldn't either.
If we make a type non-standard-layout, then it no longer is POD:
struct D
{
// Mix of public/private fields => not POD
public:
int a;
private:
int b;
};
This (D) case is likely to not really be problematic in practice WRT
to allocation/deallocation with malloc/free, but it still feels
like a code smell to me. I'd be willing to try forcing use
of new/delete for these types too. This would suggest using the
bigger std::is_pod hammer in XNEW/xfree instead of just
std::is_trivially_*ctible. But I'd understand if others disagree.
Thanks,
Pedro Alves