[ObjC] Fix lifetime markers of loop variable in EmitObjCForCollectionStmt [take 2]

CodeGenFunction::EmitObjCForCollectionStmt currently emits lifetime markers for the loop variable in an inconsistent way:  lifetime.start is emitted before the loop is entered, but lifetime.end is emitted inside the loop. AddressSanitizer uses these markers to track out-of-scope accesses to local variables, and we get false positives in Obj-C foreach loops (in the 2nd iteration of the loop). This patch keeps the loop variable alive for the whole loop by extending ForScope and registering the cleanup function inside EmitAutoVarAlloca.

Differential Revision: https://reviews.llvm.org/D32029

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@300340 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp
index bb876c1be70386f2464e0338a7378bf345097623..0f959043a22ed517ff427b7b9d9f72021a5e085d 100644 (file)
--- a/lib/CodeGen/CGDecl.cpp
+++ b/lib/CodeGen/CGDecl.cpp
@@ -1118,6 +1118,12 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
   if (D.hasAttr<AnnotateAttr>())
     EmitVarAnnotations(&D, address.getPointer());
 
+  // Make sure we call @llvm.lifetime.end.
+  if (emission.useLifetimeMarkers())
+    EHStack.pushCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker,
+                                         emission.getAllocatedAddress(),
+                                         emission.getSizeForLifetimeMarkers());
+
   return emission;
 }
 
@@ -1408,13 +1414,6 @@ void CodeGenFunction::EmitAutoVarCleanups(const AutoVarEmission &emission) {
 
   const VarDecl &D = *emission.Variable;
 
-  // Make sure we call @llvm.lifetime.end.  This needs to happen
-  // *last*, so the cleanup needs to be pushed *first*.
-  if (emission.useLifetimeMarkers())
-    EHStack.pushCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker,
-                                         emission.getAllocatedAddress(),
-                                         emission.getSizeForLifetimeMarkers());
-
   // Check the type for a cleanup.
   if (QualType::DestructionKind dtorKind = D.getType().isDestructedType())
     emitAutoVarTypeCleanup(emission, dtorKind);

diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp
index 929bda9099b9dc3327c678265dfeb2be2a7e6bab..c011ab4dfc9799e53cc59513892b26e8a83f0d61 100644 (file)
--- a/lib/CodeGen/CGObjC.cpp
+++ b/lib/CodeGen/CGObjC.cpp
@@ -1469,6 +1469,8 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){
   if (DI)
     DI->EmitLexicalBlockStart(Builder, S.getSourceRange().getBegin());
 
+  RunCleanupsScope ForScope(*this);
+
   // The local variable comes into scope immediately.
   AutoVarEmission variable = AutoVarEmission::invalid();
   if (const DeclStmt *SD = dyn_cast<DeclStmt>(S.getElement()))
@@ -1499,8 +1501,6 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){
                                       ArrayType::Normal, 0);
   Address ItemsPtr = CreateMemTemp(ItemsTy, "items.ptr");
 
-  RunCleanupsScope ForScope(*this);
-
   // Emit the collection pointer.  In ARC, we do a retain.
   llvm::Value *Collection;
   if (getLangOpts().ObjCAutoRefCount) {

diff --git a/test/CodeGenObjC/arc-blocks.m b/test/CodeGenObjC/arc-blocks.m
index e33f3d3e6d17d8e15ce5cb8f20afde35aae57686..4bb618c3a415d84722d599f0a0d7d15bf3a123f0 100644 (file)
--- a/test/CodeGenObjC/arc-blocks.m
+++ b/test/CodeGenObjC/arc-blocks.m
@@ -532,8 +532,6 @@ void test13(id x) {
   // CHECK-NEXT: [[T0:%.*]] = load void ()*, void ()** [[B]]
   // CHECK-NEXT: [[T1:%.*]] = bitcast void ()* [[T0]] to i8*
   // CHECK-NEXT: call void @objc_release(i8* [[T1]])
-  // CHECK-NEXT: [[BPTR2:%.*]] = bitcast void ()** [[B]] to i8*
-  // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[BPTR2]])
 
   // CHECK-NEXT: [[T0:%.*]] = load i1, i1* [[CLEANUP_ACTIVE]]
   // CHECK-NEXT: br i1 [[T0]]
@@ -541,7 +539,9 @@ void test13(id x) {
   // CHECK-NEXT: call void @objc_release(i8* [[T0]])
   // CHECK-NEXT: br label
 
-  // CHECK:      [[T0:%.*]] = load i8*, i8** [[X]]
+  // CHECK:      [[BPTR2:%.*]] = bitcast void ()** [[B]] to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[BPTR2]])
+  // CHECK-NEXT:      [[T0:%.*]] = load i8*, i8** [[X]]
   // CHECK-NEXT: call void @objc_release(i8* [[T0]])
   // CHECK-NEXT: ret void
 }

diff --git a/test/CodeGenObjCXX/arc-references.mm b/test/CodeGenObjCXX/arc-references.mm
index 23e9c92629c3bdd19f811d8a2eb51bdb8c078a7f..983b211dfbfae181fe48b55941874f8ab0c50fea 100644 (file)
--- a/test/CodeGenObjCXX/arc-references.mm
+++ b/test/CodeGenObjCXX/arc-references.mm
@@ -21,7 +21,7 @@ void test0() {
   // CHECK: call void @_Z6calleev
   callee();
   // CHECK: call void @objc_release
-  // CHECK-NEXT: ret
+  // CHECK: ret
 }
 
 // No lifetime extension when we're binding a reference to an lvalue.
@@ -44,9 +44,9 @@ void test3() {
   const __weak id &ref = strong_id();
   // CHECK-NEXT: call void @_Z6calleev()
   callee();
+  // CHECK-NEXT: call void @objc_destroyWeak
   // CHECK-NEXT: [[PTR:%.*]] = bitcast i8*** [[REF]] to i8*
   // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[PTR]])
-  // CHECK-NEXT: call void @objc_destroyWeak
   // CHECK-NEXT: ret void
 }