Skip to content

Commit

Permalink
[ObjC][ARC] Fix non-deterministic behavior in ProvenanceAnalysis
Browse files Browse the repository at this point in the history
ProvenanceAnalysis::relatedCheck was giving different answers depending
on the order in which the pointers were passed.

Specifically, it was returning different values when A and B were both
loads and were both referring to identifiable objects, but only one was
used by a store instruction.
  • Loading branch information
ahatanaka committed Nov 8, 2022
1 parent 99fe3d2 commit 2958615
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 16 deletions.
29 changes: 13 additions & 16 deletions llvm/lib/Transforms/ObjCARC/ProvenanceAnalysis.cpp
Expand Up @@ -134,22 +134,19 @@ bool ProvenanceAnalysis::relatedCheck(const Value *A, const Value *B) {
bool BIsIdentified = IsObjCIdentifiedObject(B);

// An ObjC-Identified object can't alias a load if it is never locally stored.
if (AIsIdentified) {
// Check for an obvious escape.
if (isa<LoadInst>(B))
return IsStoredObjCPointer(A);
if (BIsIdentified) {
// Check for an obvious escape.
if (isa<LoadInst>(A))
return IsStoredObjCPointer(B);
// Both pointers are identified and escapes aren't an evident problem.
return false;
}
} else if (BIsIdentified) {
// Check for an obvious escape.
if (isa<LoadInst>(A))
return IsStoredObjCPointer(B);
}

// Check for an obvious escape.
if ((AIsIdentified && isa<LoadInst>(B) && !IsStoredObjCPointer(A)) ||
(BIsIdentified && isa<LoadInst>(A) && !IsStoredObjCPointer(B)))
return false;

if ((AIsIdentified && isa<LoadInst>(B)) ||
(BIsIdentified && isa<LoadInst>(A)))
return true;

// Both pointers are identified and escapes aren't an evident problem.
if (AIsIdentified && BIsIdentified && !isa<LoadInst>(A) && !isa<LoadInst>(B))
return false;

// Special handling for PHI and Select.
if (const PHINode *PN = dyn_cast<PHINode>(A))
Expand Down
20 changes: 20 additions & 0 deletions llvm/test/Transforms/ObjCARC/related-check.ll
Expand Up @@ -28,6 +28,7 @@
@_unnamed_cfstring_ = private global %struct.__NSConstantString_tag { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 1992, i8* getelementptr inbounds ([3 x i8], [3 x i8]* @.str.1, i32 0, i32 0), i64 2 }, section "__DATA,__cfstring", align 8 #0
@OBJC_METH_VAR_NAME_.2 = private unnamed_addr constant [18 x i8] c"stringWithFormat:\00", section "__TEXT,__objc_methname,cstring_literals", align 1
@OBJC_SELECTOR_REFERENCES_.3 = internal externally_initialized global i8* getelementptr inbounds ([18 x i8], [18 x i8]* @OBJC_METH_VAR_NAME_.2, i32 0, i32 0), section "__DATA,__objc_selrefs,literal_pointers,no_dead_strip", align 8
@global1 = external local_unnamed_addr constant i8*, align 8
@llvm.compiler.used = appending global [5 x i8*] [i8* getelementptr inbounds ([25 x i8], [25 x i8]* @OBJC_METH_VAR_NAME_, i32 0, i32 0), i8* bitcast (i8** @OBJC_SELECTOR_REFERENCES_ to i8*), i8* bitcast (%struct._class_t** @"OBJC_CLASSLIST_REFERENCES_$_" to i8*), i8* getelementptr inbounds ([18 x i8], [18 x i8]* @OBJC_METH_VAR_NAME_.2, i32 0, i32 0), i8* bitcast (i8** @OBJC_SELECTOR_REFERENCES_.3 to i8*)], section "llvm.metadata"

; Function Attrs: optsize ssp uwtable(sync)
Expand Down Expand Up @@ -121,6 +122,25 @@ if.end19: ; preds = %if.end18, %for.body
br i1 %exitcond.not, label %for.cond.cleanup.loopexit, label %for.body
}

; CHECK-LABEL: define i8* @foo() {
; CHECK-NOT: @llvm.objc
; CHECK: ret i8* %

define i8* @foo() {
%t = alloca i8*, align 8
%v4 = load i8*, i8** @global1, align 8
%v5 = tail call i8* @llvm.objc.retain(i8* %v4)
store i8* %v4, i8** %t, align 8
%v13 = load i8*, i8** bitcast (%struct._class_t** @"OBJC_CLASSLIST_REFERENCES_$_" to i8**), align 8
%call78 = call i8* @bar(i8* %v13)
call void @llvm.objc.release(i8* %v4)
ret i8* %call78
}

declare i8* @bar(i8*)

declare i8* @llvm.objc.retain(i8*)

; Function Attrs: argmemonly mustprogress nocallback nofree nosync nounwind willreturn
declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #2

Expand Down

0 comments on commit 2958615

Please sign in to comment.