[PATCH] D132634: [AArch64] Add index check before lowerInterleavedStore() uses ShuffleVectorInst's mask

[Peter Rong via Phabricator via llvm-commits]

Peter updated this revision to Diff 455743.
Peter added a comment.

Use new checks to sanitize Mask. Adding new tests and fixed test failures.
Fixed a formatting error

Previous check abort if anyone index is invalid, which is too strict.
A Mask can have undef/poison values in some of the indeces and we shouldn't abort in that case.
Thus we have failed testcases.

All we need to look out for is when the whole mask is undef, thus -1.
Therefore, we changed the checks from "abort if ANY index is invalid" to "abort if ALL index is invalid"

After discussion with Fhahn, we find that poison can also crash the code like undef, thus a poison testcase is added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132634

Files:
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/test/CodeGen/AArch64/aarch64-shufflevector.ll


Index: llvm/test/CodeGen/AArch64/aarch64-shufflevector.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/AArch64/aarch64-shufflevector.ll
@@ -0,0 +1,25 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=aarch64-linux-gnu -opaque-pointers < %s | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+define void @f_undef(<8 x i64> %a, ptr %dst) {
+; CHECK-LABEL: f_undef:
+; CHECK:       // %bb.0: // %BB
+; CHECK-NEXT:    ret
+BB:
+  %S = shufflevector <8 x i64> %a, <8 x i64> %a, <16 x i32> undef
+  store <16 x i64> %S, ptr %dst, align 64
+  ret void
+}
+
+define void @f_poison(<8 x i64> %a, ptr %dst) {
+; CHECK-LABEL: f_poison:
+; CHECK:       // %bb.0: // %BB
+; CHECK-NEXT:    ret
+BB:
+  %S = shufflevector <8 x i64> %a, <8 x i64> %a, <16 x i32> poison
+  store <16 x i64> %S, ptr %dst, align 64
+  ret void
+}
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
===================================================================
--- llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -13363,6 +13363,15 @@
 
   auto Mask = SVI->getShuffleMask();
 
+  // Sanity check if all the indices are NOT in range.
+  // If mask is `undef` or `poison`, `Mask` may be a vector of -1s.
+  // If all of them are -1, OOB read will happen later.
+  if (std::none_of(Mask.begin(), Mask.end(), [VecTy](unsigned Idx) {
+        return Idx < VecTy->getNumElements() * 2;
+      })) {
+    return false;
+  }
+  
   Type *PtrTy =
       UseScalable
           ? STVTy->getElementType()->getPointerTo(SI->getPointerAddressSpace())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D132634.455743.patch
Type: text/x-patch
Size: 1767 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220825/49c56559/attachment.bin>