[PATCH] D118602: [CodeGenPrepare] Avoid out-of-bounds shift

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope created this revision.
Herald added subscribers: pengfei, hiraditya.
bjope requested review of this revision.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118602

Files:
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/test/CodeGen/X86/codegen-prepare-oob-shl.ll


Index: llvm/test/CodeGen/X86/codegen-prepare-oob-shl.ll
===================================================================
--- /dev/null
+++ 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
+}
Index: llvm/lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -4550,9 +4550,10 @@
     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
+            ? Scale = 1LL << RHS->getLimitedValue(RHS->getBitWidth() - 1)
+            : RHS->getSExtValue();
 
     return matchScaledValue(AddrInst->getOperand(0), Scale, Depth);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118602.404493.patch
Type: text/x-patch
Size: 2041 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220131/d4f822ab/attachment.bin>