r331020 - [CodeGen] Avoid destructing a callee-destructued struct type in a
Akira Hatanaka via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 26 23:57:01 PDT 2018
Author: ahatanak
Date: Thu Apr 26 23:57:00 2018
New Revision: 331020
URL: http://llvm.org/viewvc/llvm-project?rev=331020&view=rev
Log:
[CodeGen] Avoid destructing a callee-destructued struct type in a
function if a function delegates to another function.
Fix a bug introduced in r328731, which caused a struct with ObjC __weak
fields that was passed to a function to be destructed twice, once in the
callee function and once in another function the callee function
delegates to. To prevent this, keep track of the callee-destructed
structs passed to a function and disable their cleanups at the point of
the call to the delegated function.
This reapplies r331016, which was reverted in r331019 because it caused
an assertion to fail in EmitDelegateCallArg on a windows bot. I made
changes to EmitDelegateCallArg so that it doesn't try to deactivate
cleanups for structs that have trivial destructors (cleanups for those
structs are never pushed to the cleanup stack in EmitParmDecl).
rdar://problem/39194693
Differential Revision: https://reviews.llvm.org/D45382
Added:
cfe/trunk/test/CodeGenCXX/lambda-to-function-pointer-conversion.cpp
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGCleanup.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
cfe/trunk/test/CodeGenObjCXX/arc-special-member-functions.mm
cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=331020&r1=331019&r2=331020&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Apr 26 23:57:00 2018
@@ -3063,6 +3063,18 @@ void CodeGenFunction::EmitDelegateCallAr
} else {
args.add(convertTempToRValue(local, type, loc), type);
}
+
+ // Deactivate the cleanup for the callee-destructed param that was pushed.
+ if (hasAggregateEvaluationKind(type) && !CurFuncIsThunk &&
+ getContext().isParamDestroyedInCallee(type) && type.isDestructedType()) {
+ EHScopeStack::stable_iterator cleanup =
+ CalleeDestructedParamCleanups.lookup(cast<ParmVarDecl>(param));
+ assert(cleanup.isValid() &&
+ "cleanup for callee-destructed param not recorded");
+ // This unreachable is a temporary marker which will be removed later.
+ llvm::Instruction *isActive = Builder.CreateUnreachable();
+ args.addArgCleanupDeactivation(cleanup, isActive);
+ }
}
static bool isProvablyNull(llvm::Value *addr) {
Modified: cfe/trunk/lib/CodeGen/CGCleanup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.cpp?rev=331020&r1=331019&r2=331020&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCleanup.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCleanup.cpp Thu Apr 26 23:57:00 2018
@@ -1233,8 +1233,10 @@ void CodeGenFunction::DeactivateCleanupB
EHCleanupScope &Scope = cast<EHCleanupScope>(*EHStack.find(C));
assert(Scope.isActive() && "double deactivation");
- // If it's the top of the stack, just pop it.
- if (C == EHStack.stable_begin()) {
+ // If it's the top of the stack, just pop it, but do so only if it belongs
+ // to the current RunCleanupsScope.
+ if (C == EHStack.stable_begin() &&
+ CurrentCleanupScopeDepth.strictlyEncloses(C)) {
// If it's a normal cleanup, we need to pretend that the
// fallthrough is unreachable.
CGBuilderTy::InsertPoint SavedIP = Builder.saveAndClearIP();
Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=331020&r1=331019&r2=331020&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Thu Apr 26 23:57:00 2018
@@ -1962,6 +1962,8 @@ void CodeGenFunction::EmitParmDecl(const
DtorKind == QualType::DK_nontrivial_c_struct) &&
"unexpected destructor type");
pushDestroy(DtorKind, DeclPtr, Ty);
+ CalleeDestructedParamCleanups[cast<ParmVarDecl>(&D)] =
+ EHStack.stable_begin();
}
}
} else {
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=331020&r1=331019&r2=331020&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Thu Apr 26 23:57:00 2018
@@ -587,7 +587,7 @@ public:
/// \brief Enters a new scope for capturing cleanups, all of which
/// will be executed once the scope is exited.
class RunCleanupsScope {
- EHScopeStack::stable_iterator CleanupStackDepth;
+ EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupScopeDepth;
size_t LifetimeExtendedCleanupStackSize;
bool OldDidCallStackSave;
protected:
@@ -610,6 +610,8 @@ public:
CGF.LifetimeExtendedCleanupStack.size();
OldDidCallStackSave = CGF.DidCallStackSave;
CGF.DidCallStackSave = false;
+ OldCleanupScopeDepth = CGF.CurrentCleanupScopeDepth;
+ CGF.CurrentCleanupScopeDepth = CleanupStackDepth;
}
/// \brief Exit this cleanup scope, emitting any accumulated cleanups.
@@ -635,9 +637,14 @@ public:
CGF.PopCleanupBlocks(CleanupStackDepth, LifetimeExtendedCleanupStackSize,
ValuesToReload);
PerformCleanup = false;
+ CGF.CurrentCleanupScopeDepth = OldCleanupScopeDepth;
}
};
+ // Cleanup stack depth of the RunCleanupsScope that was pushed most recently.
+ EHScopeStack::stable_iterator CurrentCleanupScopeDepth =
+ EHScopeStack::stable_end();
+
class LexicalScope : public RunCleanupsScope {
SourceRange Range;
SmallVector<const LabelDecl*, 4> Labels;
@@ -1095,6 +1102,11 @@ private:
/// decls.
DeclMapTy LocalDeclMap;
+ // Keep track of the cleanups for callee-destructed parameters pushed to the
+ // cleanup stack so that they can be deactivated later.
+ llvm::DenseMap<const ParmVarDecl *, EHScopeStack::stable_iterator>
+ CalleeDestructedParamCleanups;
+
/// SizeArguments - If a ParmVarDecl had the pass_object_size attribute, this
/// will contain a mapping from said ParmVarDecl to its implicit "object_size"
/// parameter.
Added: cfe/trunk/test/CodeGenCXX/lambda-to-function-pointer-conversion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/lambda-to-function-pointer-conversion.cpp?rev=331020&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/lambda-to-function-pointer-conversion.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/lambda-to-function-pointer-conversion.cpp Thu Apr 26 23:57:00 2018
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc19.0.0 -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+// This code used to cause an assertion failure in EmitDelegateCallArg.
+
+// CHECK: define internal void @"?__invoke@<lambda_0>@?0??test@@YAXXZ at CA@UTrivial@@@Z"(
+// CHECK: call void @"??R<lambda_0>@?0??test@@YAXXZ at QEBA@UTrivial@@@Z"(
+
+// CHECK: define internal void @"??R<lambda_0>@?0??test@@YAXXZ at QEBA@UTrivial@@@Z"(
+
+struct Trivial {
+ int x;
+};
+
+void (*fnptr)(Trivial);
+
+void test() {
+ fnptr = [](Trivial a){ (void)a; };
+}
Modified: cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm?rev=331020&r1=331019&r2=331020&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm Thu Apr 26 23:57:00 2018
@@ -10,6 +10,17 @@ void test0(id x) {
// CHECK-NEXT: ret i8* [[T2]]
}
+// Check that the delegating block invoke function doesn't destruct the Weak
+// object that is passed.
+
+// CHECK-LABEL: define internal void @___Z8testWeakv_block_invoke(
+// CHECK: call void @"_ZZ8testWeakvENK3$_2clE4Weak"(
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal void @"_ZZ8testWeakvENK3$_2clE4Weak"(
+// CHECK: call void @_ZN4WeakD1Ev(
+// CHECK-NEXT: ret void
+
id test1_rv;
void test1() {
@@ -21,3 +32,12 @@ void test1() {
// CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[T1]])
// CHECK-NEXT: ret i8* [[T2]]
}
+
+struct Weak {
+ __weak id x;
+};
+
+void testWeak() {
+ extern void testWeak_helper(void (^)(Weak));
+ testWeak_helper([](Weak){});
+}
Modified: cfe/trunk/test/CodeGenObjCXX/arc-special-member-functions.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc-special-member-functions.mm?rev=331020&r1=331019&r2=331020&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/arc-special-member-functions.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/arc-special-member-functions.mm Thu Apr 26 23:57:00 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fobjc-arc -fblocks -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fobjc-arc -fblocks -triple x86_64-apple-darwin10.0.0 -fobjc-runtime-has-weak -emit-llvm -o - %s | FileCheck %s
struct ObjCMember {
id member;
@@ -12,6 +12,59 @@ struct ObjCBlockMember {
int (^bp)(int);
};
+// CHECK: %[[STRUCT_CONTAINSWEAK:.*]] = type { %[[STRUCT_WEAK:.*]] }
+// CHECK: %[[STRUCT_WEAK]] = type { i8* }
+
+// The Weak object that is passed is destructed in this constructor.
+
+// CHECK: define void @_ZN12ContainsWeakC2E4Weak(
+// CHECK: call void @_ZN4WeakC1ERKS_(
+// CHECK: call void @_ZN4WeakD1Ev(
+
+// Check that the Weak object passed to this constructor is not destructed after
+// the delegate constructor is called.
+
+// CHECK: define void @_ZN12ContainsWeakC1E4Weak(
+// CHECK: call void @_ZN12ContainsWeakC2E4Weak(
+// CHECK-NEXT: ret void
+
+struct Weak {
+ Weak(id);
+ __weak id x;
+};
+
+struct ContainsWeak {
+ ContainsWeak(Weak);
+ Weak w;
+};
+
+ContainsWeak::ContainsWeak(Weak a) : w(a) {}
+
+// The Weak object that is passed is destructed in this constructor.
+
+// CHECK: define void @_ZN4BaseC2E4Weak(
+// CHECK: call void @_ZN4WeakD1Ev(
+// CHECK: ret void
+
+// Check that the Weak object passed to this constructor is not destructed after
+// the delegate constructor is called.
+
+// CHECK: define linkonce_odr void @_ZN7DerivedCI14BaseE4Weak(
+// CHECK: call void @_ZN7DerivedCI24BaseE4Weak(
+// CHECK-NEXT: ret void
+
+struct Base {
+ Base(Weak);
+};
+
+Base::Base(Weak a) {}
+
+struct Derived : Base {
+ using Base::Base;
+};
+
+Derived d(Weak(0));
+
// CHECK-LABEL: define void @_Z42test_ObjCMember_default_construct_destructv(
void test_ObjCMember_default_construct_destruct() {
// CHECK: call void @_ZN10ObjCMemberC1Ev
@@ -111,6 +164,13 @@ void test_ObjCBlockMember_copy_assign(Ob
// CHECK-NEXT: call void @objc_release(i8* [[T7]])
// CHECK-NEXT: ret
+// Check that the Weak object passed to this constructor is not destructed after
+// the delegate constructor is called.
+
+// CHECK: define linkonce_odr void @_ZN7DerivedCI24BaseE4Weak(
+// CHECK: call void @_ZN4BaseC2E4Weak(
+// CHECK-NEXT: ret void
+
// Implicitly-generated default constructor for ObjCMember
// CHECK-LABEL: define linkonce_odr void @_ZN10ObjCMemberC2Ev
// CHECK-NOT: objc_release
Modified: cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm?rev=331020&r1=331019&r2=331020&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm Thu Apr 26 23:57:00 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s -fexceptions -std=c++11 -fblocks -fobjc-arc | FileCheck -check-prefix=ARC %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s -fexceptions -std=c++11 -fblocks -fobjc-arc -fobjc-runtime-has-weak -DWEAK_SUPPORTED | FileCheck -check-prefix=ARC %s
// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s -fexceptions -std=c++11 -fblocks | FileCheck -check-prefix=MRC %s
typedef int (^fp)();
@@ -138,5 +138,31 @@ namespace BlockInLambda {
}
@end
+// Check that the delegating invoke function doesn't destruct the Weak object
+// that is passed.
+
+// ARC-LABEL: define internal void @"_ZZN14LambdaDelegate4testEvEN3$_58__invokeENS_4WeakE"(
+// ARC: call void @"_ZZN14LambdaDelegate4testEvENK3$_5clENS_4WeakE"(
+// ARC-NEXT: ret void
+
+// ARC-LABEL: define internal void @"_ZZN14LambdaDelegate4testEvENK3$_5clENS_4WeakE"(
+// ARC: call void @_ZN14LambdaDelegate4WeakD1Ev(
+
+#ifdef WEAK_SUPPORTED
+
+namespace LambdaDelegate {
+
+struct Weak {
+ __weak id x;
+};
+
+void test() {
+ void (*p)(Weak) = [](Weak a) { };
+}
+
+};
+
+#endif
+
// ARC: attributes [[NUW]] = { noinline nounwind{{.*}} }
// MRC: attributes [[NUW]] = { noinline nounwind{{.*}} }
More information about the cfe-commits
mailing list