patch: avoid ubsan errors in implicit copy constructors

[Nick Lewycky]

On 9 September 2013 15:33, Richard Smith <richard at metafoo.co.uk> wrote:

> Use a ctor-initializer for OldSanOpts.
>

Done.

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).
>

Yep, I do. Fixed, tests added. Furthermore, this caused me to notice a bug
in the previous patch where I failed to call reset() when AggregatedStmts
was empty. Also fixed (no testcase because no symptoms known).

Move assignment is broken, pending the other patch:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130909/088366.html
.

 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 =)
>

I punt to a later patch. The issue with us not memcpy'ing these fields is a
problem all on its own, and I would need input from the ObjC folks to know
how to proceed.

Updated patch attached! Please review, but note that it depends on the
other patch.

Nick

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/20130909/8395d646/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ubsan-bool-cvr-2.patch
Type: application/octet-stream
Size: 5490 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130909/8395d646/attachment.obj>