[llvm][LSR] Fix where invariant on ScaledReg & Scale is violated

[JOE1994]

Comments attached to the ScaledReg field of struct Formula explains that, ScaledReg must be non-null when Scale is non-zero.

This fixes up a code path where this invariant is violated. Also, add an assert to ensure this invariant holds true.

Without this patch, compiler aborts with the attached test case.

Fixes #76504

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Youngsuk Kim (JOE1994)

Changes

Comments attached to the ScaledReg field of struct Formula explains that, ScaledReg must be non-null when Scale is non-zero.

This fixes up a code path where this invariant is violated. Also, add an assert to ensure this invariant holds true.

Without this patch, compiler aborts with the attached test case.

Fixes #76504

---

Full diff: https://github.com/llvm/llvm-project/pull/112576.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp (+5-2)
• (added) llvm/test/Transforms/LoopStrengthReduce/pr76504.ll (+29)

diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index 575395eda1c5bb..28bd6318c0b9d7 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -622,6 +622,8 @@ static bool containsAddRecDependentOnLoop(const SCEV *S, const Loop &L) {
 /// representation.
 /// \see Formula::BaseRegs.
 bool Formula::isCanonical(const Loop &L) const {
+  assert((Scale == 0 || ScaledReg) && "ScaledReg must be non-null if Scale is non-zero");
+
   if (!ScaledReg)
     return BaseRegs.size() <= 1;
 
@@ -3973,9 +3975,10 @@ void LSRInstance::GenerateReassociationsImpl(LSRUse &LU, unsigned LUIdx,
       F.UnfoldedOffset =
           Immediate::getFixed((uint64_t)F.UnfoldedOffset.getFixedValue() +
                               InnerSumSC->getValue()->getZExtValue());
-      if (IsScaledReg)
+      if (IsScaledReg) {
         F.ScaledReg = nullptr;
-      else
+        F.Scale = 0;
+      } else
         F.BaseRegs.erase(F.BaseRegs.begin() + Idx);
     } else if (IsScaledReg)
       F.ScaledReg = InnerSum;
diff --git a/llvm/test/Transforms/LoopStrengthReduce/pr76504.ll b/llvm/test/Transforms/LoopStrengthReduce/pr76504.ll
new file mode 100644
index 00000000000000..ca1edc61a7e4de
--- /dev/null
+++ b/llvm/test/Transforms/LoopStrengthReduce/pr76504.ll
@@ -0,0 +1,29 @@
+; Reduced from https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65323 by @RKSimon
+;
+; RUN: opt -S -passes=loop-reduce %s
+;
+; Make sure we don't trigger an assertion.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@G = external global i32
+
+define void @foo() {
+bb8:
+  br label %bb30
+
+bb30:                                             ; preds = %bb30, %bb8
+  %l0 = phi i64 [ -2222, %bb8 ], [ %r23, %bb30 ]
+  %A22 = alloca i16, align 2
+  %r23 = add nuw i64 1, %l0
+  %G7 = getelementptr i16, ptr %A22, i64 %r23
+  %B15 = urem i64 %r23, %r23
+  %G6 = getelementptr i16, ptr %G7, i64 %B15
+  %B1 = urem i64 %r23, %r23
+  %B8 = sub i64 -1, %r23
+  %B18 = sub i64 %B8, %B1
+  %G5 = getelementptr i16, ptr %G6, i64 %B18
+  store ptr %G5, ptr undef, align 8
+  br label %bb30
+}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[nikic]

LGTM

[nikic]

Please also generate check lines, even if you're just checking for a crash.

[nikic]

This needs to be in the X86 subdirectory.

[qcolombet]

Nice catch!

[qcolombet]

In the future please use filename that are more expressive.
You can still reference the bug number in the comment.