[PATCH] D50902: [compiler-rt][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/ubsan/ubsan_handlers.cc:468
+  case ICCK_IntegerTruncation: { // Legacy, no longer used.
+    // Let's figure out what it should be as per the new types, and upgrade.
+    // If both types are unsigned, then it's an unsigned truncation.
----------------
filcab wrote:
> If we can unambiguously know which type of error we should emit, why not have the clang change, but always emit the ICCK_IntegerTruncation check type? Then we'd only get this small chunk of code added to compiler-rt.
I think it shifts/duplicates the responsibilities.
At the time of the `call void @__ubsan_handle_implicit_conversion` we already *know* the kind.
If we pass some generic kind, we will then have to re-figure it out here in compiler-rt.
And both the places will need to be kept in sync.
And double the attention needs to be paid to the tests.
I'd honestly better emit `ErrorType::GenericUB` in place of `ICCK_IntegerTruncation` instead of not passing the right `kind`..


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D50902