Differential D25295
[ubsan] Handle undef values in the integer overflow diagnostic Authored by vsk on Oct 5 2016, 4:16 PM.
Details
The compiler may optimize away the values passed into ubsan's integer struct Undef { int I; }
int getUndefInt() { Undef U; return U.I; }
int main() { return getUndefInt() + getUndefInt(); }Improve the situation by checking for an actual overflow in the handler. Depends on D25294.
Diff Detail Event TimelineComment Actions Ping. @filcab, would you mind taking a look? In particular, I'd be interested in any alternative implementations of hasIntegerOverflow, since the current version can't handle overflows in 128-bit integers. Comment Actions You can use #if HAVE_INT128_T (and UIntMax) to get 128 bit integers if available. Why is the test a bc file instead of the plain ll file? Thank you,
Comment Actions
llvm has a bitcode backwards compatibility policy. Checking in bitcode means that people can change the textual IR without breaking this test (e.g when the typeless pointer work lands). We should also check in the textual IR used to produce the bitcode so that people have an easy time updating the test in the future.
I can reproduce this issue several ways with my system compiler. The problem is that we can't guarantee that all versions of all compilers which compile a test case in C will treat overflow UB in the same way. That's why I thought it'd be better to check in a bitcode test which unambiguously demonstrates the issue. Fwiw, here is the original program I tried: // Compile at -fsanitize=integer -O2
struct Undef {
int I;
}
int getUndefInt() {
Undef U;
return U.I; //< Read of uninitialized memory.
}
int main() { return getUndefInt() + getUndefInt(); }
Comment Actions
Comment Actions
Comment Actions I really don't like that the test is reimplementing part of the instrumentation. The original test (the one in the phab summary) works, and gets reduced to a call to the reported function on -O2 -fsanitize=undefined. It looks safe enough to use tests like that (With some comments so people understand what we're doing there if the tests break for some reason).
Comment Actions OK. Then we should see if we can do anything about llvm.sadd.with.overflow() returning different values at -O0/-O3 when its operands are %undef. I'll ask around.
Right. I have tried changing together e.g multiple calls to getUndef() in one program, but the optimizer throws it away. We may be able to play some tricks with noinline to make this work.
Right, I can try this approach again.
Comment Actions I've asked around and haven't found a way around this. Part of the nature of undef is that two uses of a single undef may assume different values: "an ‘undef‘ “variable” can arbitrarily change its value over its “live range”" (http://llvm.org/docs/LangRef.html#undefined-values).
I tried again to write a plain-C test which gets compiled down to unconditional branches into the ubsan diagnostic routines, but haven't managed to do it. Here is my latest attempt: https://ghostbin.com/paste/wy2g2. The compiler just decides not to codegen the branch-on-undef in the way I like. Do you have other suggestions for how to test this? I've taken the rest of your suggestions into account in a local patch.
Comment Actions
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
No need for \brief