[PATCH] D83576: [BasicAA] Fix -basicaa-recphi for geps with negative offsets

[Dave Green via Phabricator via llvm-commits]

dmgreen created this revision.
dmgreen added reviewers: efriedma, hfinkel, fhahn, tobiasvk.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

As shown in D82998 <https://reviews.llvm.org/D82998>, the basic-aa-recphi option can cause miscompiles for gep's with negative constants. The option checks for recursive phi, that recurse through a contant gep. If it finds one, it performs aliasing calculations using the other phi operands with an unknown size, to specify that an unknown number of elements after the initial value are potentially accessed. This works fine expect where the constant is negative, as the size is still considered to be positive. So this patch checks to make sure that the constant is also positive.

I will not attempt to turn the option back on until after the branch is made next week, to give us lots of time to catch anything else. But this should hopefully fix the issues.


https://reviews.llvm.org/D83576

Files:
  llvm/lib/Analysis/BasicAliasAnalysis.cpp
  llvm/test/Analysis/BasicAA/recphi.ll


Index: llvm/test/Analysis/BasicAA/recphi.ll
===================================================================
--- llvm/test/Analysis/BasicAA/recphi.ll
+++ llvm/test/Analysis/BasicAA/recphi.ll
@@ -92,8 +92,8 @@
 ; CHECK:         NoAlias:      i32* %arrayidx1, i8* %0
 ; CHECK:         NoAlias:      i32* %arrayidx, i32* %arrayidx1
 ; CHECK:         MayAlias:     [10 x i32]* %tab, i32* %p.addr.05.i
-; CHECK:         NoAlias:      i32* %p.addr.05.i, i8* %0
-; CHECK:         NoAlias:      i32* %arrayidx, i32* %p.addr.05.i
+; CHECK:         MayAlias:     i32* %p.addr.05.i, i8* %0
+; CHECK:         MayAlias:     i32* %arrayidx, i32* %p.addr.05.i
 ; CHECK:         MayAlias:     i32* %arrayidx1, i32* %p.addr.05.i
 ; CHECK:         MayAlias:     [10 x i32]* %tab, i32* %incdec.ptr.i
 ; CHECK:         MayAlias:     i32* %incdec.ptr.i, i8* %0
@@ -141,17 +141,17 @@
 ; CHECK:         NoAlias:      [3 x i16]* %int_arr.10, i16** %argv.6.par
 ; CHECK:         NoAlias:      i16* %_tmp1, i16** %argv.6.par
 ; CHECK:         PartialAlias: [3 x i16]* %int_arr.10, i16* %_tmp1
-; CHECK:         NoAlias:      i16* %ls1.9.0, i16** %argv.6.par
+; CHECK:         MayAlias:     i16* %ls1.9.0, i16** %argv.6.par
 ; CHECK:         MayAlias:     [3 x i16]* %int_arr.10, i16* %ls1.9.0
 ; CHECK:         MayAlias:     i16* %_tmp1, i16* %ls1.9.0
-; CHECK:         NoAlias:      i16* %_tmp7, i16** %argv.6.par
+; CHECK:         MayAlias:     i16* %_tmp7, i16** %argv.6.par
 ; CHECK:         MayAlias:     [3 x i16]* %int_arr.10, i16* %_tmp7
 ; CHECK:         MayAlias:     i16* %_tmp1, i16* %_tmp7
 ; CHECK:         NoAlias:      i16* %_tmp7, i16* %ls1.9.0
 ; CHECK:         NoAlias:      i16* %_tmp11, i16** %argv.6.par
 ; CHECK:         PartialAlias: [3 x i16]* %int_arr.10, i16* %_tmp11
 ; CHECK:         NoAlias:      i16* %_tmp1, i16* %_tmp11
-; CHECK:         NoAlias:      i16* %_tmp11, i16* %ls1.9.0
+; CHECK:         MayAlias:     i16* %_tmp11, i16* %ls1.9.0
 ; CHECK:         MayAlias:     i16* %_tmp11, i16* %_tmp7
 ; CHECK:         Both ModRef:  Ptr: i16** %argv.6.par  <->  %_tmp16 = call i16 @call(i32 %_tmp13)
 ; CHECK:         NoModRef:  Ptr: [3 x i16]* %int_arr.10        <->  %_tmp16 = call i16 @call(i32 %_tmp13)
Index: llvm/lib/Analysis/BasicAliasAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1666,8 +1666,10 @@
           // result of this PHI node (e.g. in a loop). If this is the case, we
           // would recurse and always get a MayAlias. Handle this case specially
           // below.
-          if (PV1GEP->getPointerOperand() == PN && PV1GEP->getNumIndices() == 1 &&
-              isa<ConstantInt>(PV1GEP->idx_begin())) {
+          if (PV1GEP->getPointerOperand() == PN &&
+              PV1GEP->getNumIndices() == 1 &&
+              isa<ConstantInt>(PV1GEP->idx_begin()) &&
+              cast<ConstantInt>(PV1GEP->idx_begin())->getSExtValue() >= 0) {
             isRecursive = true;
             continue;
           }
@@ -1693,8 +1695,10 @@
           // result of this PHI node (e.g. in a loop). If this is the case, we
           // would recurse and always get a MayAlias. Handle this case specially
           // below.
-          if (PV1GEP->getPointerOperand() == PN && PV1GEP->getNumIndices() == 1 &&
-              isa<ConstantInt>(PV1GEP->idx_begin())) {
+          if (PV1GEP->getPointerOperand() == PN &&
+              PV1GEP->getNumIndices() == 1 &&
+              isa<ConstantInt>(PV1GEP->idx_begin()) &&
+              cast<ConstantInt>(PV1GEP->idx_begin())->getSExtValue() >= 0) {
             isRecursive = true;
             continue;
           }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83576.277081.patch
Type: text/x-patch
Size: 3716 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200710/c34fff14/attachment.bin>