[llvm] 2958615 - [ObjC][ARC] Fix non-deterministic behavior in ProvenanceAnalysis

[Akira Hatanaka via llvm-commits]

Author: Akira Hatanaka
Date: 2022-11-08T15:05:25-08:00
New Revision: 295861514e0d1e48df2918b630dd692ac27ee0de

URL: https://github.com/llvm/llvm-project/commit/295861514e0d1e48df2918b630dd692ac27ee0de
DIFF: https://github.com/llvm/llvm-project/commit/295861514e0d1e48df2918b630dd692ac27ee0de.diff

LOG: [ObjC][ARC] Fix non-deterministic behavior in ProvenanceAnalysis

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.

Added: 
    

Modified: 
    llvm/lib/Transforms/ObjCARC/ProvenanceAnalysis.cpp
    llvm/test/Transforms/ObjCARC/related-check.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/ObjCARC/ProvenanceAnalysis.cpp b/llvm/lib/Transforms/ObjCARC/ProvenanceAnalysis.cpp
index f447c2a23f0d..845038b8b82e 100644
--- a/llvm/lib/Transforms/ObjCARC/ProvenanceAnalysis.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ProvenanceAnalysis.cpp
@@ -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))

diff  --git a/llvm/test/Transforms/ObjCARC/related-check.ll b/llvm/test/Transforms/ObjCARC/related-check.ll
index 293297d5f202..91814b9fcf1c 100644
--- a/llvm/test/Transforms/ObjCARC/related-check.ll
+++ b/llvm/test/Transforms/ObjCARC/related-check.ll
@@ -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
+ at 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)
@@ -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