https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89729
Martin Sebor <msebor at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |diagnostic
Status|UNCONFIRMED |RESOLVED
CC| |msebor at gcc dot gnu.org
Resolution|--- |INVALID
--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
Declaring a default ctor for a class like script_data_t in attachment 45975
means that objects of the class need to be constructed and initialized using
the default ctor. Calling memset to zero-out an object of such a class
suggests the caller might be treating the class as a plain old C struct which
it shouldn't be: it could bypass the ctor and fail to begin the object's
lifetime or break invariants previously established by the ctor.
Similarly, declaring a class copy assignment operator private means that the
class is not meant to be copy-assignable. Calling memcpy on an object of such
a class could violate invariants established by its ctor/assignment operator.
The warning points out these potential bugs. It's meant to help avoid them
especially when C code is transitioning to C++, e.g., when an object of C++
class is added as a member of a plain C struct (imagine what might happen if a
std::string member were added to script_data_t).
In the test case in attachment 45975, d2 is initialized on declaration:
void script_data_init_empty(script_data_t *data)
{
script_data_t d2; // ctor initializes (e.g., clears)
// p_clear(&d2, 1); // should not be necessary (done by ctor)
// memcpy(data, &d2, sizeof(d2)); // unsafe
// if data points to an already initialized script_data_t object
// destroy it first:
data->~script_data_t()
// ...and then create a new object in its place by using
// the implicitly provided copy ctor (or just create a new
// one if data points to uninitialized memory):
new (data) script_data_t (d2);
}
But judging by the name of the function I would expect it to be a no-op in C++:
there should be no need to call it to initialize a script_data_t object: its
ctor does it. Simply declaring an object of the class initializes it. The
same is true for objects dynamically allocated via a new expression. If you
allocate memory for those objects using malloc then initialize them using
placement new as shown above (and remember to destroy them by explicitly
calling the dtor before freeing the memory.
It's also possible (even likely) that script_data_t is poorly designed and that
calling memset/memcpy on it is, in fact, safe and necessary. A class that
declares a private copy assignment probably isn't meant to be constructible
either and so it should also make its copy constructor inaccessible (by
declaring it private in C++ 98). Either way, when dealing with a poorly
designed class that can't be easily fixed, casting the pointer to it to void*
before passing it to memcpy/memset suppresses the warning:
#define p_clear(p, count)
\
({ (typeof(*(p)) *)memset((void*)(p), 0, sizeof(*(p)) * (count)); })
You can also use #pragma GCC diagnostic to suppress the warning before each
such access (and restore it afterwards):
void script_data_init_empty(script_data_t *data)
{
script_data_t d2;
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wclass-memaccess"
p_clear(&d2, 1); // no warning here
#pragma GCC diagnostic pop
memcpy(data, &d2, sizeof(d2)); // warning here
}
