[llvm] 0352ee1 - [CodeGenPrepare] Avoid out-of-bounds shift

[Bjorn Pettersson via llvm-commits]

Author: Bjorn Pettersson
Date: 2022-02-03T21:03:58+01:00
New Revision: 0352ee1a225ae3a54f8cda9ead710498a60ff09a

URL: https://github.com/llvm/llvm-project/commit/0352ee1a225ae3a54f8cda9ead710498a60ff09a
DIFF: https://github.com/llvm/llvm-project/commit/0352ee1a225ae3a54f8cda9ead710498a60ff09a.diff

LOG: [CodeGenPrepare] Avoid out-of-bounds shift

AddressingModeMatcher::matchOperationAddr may attempt to shift a
variable by the same amount of steps as found in the IR in a SHL
instruction. This was done without considering that there could be
undefined behavior in the IR, so the shift performed when compiling
could end up having undefined behavior as well.

This patch avoid UB in the codegenprepare by making sure that we
limit the shift amount used, in a similar way as already being done
in CodeGenPrepare::optimizeLoadExt.

Differential Revision: https://reviews.llvm.org/D118602

Added: 
    llvm/test/CodeGen/X86/codegen-prepare-oob-shl.ll

Modified: 
    llvm/lib/CodeGen/CodeGenPrepare.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index c888adeafca5d..9df3411c59658 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -4550,9 +4550,9 @@ bool AddressingModeMatcher::matchOperationAddr(User *AddrInst, unsigned Opcode,
     ConstantInt *RHS = dyn_cast<ConstantInt>(AddrInst->getOperand(1));
     if (!RHS || RHS->getBitWidth() > 64)
       return false;
-    int64_t Scale = RHS->getSExtValue();
-    if (Opcode == Instruction::Shl)
-      Scale = 1LL << Scale;
+    int64_t Scale = Opcode == Instruction::Shl
+                        ? 1LL << RHS->getLimitedValue(RHS->getBitWidth() - 1)
+                        : RHS->getSExtValue();
 
     return matchScaledValue(AddrInst->getOperand(0), Scale, Depth);
   }

diff  --git a/llvm/test/CodeGen/X86/codegen-prepare-oob-shl.ll b/llvm/test/CodeGen/X86/codegen-prepare-oob-shl.ll
new file mode 100644
index 0000000000000..d37b34e06842a
--- /dev/null
+++ b/llvm/test/CodeGen/X86/codegen-prepare-oob-shl.ll
@@ -0,0 +1,22 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -mtriple i686-unknown-unknown -codegenprepare -S | FileCheck %s
+
+target datalayout = "e-p:8:8"
+
+; The shl has UB (shift count oob). This used to result in undefined behavior
+; in codegenprepare when AddressingModeMatcher::matchOperationAddr tried to
+; shift a variable by that amount during compilation. Intent with the test
+; case is to verify that this compiles without complaints even if opt is built
+; with ubsan enabled.
+define dso_local void @main(i32 %a, [3 x { i8, i8 }*]* %p) {
+; CHECK-LABEL: @main(
+; CHECK-NEXT:    [[SHL:%.*]] = shl i32 [[A:%.*]], -1229216766
+; CHECK-NEXT:    [[ARRAYIDX926:%.*]] = getelementptr inbounds [3 x { i8, i8 }*], [3 x { i8, i8 }*]* [[P:%.*]], i32 0, i32 [[SHL]]
+; CHECK-NEXT:    [[L0:%.*]] = load { i8, i8 }*, { i8, i8 }** [[ARRAYIDX926]], align 1
+; CHECK-NEXT:    ret void
+;
+  %shl = shl i32 %a, -1229216766
+  %arrayidx926 = getelementptr inbounds [3 x { i8, i8 }*], [3 x { i8, i8 }*]* %p, i32 0, i32 %shl
+  %l0 = load { i8, i8 }*, { i8, i8 }** %arrayidx926, align 1
+  ret void
+}