Author: kuba.brecka Date: Fri Apr 14 11:53:25 2017 New Revision: 300340 URL: http://llvm.org/viewvc/llvm-project?rev=300340&view=rev Log: [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 Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp cfe/trunk/lib/CodeGen/CGObjC.cpp cfe/trunk/test/CodeGenObjC/arc-blocks.m cfe/trunk/test/CodeGenObjCXX/arc-references.mm Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=300340&r1=300339&r2=300340&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGDecl.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Fri Apr 14 11:53:25 2017 @@ -1118,6 +1118,12 @@ CodeGenFunction::EmitAutoVarAlloca(const 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::EmitAutoVarCleanup 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); Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=300340&r1=300339&r2=300340&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGObjC.cpp (original) +++ cfe/trunk/lib/CodeGen/CGObjC.cpp Fri Apr 14 11:53:25 2017 @@ -1469,6 +1469,8 @@ void CodeGenFunction::EmitObjCForCollect 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::EmitObjCForCollect 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) { Modified: cfe/trunk/test/CodeGenObjC/arc-blocks.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc-blocks.m?rev=300340&r1=300339&r2=300340&view=diff ============================================================================== --- cfe/trunk/test/CodeGenObjC/arc-blocks.m (original) +++ cfe/trunk/test/CodeGenObjC/arc-blocks.m Fri Apr 14 11:53:25 2017 @@ -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 } Modified: cfe/trunk/test/CodeGenObjCXX/arc-references.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc-references.mm?rev=300340&r1=300339&r2=300340&view=diff ============================================================================== --- cfe/trunk/test/CodeGenObjCXX/arc-references.mm (original) +++ cfe/trunk/test/CodeGenObjCXX/arc-references.mm Fri Apr 14 11:53:25 2017 @@ -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 } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits