[llvm] 412a010 - [Attributor][FIX] Do not derive nonnull and dereferenceable w/o access
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 3 08:44:48 PST 2020
Author: Johannes Doerfert
Date: 2020-01-03T10:43:40-06:00
New Revision: 412a0101a99e5330d5bbe4954cd421af81d0dc64
URL: https://github.com/llvm/llvm-project/commit/412a0101a99e5330d5bbe4954cd421af81d0dc64
DIFF: https://github.com/llvm/llvm-project/commit/412a0101a99e5330d5bbe4954cd421af81d0dc64.diff
LOG: [Attributor][FIX] Do not derive nonnull and dereferenceable w/o access
An inbounds GEP results in poison if the value is not "inbounds", not in
UB. We accidentally derived nonnull and dereferenceable from these
inbounds GEPs even in the absence of accesses that would make the poison
to UB.
Added:
Modified:
llvm/lib/Transforms/IPO/Attributor.cpp
llvm/test/Transforms/Attributor/dereferenceable-1.ll
llvm/test/Transforms/Attributor/nocapture-1.ll
llvm/test/Transforms/Attributor/nonnull.ll
llvm/test/Transforms/Attributor/nosync.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 5f9dccad0600..2655930aec57 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -1792,20 +1792,6 @@ static int64_t getKnownNonNullAndDerefBytesForUse(
return std::max(int64_t(0), DerefBytes);
}
}
- if (const Value *Base =
- GetPointerBaseWithConstantOffset(UseV, Offset, DL,
- /*AllowNonInbounds*/ false)) {
- if (Base == &AssociatedValue) {
- // As long as we only use known information there is no need to track
- // dependences here.
- auto &DerefAA = A.getAAFor<AADereferenceable>(
- QueryingAA, IRPosition::value(*Base), /* TrackDependence */ false);
- IsNonNull |= (!NullPointerIsDefined && DerefAA.isKnownNonNull());
- IsNonNull |= (!NullPointerIsDefined && (Offset != 0));
- int64_t DerefBytes = DerefAA.getKnownDereferenceableBytes();
- return std::max(int64_t(0), DerefBytes - std::max(int64_t(0), Offset));
- }
- }
return 0;
}
diff --git a/llvm/test/Transforms/Attributor/dereferenceable-1.ll b/llvm/test/Transforms/Attributor/dereferenceable-1.ll
index 951b5047747f..718f5e9b9504 100644
--- a/llvm/test/Transforms/Attributor/dereferenceable-1.ll
+++ b/llvm/test/Transforms/Attributor/dereferenceable-1.ll
@@ -30,7 +30,7 @@ define i32* @test3_1(i32* dereferenceable(8) %0) local_unnamed_addr {
}
define i32* @test3_2(i32* dereferenceable_or_null(32) %0) local_unnamed_addr {
-; ATTRIBUTOR: define nonnull dereferenceable(16) i32* @test3_2(i32* nofree nonnull readnone dereferenceable(32) "no-capture-maybe-returned" %0)
+; ATTRIBUTOR: define nonnull dereferenceable(16) i32* @test3_2(i32* nofree readnone dereferenceable_or_null(32) "no-capture-maybe-returned" %0)
%ret = getelementptr inbounds i32, i32* %0, i64 4
ret i32* %ret
}
diff --git a/llvm/test/Transforms/Attributor/nocapture-1.ll b/llvm/test/Transforms/Attributor/nocapture-1.ll
index abb148d883ed..34bdb1c83f5a 100644
--- a/llvm/test/Transforms/Attributor/nocapture-1.ll
+++ b/llvm/test/Transforms/Attributor/nocapture-1.ll
@@ -288,7 +288,7 @@ define i1 @captureICmpRev(i32* %x) {
ret i1 %1
}
-; ATTRIBUTOR: define i1 @nocaptureInboundsGEPICmp(i32* nocapture nofree nonnull readnone %x)
+; ATTRIBUTOR: define i1 @nocaptureInboundsGEPICmp(i32* nocapture nofree readnone %x)
define i1 @nocaptureInboundsGEPICmp(i32* %x) {
%1 = getelementptr inbounds i32, i32* %x, i32 5
%2 = bitcast i32* %1 to i8*
@@ -296,7 +296,7 @@ define i1 @nocaptureInboundsGEPICmp(i32* %x) {
ret i1 %3
}
-; ATTRIBUTOR: define i1 @nocaptureInboundsGEPICmpRev(i32* nocapture nofree nonnull readnone %x)
+; ATTRIBUTOR: define i1 @nocaptureInboundsGEPICmpRev(i32* nocapture nofree readnone %x)
define i1 @nocaptureInboundsGEPICmpRev(i32* %x) {
%1 = getelementptr inbounds i32, i32* %x, i32 5
%2 = bitcast i32* %1 to i8*
diff --git a/llvm/test/Transforms/Attributor/nonnull.ll b/llvm/test/Transforms/Attributor/nonnull.ll
index e9d2b530a5e1..519d48bd7579 100644
--- a/llvm/test/Transforms/Attributor/nonnull.ll
+++ b/llvm/test/Transforms/Attributor/nonnull.ll
@@ -817,5 +817,24 @@ define void @PR43833_simple(i32* %0, i32 %1) {
br i1 %11, label %7, label %8
}
+declare i8* @strrchr(i8* %0, i32 %1) nofree nounwind readonly
+
+; We should not mark the return of @strrchr as `nonnull`, it may well be NULL!
+define i8* @mybasename(i8* nofree readonly %str) {
+; ATTRIBUTOR-LABEL: define {{[^@]+}}@mybasename
+; ATTRIBUTOR-SAME: (i8* nofree readonly [[STR:%.*]])
+; ATTRIBUTOR-NEXT: [[CALL:%.*]] = call i8* @strrchr(i8* nofree readonly [[STR]], i32 47)
+; ATTRIBUTOR-NEXT: [[TOBOOL:%.*]] = icmp ne i8* [[CALL]], null
+; ATTRIBUTOR-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds i8, i8* [[CALL]], i64 1
+; ATTRIBUTOR-NEXT: [[COND:%.*]] = select i1 [[TOBOOL]], i8* [[ADD_PTR]], i8* [[STR]]
+; ATTRIBUTOR-NEXT: ret i8* [[COND]]
+;
+ %call = call i8* @strrchr(i8* %str, i32 47)
+ %tobool = icmp ne i8* %call, null
+ %add.ptr = getelementptr inbounds i8, i8* %call, i64 1
+ %cond = select i1 %tobool, i8* %add.ptr, i8* %str
+ ret i8* %cond
+}
+
attributes #0 = { "null-pointer-is-valid"="true" }
attributes #1 = { nounwind willreturn}
diff --git a/llvm/test/Transforms/Attributor/nosync.ll b/llvm/test/Transforms/Attributor/nosync.ll
index cd01a0caa58d..67125fce403e 100644
--- a/llvm/test/Transforms/Attributor/nosync.ll
+++ b/llvm/test/Transforms/Attributor/nosync.ll
@@ -25,7 +25,7 @@ target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
%struct.ST = type { i32, double, %struct.RT }
; ATTRIBUTOR: Function Attrs: nofree nosync nounwind optsize readnone ssp uwtable
-; ATTRIBUTOR-NEXT: define nonnull i32* @foo(%struct.ST* nofree nonnull readnone "no-capture-maybe-returned" %s)
+; ATTRIBUTOR-NEXT: define nonnull i32* @foo(%struct.ST* nofree readnone "no-capture-maybe-returned" %s)
define i32* @foo(%struct.ST* %s) nounwind uwtable readnone optsize ssp {
entry:
%arrayidx = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 1, i32 2, i32 1, i64 5, i64 13
More information about the llvm-commits
mailing list