patch: avoid ubsan errors in implicit copy constructors

[Richard Smith]

Updated patch LGTM, with a test for the enum case.


On Mon, Sep 9, 2013 at 3:33 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> Use a ctor-initializer for OldSanOpts.
>
> Do you need to handle the assignment operator too? A test for move
> constructors / move assignments would be great, too (not that I expect any
> problems there).
>
> Do you care that the UBSan checks will still be enabled
> when CGF.getLangOpts().getGC() != LangOptions::NonGC? We seem to also fail
> to memcpy non-GC'd fields in that case too =)
>
>
> On Mon, Sep 9, 2013 at 3:03 PM, Nick Lewycky <nlewycky at google.com> wrote:
>
>> The attached patch disables the bool and enum sanitizers when emitting
>> the implicitly-defined copy constructor.
>>
>> To start with an example:
>>   struct X { X(); X(const X&); };
>>   struct Y { X x; bool b; };
>> if you don't initialize Y::b then try to copy an object of Y type, ubsan
>> will complain. Technically, with the standard as written, ubsan is correct.
>> However, this is a useful thing to do -- you may have a discriminator which
>> decides which elements are not interesting and therefore never initialize
>> or read them. Secondly, it's a departure from the rules in C, making
>> well-defined C code have undefined behaviour in C++ (structs are never trap
>> values, see C99 6.2.6.1p6). Thirdly, it's checked incompletely right now --
>> if you make subtle changes (f.e. add an "int i;" member to Y) ubsan will
>> stop complaining. The semantic I'm implementing is as if the implicit copy
>> constructor is copying the value representation (the bytes) not the object
>> representation (the meaning of those bytes).
>>
>> Nick
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130910/16571db7/attachment.html>