[cfe-dev] [LLVMdev] ubsan - active member check for unions

[Nick Lewycky]

On 12/16/2014 02:43 AM, Yury Gribov wrote:
> On 12/15/2014 10:24 PM, Ismail Pazarbasi wrote:
>>      s.d = 42.0;
>>      if (s.l > 100) // fire here
>
> Note that code like this is frequently used to convert integers to
> floats so you'll get tons of false positives.

True positives. The fix is to use memcpy instead.

   Emitting error for
> accessing differently sized elements of enum may work (but should
> already be handled by MSan?).
>
>> I have a few questions regarding the overall design:
>> 1. Do you think this is a useful check?
>
> That's actually an interesting questions. It could be useful for tagged
> unions although I believe programmers usually surround them with
> checking asserts anyway.

Useful, yes. It will find bugs. I haven't heard anyone clamoring for it 
though.

>> 2. Where can I store type and field info about the union; some form of
>> a shadow memory or a simple array/map?
>
> Without shadow it may be unacceptably slow in union-intensive
> applications. But with shadow, it'll greatly complicate UBSan.

None of the other checks in UBSan change the ABI. You can freely link 
ubsan .o files with non-ubsan .o files and the program will still work 
and ubsan will generate no false positives.

With this check, that is not so. Uninstrumented code changing the active 
member of a union will cause a false positive when we next read it in 
instrumented code.

>> 3. Should sanitizer abort immediately or continue upon detection?
>
> AFAIK normally UBSan checks continue after error (but there's a flag to
> alter this).
>
>> 4. When/how can I remove entries from ubsan shadow memory when union's
>> lifetime ends; perhaps in a module pass or at the end of each
>> function?
>
> Take a look at how ASan does this (it's not easy).
>
> -Y
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>