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<CXXConstructorDecl>(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
===================================================================
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1949,7 +1949,7 @@
// Push a destructor cleanup for this parameter if the ABI requires it.
// Don't push a cleanup in a thunk for a method that will also emit a
// cleanup.
- if (!IsScalar && !CurFuncIsThunk &&
+ if (!IsScalar && !CurFuncIsThunk && !DelegateCXXConstructorCall &&
getContext().isParamDestroyedInCallee(Ty)) {
if (QualType::DestructionKind DtorKind = Ty.isDestructedType()) {
assert((DtorKind == QualType::DK_cxx_destructor ||
Index: lib/CodeGen/CGClass.cpp
===================================================================
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -819,8 +819,7 @@
// Before we go any further, try the complete->base constructor
// delegation optimization.
- if (CtorType == Ctor_Complete && IsConstructorDelegationValid(Ctor) &&
- CGM.getTarget().getCXXABI().hasConstructorVariants()) {
+ if (DelegateCXXConstructorCall) {
EmitDelegateCXXConstructorCall(Ctor, Ctor_Base, Args, Ctor->getLocEnd());
return;
}
@@ -2033,12 +2032,13 @@
if (Ctor->isVariadic())
return false;
- if (CGF.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) {
- // If the parameters are callee-cleanup, it's not safe to forward.
- for (auto *P : Ctor->parameters())
- if (P->getType().isDestructedType())
- return false;
+ // If the parameters are callee-cleanup, it's not safe to forward.
+ for (auto *P : Ctor->parameters())
+ if (CGF.getContext().isParamDestroyedInCallee(P->getType()) &&
+ P->getType().isDestructedType())
+ return false;
+ if (CGF.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) {
// Likewise if they're inalloca.
const CGFunctionInfo &Info =
CGF.CGM.getTypes().arrangeCXXConstructorCall(Args, Ctor, Type, 0, 0);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
