[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
This revision was automatically updated to reflect the committed changes.
Closed by commit rC331016: [CodeGen] Avoid destructing a callee-destructued
struct type in a (authored by ahatanak, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D45382
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCleanup.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
test/CodeGenObjCXX/arc-special-member-functions.mm
test/CodeGenObjCXX/lambda-expressions.mm
Index: test/CodeGenObjCXX/lambda-expressions.mm
===
--- test/CodeGenObjCXX/lambda-expressions.mm
+++ test/CodeGenObjCXX/lambda-expressions.mm
@@ -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 @@
}
@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{{.*}} }
Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -10,6 +10,17 @@
// 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 @@
// 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){});
+}
Index: test/CodeGenObjCXX/arc-special-member-functions.mm
===
--- test/CodeGenObjCXX/arc-special-member-functions.mm
+++ test/CodeGenObjCXX/arc-special-member-functions.mm
@@ -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 @@
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() {
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, LGTM. Repository: rC Clang https://reviews.llvm.org/D45382 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
ahatanak updated this revision to Diff 144231.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Rename variable to something OldCleanupScopeDepth.
Repository:
rC Clang
https://reviews.llvm.org/D45382
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCleanup.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
test/CodeGenObjCXX/arc-special-member-functions.mm
test/CodeGenObjCXX/lambda-expressions.mm
Index: test/CodeGenObjCXX/lambda-expressions.mm
===
--- test/CodeGenObjCXX/lambda-expressions.mm
+++ test/CodeGenObjCXX/lambda-expressions.mm
@@ -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 @@
}
@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{{.*}} }
Index: test/CodeGenObjCXX/arc-special-member-functions.mm
===
--- test/CodeGenObjCXX/arc-special-member-functions.mm
+++ test/CodeGenObjCXX/arc-special-member-functions.mm
@@ -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 @@
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 @@
// 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
Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -10,6 +10,17 @@
// 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 @
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCall.cpp:3069
+ if (hasAggregateEvaluationKind(type) &&
+ getContext().isParamDestroyedInCallee(type)) {
+EHScopeStack::stable_iterator cleanup =
ahatanak wrote:
> rjmccall wrote:
> > I wonder if this is something we should be taking from the CGFunctionInfo
> > instead. It does seem plausible that it could vary, e.g. according to the
> > calling convention. But maybe that's something we can handle in a separate
> > patch?
> I assume you are talking about the call to isParamDestroyedInCallee? If so,
> yes, I think we can discuss it in a separate patch. I have plans to clean up
> the way ParamDestroyedInCallee is handled in Sema and IRGen.
> I assume you are talking about the call to isParamDestroyedInCallee?
Yeah.
> I assume you are talking about the call to isParamDestroyedInCallee? If so,
> yes, I think we can discuss it in a separate patch.
Okay, great, thanks.
Repository:
rC Clang
https://reviews.llvm.org/D45382
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
ahatanak added a comment.
x.
Comment at: lib/CodeGen/CGCall.cpp:3069
+ if (hasAggregateEvaluationKind(type) &&
+ getContext().isParamDestroyedInCallee(type)) {
+EHScopeStack::stable_iterator cleanup =
rjmccall wrote:
> I wonder if this is something we should be taking from the CGFunctionInfo
> instead. It does seem plausible that it could vary, e.g. according to the
> calling convention. But maybe that's something we can handle in a separate
> patch?
I assume you are talking about the call to isParamDestroyedInCallee? If so,
yes, I think we can discuss it in a separate patch. I have plans to clean up
the way ParamDestroyedInCallee is handled in Sema and IRGen.
Repository:
rC Clang
https://reviews.llvm.org/D45382
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
rsmith added a comment. In https://reviews.llvm.org/D45382#1060452, @ahatanak wrote: > In https://reviews.llvm.org/D45382#1060163, @rjmccall wrote: > > > Hmm. I'm not actually sure *why* it's not okay to forward callee-cleanup > > arguments. Do we just not have the necessary logic to disable the cleanup > > in the caller? > > > It seems that it would be OK if there was a way to disable the cleanup in the > caller when we know that we are delegating to another constructor, possibly > by setting the DelegateCXXConstructorCall here too. But maybe there are other > reasons for not doing so. > > Perhaps Richard (who committed r274049) knows why we can't call the delegated > constructor when there are callee-cleanup arguments? I think the only reason was that we didn't have a good way to handle cleanup emission in the thunk (as @rjmccall points out, we do need parameter cleanups in the thunk, but we need to disable them at the point of the call to the inherited constructor). I cannot think of any correctness reason that prevents using a thunk for the callee-cleanup case. Repository: rC Clang https://reviews.llvm.org/D45382 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCall.cpp:3069
+ if (hasAggregateEvaluationKind(type) &&
+ getContext().isParamDestroyedInCallee(type)) {
+EHScopeStack::stable_iterator cleanup =
I wonder if this is something we should be taking from the CGFunctionInfo
instead. It does seem plausible that it could vary, e.g. according to the
calling convention. But maybe that's something we can handle in a separate
patch?
Comment at: lib/CodeGen/CodeGenFunction.h:590
class RunCleanupsScope {
-EHScopeStack::stable_iterator CleanupStackDepth;
+EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupStackDepth;
size_t LifetimeExtendedCleanupStackSize;
Please rename this variable to something like `OldCleanupScopeDepth`.
Repository:
rC Clang
https://reviews.llvm.org/D45382
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
ahatanak updated this revision to Diff 143624.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D45382
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCleanup.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
test/CodeGenObjCXX/arc-special-member-functions.mm
test/CodeGenObjCXX/lambda-expressions.mm
Index: test/CodeGenObjCXX/lambda-expressions.mm
===
--- test/CodeGenObjCXX/lambda-expressions.mm
+++ test/CodeGenObjCXX/lambda-expressions.mm
@@ -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 @@
}
@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{{.*}} }
Index: test/CodeGenObjCXX/arc-special-member-functions.mm
===
--- test/CodeGenObjCXX/arc-special-member-functions.mm
+++ test/CodeGenObjCXX/arc-special-member-functions.mm
@@ -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 @@
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 @@
// 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
Index: test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
===
--- test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
+++ test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
@@ -10,6 +10,17 @@
// 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$_2clE4W
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
rjmccall added inline comments. Herald added a reviewer: javed.absar. Comment at: lib/CodeGen/CodeGenFunction.h:847 +CurrentCleanupStackDepth = C; + } + You don't need (or want) these accessors, I think; this is just private state of the CGF object, and nobody else should be using it. Comment at: lib/CodeGen/CodeGenFunction.h:1112 + llvm::DenseMap + CalleeDestructedParamCleanups; + It's too bad that we need this DenseMap in every CGF when actually only a very specific set of thunk functions will actually use it. But I guess DenseMap is at least trivial to construct/destroy when empty, which will be the most common case. Comment at: lib/CodeGen/CodeGenFunction.h:1116 + EHScopeStack::stable_iterator CurrentCleanupStackDepth = + EHScopeStack::stable_end(); + How about `CurrentCleanupScopeDepth`? The current name makes it sound like it's the active depth of the cleanup stack. Repository: rC Clang https://reviews.llvm.org/D45382 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
ahatanak updated this revision to Diff 142792.
ahatanak added a comment.
Deactivate the cleanups for callee-destructed parameters that are being
forwarded to a delegated constructor.
I also added a new member (CurrentCleanupStackDepth) to CodeGenFunction that
tracks the stack depth of the most recent RunCleanupsScope. This is needed to
prevent popping the cleanup at the top of the stack in DeactivateCleanupBlock
if the cleanup doesn't belong to the current RunCleanupsScope. Without this,
the current RunCleanupsScope's CleanupStackDepth can point to a cleanup that
doesn't exist on the stack anymore
(CleanupStackDepth.encloses(EHStack.stable_begin()) doesn't hold true anymore),
which can cause CodeGenFunction::PopCleanupBlocks to pop all the cleanups
before it hits the bottom of the stack and crash.
Repository:
rC Clang
https://reviews.llvm.org/D45382
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCleanup.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
test/CodeGenObjCXX/arc-special-member-functions.mm
test/CodeGenObjCXX/lambda-expressions.mm
Index: test/CodeGenObjCXX/lambda-expressions.mm
===
--- test/CodeGenObjCXX/lambda-expressions.mm
+++ test/CodeGenObjCXX/lambda-expressions.mm
@@ -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 @@
}
@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{{.*}} }
Index: test/CodeGenObjCXX/arc-special-member-functions.mm
===
--- test/CodeGenObjCXX/arc-special-member-functions.mm
+++ test/CodeGenObjCXX/arc-special-member-functions.mm
@@ -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 @@
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 @@
// 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(
+// CHE
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
rjmccall added a comment. If that's the problem, then I think the right design is for CallArg to include an optional cleanup reference for a cleanup that can be deactivated at the instant of the call (we should assert that this exists for parameters that are destroyed in the callee), and then for forwarding it's just a matter of getting that cleanup from parameter emission to forwarding-argument emission. Repository: rC Clang https://reviews.llvm.org/D45382 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
ahatanak added a comment. In https://reviews.llvm.org/D45382#1060163, @rjmccall wrote: > Hmm. I'm not actually sure *why* it's not okay to forward callee-cleanup > arguments. Do we just not have the necessary logic to disable the cleanup in > the caller? It seems that it would be OK if there was a way to disable the cleanup in the caller when we know that we are delegating to another constructor, possibly by setting the DelegateCXXConstructorCall here too. But maybe there are other reasons for not doing so. Perhaps Richard (who committed r274049) knows why we can't call the delegated constructor when there are callee-cleanup arguments? Repository: rC Clang https://reviews.llvm.org/D45382 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
rjmccall added a comment. Hmm. I'm not actually sure *why* it's not okay to forward callee-cleanup arguments. Do we just not have the necessary logic to disable the cleanup in the caller? Repository: rC Clang https://reviews.llvm.org/D45382 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor
ahatanak created this revision.
ahatanak added reviewers: rjmccall, rsmith.
Herald added a subscriber: kristof.beyls.
This patch fixes a bug where a struct with an ObjC `__weak` field gets
destructed after it has already been destructed. This bug was introduced in
r328731, which made changes to the ABI that caused structs with ObjC pointer
fields to be destructed in the callee in some cases.
This happens in two cases:
1. C++11 inheriting constructors.
2. When EmitConstructorBody does complete->base constructor delegation
optimization.
I fixed the first case by making changes to canEmitDelegateCallArgs so that it
returns false when the constructor has a parameter that is destructed in the
callee.
For the second case, I made changes so that EmitParmDecl doesn't push the
destructor cleanup for the struct parameter if the function is a constructor
that is going to delegate to the base constructor. Alternatively, I think it's
possible to just disable the optimization in EmitConstructorBody if
canEmitDelegateCallArgs returns false.
rdar://problem/39194693
Repository:
rC Clang
https://reviews.llvm.org/D45382
Files:
lib/CodeGen/CGClass.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenObjCXX/arc-special-member-functions.mm
Index: test/CodeGenObjCXX/arc-special-member-functions.mm
===
--- test/CodeGenObjCXX/arc-special-member-functions.mm
+++ test/CodeGenObjCXX/arc-special-member-functions.mm
@@ -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,54 @@
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 Weak's destructor is not called 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) {}
+
+// Check that the call to the inheriting constructor is inlined.
+
+// CHECK: define internal void @__cxx_global_var_init{{.*}}()
+// CHECK: call void @_ZN4WeakC1EP11objc_object(
+// CHECK-NOT: call
+// CHECK: call void @_ZN4BaseC2E4Weak(
+// 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
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -380,6 +380,10 @@
/// should emit cleanups.
bool CurFuncIsThunk;
+ /// In C++, whether we are code generating a constructor that is calling a
+ /// delegating constructor.
+ bool DelegateCXXConstructorCall;
+
/// In ARC, whether we should autorelease the return value.
bool AutoreleaseResult;
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -67,7 +67,8 @@
CGBuilderInserterTy(this)),
CurFn(nullptr), ReturnValue(Address::invalid()),
CapturedStmtInfo(nullptr), SanOpts(CGM.getLangOpts().Sanitize),
- IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false),
+ IsSanitizerScope(false), CurFuncIsThunk(false),
+ DelegateCXXConstructorCall(false), AutoreleaseResult(false),
SawAsmBlock(false), IsOutlinedSEHHelper(false), BlockInfo(nullptr),
BlockPointer(nullptr), LambdaThisCaptureField(nullptr),
NormalCleanupDest(Address::invalid()), NextCleanupDestIndex(1),
@@ -1307,6 +1308,13 @@
if (Body && ShouldEmitLifetimeMarkers)
Bypasses.Init(Body);
+ if (const auto *Ctor = dyn_cast(CurGD.getDecl())) {
+if (CurGD.getCtorType() == Ctor_Complete &&
+IsConstructorDelegationValid(Ctor) &&
+CGM.getTarget().getCXXABI().hasConstructorVariants())
+ DelegateCXXConstructorCall = true;
+ }
+
// Emit the standard function prologue.
StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin());
Index: lib/CodeGen/CGDecl.cpp
==
