[llvm] 2404313 - [instsimplify] Fix a miscompile with zero sized allocas

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2022-02-17T09:27:34-08:00
New Revision: 2404313d8023d2a650f4cd12f8b4e334c58f5736

URL: https://github.com/llvm/llvm-project/commit/2404313d8023d2a650f4cd12f8b4e334c58f5736
DIFF: https://github.com/llvm/llvm-project/commit/2404313d8023d2a650f4cd12f8b4e334c58f5736.diff

LOG: [instsimplify] Fix a miscompile with zero sized allocas

Remove some code which tried to handle the case of comparing two allocas where an object size could not be precisely computed.  This code had zero coverage in tree, and at least one nasty bug.

The bug comes from the fact that the code uses the size of the result pointer as a proxy for whether the alloca can be of size zero.  Since the result of an alloca is *always* a pointer type, and a pointer type can *never* be empty, this check was a nop.  As a result, we blindly consider a zero offset from two allocas to never be equal.  They can in fact be equal when one or more of the allocas is zero sized.

This is particularly ugly because instcombine contains the exact opposite rule.  If instcombine reaches the allocas first, it combines them into one (making them equal).  If instsimplify reaches the compare first, it would consider them not equal.  This creates all kinds of fun scenarios for order of optimization reaching different and contradictory conclusions.

Added: 
    

Modified: 
    llvm/lib/Analysis/InstructionSimplify.cpp
    llvm/test/Transforms/InstSimplify/compare.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 23f2e06b6e777..7d5b62d9a8048 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -2640,14 +2640,6 @@ computePointerICmp(CmpInst::Predicate Pred, Value *LHS, Value *RHS,
         return ConstantInt::get(GetCompareTy(LHS),
                                 !CmpInst::isTrueWhenEqual(Pred));
       }
-
-      // Repeat the above check but this time without depending on DataLayout
-      // or being able to compute a precise size.
-      if (!cast<PointerType>(LHS->getType())->isEmptyTy() &&
-          !cast<PointerType>(RHS->getType())->isEmptyTy() &&
-          LHSOffset.isNullValue() && RHSOffset.isNullValue())
-        return ConstantInt::get(GetCompareTy(LHS),
-                                !CmpInst::isTrueWhenEqual(Pred));
     }
 
     // If one side of the equality comparison must come from a noalias call

diff  --git a/llvm/test/Transforms/InstSimplify/compare.ll b/llvm/test/Transforms/InstSimplify/compare.ll
index b305296a49769..7daee2a8a8da5 100644
--- a/llvm/test/Transforms/InstSimplify/compare.ll
+++ b/llvm/test/Transforms/InstSimplify/compare.ll
@@ -2703,7 +2703,10 @@ define <2 x i1> @cttz_slt_bitwidth_splat(<2 x i13> %x) {
 ; FIXME: A zero sized alloca *can* be equal to another alloca
 define i1 @zero_sized_alloca1() {
 ; CHECK-LABEL: @zero_sized_alloca1(
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, i32 0, align 4
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, i32 0, align 4
+; CHECK-NEXT:    [[RES:%.*]] = icmp ne i32* [[A]], [[B]]
+; CHECK-NEXT:    ret i1 [[RES]]
 ;
   %a = alloca i32, i32 0
   %b = alloca i32, i32 0
@@ -2713,7 +2716,10 @@ define i1 @zero_sized_alloca1() {
 
 define i1 @zero_sized_alloca2() {
 ; CHECK-LABEL: @zero_sized_alloca2(
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, i32 0, align 4
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[RES:%.*]] = icmp ne i32* [[A]], [[B]]
+; CHECK-NEXT:    ret i1 [[RES]]
 ;
   %a = alloca i32, i32 0
   %b = alloca i32