Simple plugin to detect arithmetic exceptions #122
Conversation
It's capabilities are unfortunately limited by the fact that LLVM doesn't make a difference between signed and unsigned operations in its LLVM IR. To avoid excessive false positive warnings signed overflows are only checked if the LLVM IR instructions contains the 'no sign wrap' (nsw) flag. Vector operations seem no to include this flag. Further the pointer to integer warning if the destination type is too narrow to hold the pointer value is sometimes suppressed by the fact that first a conversion from *iX to i64 is performed and only afterwards a truncation from i64 to iY (destination type).
There was a problem hiding this comment.
Finally got round to taking a look at this. Looks good in general.
It would be nice to know whether there's a good reason for the lack of nsw/nuw flags for vector instructions, or whether this is just an oversight (i.e. can we expect this to be fixed in Clang/LLVM in the future?).
The PtrToInt warning is an interesting case. Because of the way that Oclgrind formulates its virtual addresses, I imagine that any PtrToInt instruction where the integer width is less than the machine pointer width will generate a warning with this plugin. Since this is somewhat specific to the way that Oclgrind does its addressing, I wonder how useful this particular feature will be for real codes?
| void ArithmeticExceptions::logArithmeticException() const | ||
| { | ||
| Context::Message msg(WARNING, m_context); | ||
| msg << "Undefined behaviour due to an arithmetic exception" << endl |
There was a problem hiding this comment.
Can we make this a bit more specific to the particular exception that we are logging? I think it be useful for developers to be able to see the difference between, for example, a division by zero and a signed overflow.
| // Check for signed overflow | ||
| // Report an exception iff LLVM would create a poisoned value on overflow | ||
| // Other than that it is not possible to differentiate between signed/unsigned values | ||
| if(BinOp->hasNoSignedWrap()) |
There was a problem hiding this comment.
Will unsigned wrap be covered as well?
There was a problem hiding this comment.
No, the plugin will only warn about signed wrap. Unsigned wrap is well defined in OpenCL (and C99).
|
@mpflanzer, you also created a branch where you directly use the undefined sanitizer from llvm (https://github.com/mpflanzer/Oclgrind/commits/ubsan). I would like to know, why stopped this approach? Wouldn't it be better to use already available checks, which directly comes with llvm? jerryct |
It's capabilities are unfortunately limited by the fact that LLVM
doesn't make a difference between signed and unsigned operations in its
LLVM IR. To avoid excessive false positive warnings signed overflows are
only checked if the LLVM IR instructions contains the 'no sign wrap'
(nsw) flag. Vector operations seem no to include this flag.
Further the pointer to integer warning if the destination type is too
narrow to hold the pointer value is sometimes suppressed by the fact
that first a conversion from *iX to i64 is performed and only afterwards
a truncation from i64 to iY (destination type).
The some tests currently fail due to the limitations. They should be added as expected failures or could be deleted. Furthermore, not all checks are covered by the tests. More tests should be added.