[PATCH] D97665: [InstSimplify] Don't fold comparisons of non-inbounds GEPs

[LemonBoy via Phabricator via llvm-commits]

LemonBoy created this revision.
LemonBoy added reviewers: nikic, nlopes.
Herald added subscribers: dexonsmith, hiraditya.
LemonBoy requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The logic employed by the constant folder assumes the GEPs to be inbounds, when applied to other GEPs the result can be potentially incorrect and cause miscompilations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97665

Files:
  llvm/lib/IR/ConstantFold.cpp
  llvm/test/Transforms/InstSimplify/icmp.ll


Index: llvm/test/Transforms/InstSimplify/icmp.ll
===================================================================
--- llvm/test/Transforms/InstSimplify/icmp.ll
+++ llvm/test/Transforms/InstSimplify/icmp.ll
@@ -3,6 +3,8 @@
 
 target datalayout = "e-p:64:64:64-p1:16:16:16-p2:32:32:32-p3:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 
+ at g = internal unnamed_addr constant [2 x i8] c".\00", align 1
+
 declare void @usei8ptr(i8* %ptr)
 
 ; Ensure that we do not crash when looking at such a weird bitcast.
@@ -19,6 +21,15 @@
   ret i1 %cmp
 }
 
+; Ensure the comparison is not incorrectly folded into `false`.
+define i1 @compare_non_inbounds_gep_zero() {
+; CHECK-LABEL: @compare_non_inbounds_gep_zero(
+; CHECK-NEXT:    ret i1 icmp eq (i8* getelementptr ([2 x i8], [2 x i8]* @g, i64 0, i64 sub (i64 0, i64 ptrtoint ([2 x i8]* @g to i64))), i8* null)
+;
+  %cmp = icmp eq i8* getelementptr ([2 x i8], [2 x i8]* @g, i64 0, i64 sub (i64 0, i64 ptrtoint ([2 x i8]* @g to i64))), null
+  ret i1 %cmp
+}
+
 ; TODO: these should return poison
 
 define i1 @poison(i32 %x) {
Index: llvm/lib/IR/ConstantFold.cpp
===================================================================
--- llvm/lib/IR/ConstantFold.cpp
+++ llvm/lib/IR/ConstantFold.cpp
@@ -1832,6 +1832,12 @@
 
     case Instruction::GetElementPtr: {
       GEPOperator *CE1GEP = cast<GEPOperator>(CE1);
+
+      // The foldings below assume the GEP to be inbounds.
+      // TODO: Some of the logic can be applied for non-inbounds GEPs as well.
+      if (!CE1GEP->isInBounds())
+        return ICmpInst::BAD_ICMP_PREDICATE;
+
       // Ok, since this is a getelementptr, we know that the constant has a
       // pointer type.  Check the various cases.
       if (isa<ConstantPointerNull>(V2)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97665.327041.patch
Type: text/x-patch
Size: 1844 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210301/7ab55cdf/attachment.bin>