[llvm] r353736 - [InstCombine] Fix matchRotate bug when one operand is a ConstantExpr shift

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 11 11:26:27 PST 2019


Author: spatel
Date: Mon Feb 11 11:26:27 2019
New Revision: 353736

URL: http://llvm.org/viewvc/llvm-project?rev=353736&view=rev
Log:
[InstCombine] Fix matchRotate bug when one operand is a ConstantExpr shift

This bug seems to be harmless in release builds, but will cause an error in UBSAN
builds or an assertion failure in debug builds.

When it gets to this opcode comparison, it assumes both of the operands are BinaryOperators,
but the prior m_LogicalShift will also match a ConstantExpr. The cast<BinaryOperator> will
assert in a debug build, or reading an invalid value for BinaryOp from memory with
((BinaryOperator*)constantExpr)->getOpcode() will cause an error in a UBSAN build.

The test I added will fail without this change in debug/UBSAN builds, but not in release.

Patch by: @AndrewScheidecker (Andrew Scheidecker)

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

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    llvm/trunk/test/Transforms/InstCombine/rotate.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp?rev=353736&r1=353735&r2=353736&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Mon Feb 11 11:26:27 2019
@@ -1819,14 +1819,18 @@ static Instruction *matchRotate(Instruct
 
   // First, find an or'd pair of opposite shifts with the same shifted operand:
   // or (lshr ShVal, ShAmt0), (shl ShVal, ShAmt1)
-  Value *Or0 = Or.getOperand(0), *Or1 = Or.getOperand(1);
+  BinaryOperator *Or0, *Or1;
+  if (!match(Or.getOperand(0), m_BinOp(Or0)) ||
+      !match(Or.getOperand(1), m_BinOp(Or1)))
+    return nullptr;
+
   Value *ShVal, *ShAmt0, *ShAmt1;
   if (!match(Or0, m_OneUse(m_LogicalShift(m_Value(ShVal), m_Value(ShAmt0)))) ||
       !match(Or1, m_OneUse(m_LogicalShift(m_Specific(ShVal), m_Value(ShAmt1)))))
     return nullptr;
 
-  auto ShiftOpcode0 = cast<BinaryOperator>(Or0)->getOpcode();
-  auto ShiftOpcode1 = cast<BinaryOperator>(Or1)->getOpcode();
+  BinaryOperator::BinaryOps ShiftOpcode0 = Or0->getOpcode();
+  BinaryOperator::BinaryOps ShiftOpcode1 = Or1->getOpcode();
   if (ShiftOpcode0 == ShiftOpcode1)
     return nullptr;
 

Modified: llvm/trunk/test/Transforms/InstCombine/rotate.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/rotate.ll?rev=353736&r1=353735&r2=353736&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/rotate.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/rotate.ll Mon Feb 11 11:26:27 2019
@@ -689,3 +689,17 @@ define i24 @rotl_select_weird_type(i24 %
   ret i24 %r
 }
 
+; Test that the transform doesn't crash when there's an "or" with a ConstantExpr operand.
+
+ at external_global = external global i8
+
+define i32 @rotl_constant_expr(i32 %shamt) {
+; CHECK-LABEL: @rotl_constant_expr(
+; CHECK-NEXT:    [[SHR:%.*]] = lshr i32 ptrtoint (i8* @external_global to i32), [[SHAMT:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = or i32 [[SHR]], shl (i32 ptrtoint (i8* @external_global to i32), i32 11)
+; CHECK-NEXT:    ret i32 [[R]]
+;
+  %shr = lshr i32 ptrtoint (i8* @external_global to i32), %shamt
+  %r = or i32 %shr, shl (i32 ptrtoint (i8* @external_global to i32), i32 11)
+  ret i32 %r
+}




More information about the llvm-commits mailing list