[PATCH] New warning when using memset etc on std class data

[Jordan Rose]

On Dec 2, 2013, at 8:51, David Blaikie <dblaikie at gmail.com> wrote:

> On Sun, Dec 1, 2013 at 10:17 PM, Daniel Marjamäki
> <Daniel.Marjamaki at evidente.se> wrote:
>> 
>> Hello!
>> 
>> I try again (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131111/093111.html).
>> 
>> Sometimes memset is used on classes. In general this is ok. However using memset/memcpy/memmove on standard stl classes, such as std::string, is a bad idea. This patch adds warnings when memset is used on a class that has stl class members.
>> 
>> My patch is hardcoded for std classes. I normally would not suggest hard coding. However when it comes to standard C/C++ classes and functions I wonder if it's ok with hard coding?
>> 
>> There was a comment to make this check more generic by looking at the classes instead. I don't see any trivial class property that makes using memset on the class bad, except when there is a virtual table and when there are reference members. Warnings about that doesn't mean there will be warnings about STL classes.
> 
> What features did you consider?
> 
> I imagine "this class has a copy constructor" might be a fairly good
> hint. But it'd be useful to run any such heuristic implementations
> over a large code base to see if there is a heuristic that's
> sufficiently powerful.
> 
> I'm not sure whether any of copy ctor, copy assignment, default ctor,
> or dtor would be sufficient - perhaps a combination of several would
> be necessary. (dtor seems one of the more likely signals)
> 
> And of course we'd /probably/ want to suppress this warning inside the
> implementation of any member functions of a type. (eg: one might
> implement the default ctor with a memset call)

I'm still with David on this; hardcoding STL is silly and error-prone. std::pair, for example, can be memset just fine. So can std::array, std::tuple, and most data structures pulled in from the C standard library.

I'd be happy to say "warn on anything non-trivially-copyable" and see how that performs on a large code base. That's still not perfect for memset, since sometimes the value being set isn't a valid value for all fields, but it's a fairly good heuristic that isn't as likely to depend on the standard changing.

Jordan