From 5e5e4e790f8758000fe08bc784d9cf6187d5e950 Mon Sep 17 00:00:00 2001
From: Kuba Mracek <mracek@apple.com>
Date: Fri, 14 Apr 2017 16:53:25 +0000
Subject: [PATCH] [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

llvm-svn: 300340
---
 clang/lib/CodeGen/CGDecl.cpp               | 13 ++++++-------
 clang/lib/CodeGen/CGObjC.cpp               |  4 ++--
 clang/test/CodeGenObjC/arc-blocks.m        |  6 +++---
 clang/test/CodeGenObjCXX/arc-references.mm |  4 ++--
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index bb876c1..0f95904 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/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/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index 929bda9..c011ab4 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/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/clang/test/CodeGenObjC/arc-blocks.m b/clang/test/CodeGenObjC/arc-blocks.m
index e33f3d3..4bb618c 100644
--- a/clang/test/CodeGenObjC/arc-blocks.m
+++ b/clang/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/clang/test/CodeGenObjCXX/arc-references.mm b/clang/test/CodeGenObjCXX/arc-references.mm
index 23e9c92..983b211 100644
--- a/clang/test/CodeGenObjCXX/arc-references.mm
+++ b/clang/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
 }
 
-- 
2.7.4