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

[David Blaikie]

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)

>
> We could perhaps detect by full analysis of the control and data flow through constructors and functions that it's bad to use memset on a class if it causes for instance memory leak or division by zero.
> However:
>  * It might not always be possible to see some bad behaviour when std classes are overwritten.
>  * I'd say a warning message such as "using memset on class that contains std::string member" is more clear than some more generic "memory leak".
>
> Best regards,
> Daniel Marjamäki
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits