[PATCH] D146208: [ASAN] Support memory checks on vp.load/store.

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1472
+    // the minimum of evl and element count as trip count.
+    if (IntptrTy->getPrimitiveSizeInBits() <
+        EVL->getType()->getPrimitiveSizeInBits())
----------------
I believe this case should be a trunc.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1478
+      EVL = IB.CreateZExt(EVL, IntptrTy);
+    End = IB.CreateBinaryIntrinsic(Intrinsic::umin, EVL, End);
+  }
----------------
I don't believe there's anything preventing AVL from being 0 here right?  If so, this code is wrong.  The routine you're calling has a (documented!) assumption that End > 0.  To reuse, you need to add the loop guard and remove that assumption.


================
Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:1637
+  if (!isa<ConstantInt>(End)) {
+    auto [BodyIP, Index] = SplitBlockAndInsertSimpleForLoop(End, InsertBefore);
 
----------------
It's this call where a dynamically zero End becomes problematic.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146208