[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 3 06:22:51 PDT 2018

lebedev.ri created this revision.
lebedev.ri added reviewers: vsk, rsmith, rjmccall, Sanitizers.
lebedev.ri added a project: Sanitizers.

C and C++ are interesting languages. They are statically typed, but weakly.
The implicit conversions are allowed. This is nice, allows to write code
while balancing between getting drowned in everything being convertible,
and nothing being convertible. As usual, this comes with a price:

  void consume(unsigned int val);
  void test(int val) {
    // The 'val' is `signed int`, but `consume()` takes `unsigned int`.
    // If val is negative, then consume() will be operating on a large
    // unsigned value, and you may or may not have a bug.
    // But yes, sometimes this is intentional.
    // Making the conversion explicit silences the sanitizer.
    consume((unsigned int)val);

Yes, there is a `-Wsign-conversion`` diagnostic group, but first, it is kinda
noisy, since it warns on everything (unlike sanitizers, warning on an
actual issues), and second, likely there are cases where it does **not** warn.

The actual detection is pretty easy. We just need to check each of the values
whether it is negative, and equality-compare the results of those comparisons.
The unsigned value is obviously non-negative. Zero is non-negative too.

We do not have to emit the check *always*, there are obvious situations
where we can avoid emitting it, since it would **always** get optimized-out.
But i do think the tautological IR (`icmp ult %x, 0`, which is always false)
should be emitted, and the middle-end should cleanup it.

This sanitizer is in the `-fsanitize=implicit-conversion` group,
and is a logical continuation of https://reviews.llvm.org/D48958 `-fsanitize=implicit-integer-truncation`.
As for the ordering, i'we opted to emit the check **after**
`-fsanitize=implicit-integer-truncation`. At least on these simple 16 test cases,
this results in 1 of the 12 emitted checks being optimized away,
as compared to 0 checks being optimized away if the order is reversed.

This is a clang part.
The compiler-rt part is ???.

Finishes fixing PR21530 <https://bugs.llvm.org/show_bug.cgi?id=21530>, PR37552 <https://bugs.llvm.org/show_bug.cgi?id=37552>, PR35409 <https://bugs.llvm.org/show_bug.cgi?id=35409>.
Finishes partially fixing PR9821 <https://bugs.llvm.org/show_bug.cgi?id=9821>.
Finishes fixing https://github.com/google/sanitizers/issues/940.

Only the bitfield handling is missing.

  rC Clang



-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50250.158987.patch
Type: text/x-patch
Size: 60498 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180803/c150b240/attachment-0001.bin>

More information about the cfe-commits mailing list