This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560)


On 04/30/2017 02:02 PM, Pedro Alves wrote:
Hi Martin,

Thanks much for doing this.  A few comments below, in light of my
experience doing the equivalent checks in the gdb patch linked below,
using standard C++11.

Thanks for the feedback!  It gave me quite a bit to think about.

The challenge with using memcpy or memset with class types is
figuring out if it's being called to copy-construct a new object
or assign a new value to an existing one.  In general it's not
possible to tell so the conservative assumption is that it's
the latter.

Because of that, relying on the trivially copyable property to
determine whether it's safe to assign a new value to an object
is not sufficient (or even necessary).  The class must be at
a minimum trivially assignable.  But it turns out that even
trivial assignability as defined by C++ isn't enough.  A class
with a const data member or a member of a reference type is
considered "trivially assignable" but its copy assignment is
deleted, and so it can't be assigned to.   Calling memset or
memcpy to write over an object of such a class can violate
the const invariant or corrupt the reference, respectively.

On the other hand, relying on the standard layout property
to decide whether or not calling memset on an object is safe,
while on the surface reasonable, is at the same time too strict
and overly permissive.  It's too strict because it warns for
calls where the destination is an object of a trivial derived
class that declares data members in one of its bases as well as
in the derived class (GCC has code like that).  It's not strict
enough because it doesn't catch cases where the class contains
only private or only protected data members (GCC is guilty of
abusing memset to violate encapsulation like that as well).

That said, I'm testing a solution that overcomes these problems.
I adjusted it so it doesn't warn on the GDB code in your example
(or any GDB code on trunk), even though in my opinion "tricks"
like that would best be avoided in favor of safer alternatives.

Unlike in C, the preferred way to initialize objects in C++
is to use some form of initialization (as opposed to memset).
The preferred way to copy objects is using the copy ctor or
assignment operator (as opposed to memcpy).  Using the special
member functions is clearer, less prone to mistakes and so
safer, and in some cases can also be more efficient.  Memcpy
and memset should be reserved for manipulating raw storage,
not typed objects.

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]