[PATCH] D25295: [ubsan] Handle undef values in the integer overflow diagnostic

[Filipe Cabecinhas via llvm-commits]

filcab added a comment.

In https://reviews.llvm.org/D25295#568734, @vsk wrote:

> In particular, I'd be interested in any alternative implementations of hasIntegerOverflow, since the current version can't handle overflows in 128-bit integers.


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?
Better yet: Can you get a reproducible for this bug in C/C++? (I can't get your original example to trigger the bug) Would be better, and closer to UBSan uses.

Thank you,
Filipe



================
Comment at: lib/ubsan/ubsan_handlers.cc:115
+  default:
+    return 0;
+  }
----------------
Don't make errors disappear like this. Add an `UNREACHABLE()` to the default case.


================
Comment at: lib/ubsan/ubsan_handlers.cc:123
+  if (IsSigned) {
+    SIntMax V = evaluateOperation(L.getSIntValue(), Operator, R.getSIntValue());
+    auto UpperLimit =
----------------
Won't you overflow if the values would overflow (for the case where your values are the same type as `SIntMax`)?


================
Comment at: lib/ubsan/ubsan_handlers.cc:128
+    if (V > UpperLimit || V < LowerLimit)
+      return true;
+  } else {
----------------
`return V > UpperLimit || V < LowerLimit;`
Then remove the indentation level for the `else`, make the same transformation, and remove the `return false;` at the end.


https://reviews.llvm.org/D25295