[PATCH] D55538: [InterleavedAccessAnalysis] Fix integer overflow in insertMember.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

If it's safe to conservatively return false here, and we're convinced realistic code isn't going to require larger strides/indexes, it's fine to just reject anything that won't fit into a 32-bit integer, I guess.



================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:261
     Factor = std::abs(Stride);
     assert(Factor > 1 && "Invalid interleave factor");
 
----------------
Not part of your patch, I guess, but do we have any particular reason to expect that Stride does not equal INT_MIN?


================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:284
+    if (KeyLong > std::numeric_limits<int32_t>::max() - 2 ||
+        KeyLong < std::numeric_limits<int32_t>::min() + 2)
+      return false;
----------------
Not sure I understand the "-2" and "+2" here; is the issue that it collides with the default DenseMap tombstone otherwise?  Can we use some map that doesn't randomly reserve certain integers?  Otherwise, please add an explicit note, and check for the tombstone/empty keys separately from the overflow check.

Do we have some overflow-checked addition helper we could use instead of writing out the overflow check here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55538/new/

https://reviews.llvm.org/D55538