ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added subscribers: llvm-commits, dexonsmith, jkorous.
Herald added a project: LLVM.
This is the patch I split out of https://reviews.llvm.org/D64464.
The cleanup is pushed in `EmitCallExpr` and `EmitObjCMessageExpr` so that the
destructor is called to destruct function call and ObjC message returns. I also
added test cases for block function calls since the patch in
https://reviews.llvm.org/D64464 wasn't handling that case correctly.
rdar://problem/51867864
Repository:
rC Clang
https://reviews.llvm.org/D66094
Files:
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/Parse/ParseObjc.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/strong-in-c-struct.m
llvm/test/Bitcode/upgrade-arc-runtime-calls.bc
llvm/test/Bitcode/upgrade-mrr-runtime-calls.bc
Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===================================================================
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -90,6 +90,13 @@
void calleeStrongSmall(StrongSmall);
void func(Strong *);
+@interface C
+-(StrongSmall)getStrongSmall;
++(StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
// CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
// CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
// CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -477,6 +484,18 @@
getStrongSmall();
}
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+ [c getStrongSmall];
+}
+
// CHECK: define void @test_copy_constructor_StrongBlock(
// CHECK: call void @__copy_constructor_8_8_sb0(
// CHECK: call void @__destructor_8_sb0(
@@ -521,7 +540,9 @@
// CHECK: define void @test_copy_constructor_StrongVolatile0(
// CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
// CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
// CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -718,4 +739,62 @@
VolatileArray t = *a;
}
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+ g0 = getStrongSmall().f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+ g0 = [c getStrongSmall].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+ g0 = [C getStrongSmallClass].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+ g0 = ^{ StrongSmall s; return s; }().f1;
+ func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+ (void)*a;
+ func(0);
+}
+
#endif /* USESTRUCT */
Index: clang/test/CodeGenObjC/arc.m
===================================================================
--- clang/test/CodeGenObjC/arc.m
+++ clang/test/CodeGenObjC/arc.m
@@ -1536,12 +1536,13 @@
// CHECK-LABEL: define void @test71
void test71(void) {
- // FIXME: It would be nice if the __destructor_8_s40 for the first call (and
- // the following lifetime.end) came before the second call.
- //
// CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1:[^ ]+]] to i8*
// CHECK: call void @llvm.lifetime.start.p0i8({{[^,]+}}, i8* %[[T]])
// CHECK: call void @getAggDtor(%struct.AggDtor* sret %[[TMP1]])
+ // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1]] to i8**
+ // CHECK: call void @__destructor_8_s40(i8** %[[T]])
+ // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1:[^ ]+]] to i8*
+ // CHECK: call void @llvm.lifetime.end.p0i8({{[^,]+}}, i8* %[[T]])
// CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP2:[^ ]+]] to i8*
// CHECK: call void @llvm.lifetime.start.p0i8({{[^,]+}}, i8* %[[T]])
// CHECK: call void @getAggDtor(%struct.AggDtor* sret %[[TMP2]])
@@ -1549,10 +1550,6 @@
// CHECK: call void @__destructor_8_s40(i8** %[[T]])
// CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP2:[^ ]+]] to i8*
// CHECK: call void @llvm.lifetime.end.p0i8({{[^,]+}}, i8* %[[T]])
- // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1]] to i8**
- // CHECK: call void @__destructor_8_s40(i8** %[[T]])
- // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1:[^ ]+]] to i8*
- // CHECK: call void @llvm.lifetime.end.p0i8({{[^,]+}}, i8* %[[T]])
getAggDtor();
getAggDtor();
}
Index: clang/lib/Sema/SemaExprObjC.cpp
===================================================================
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3238,6 +3238,12 @@
RBracLoc, Args);
}
+void Sema::ActOnFinishObjCMessageSend(ExprResult Result) {
+ if (Result.get()->getType().isDestructedType() ==
+ QualType::DK_nontrivial_c_struct)
+ Cleanup.setExprNeedsCleanups(true);
+}
+
enum ARCConversionTypeClass {
/// int, void, struct A
ACTC_none,
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -635,6 +635,10 @@
if (E->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
Cleanup.setExprNeedsCleanups(true);
+ if (E->getType().isVolatileQualified() &&
+ E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+ Cleanup.setExprNeedsCleanups(true);
+
// C++ [conv.lval]p3:
// If T is cv std::nullptr_t, the result is a null pointer constant.
CastKind CK = T->isNullPtrType() ? CK_NullToPointer : CK_LValueToRValue;
@@ -5569,6 +5573,10 @@
}
}
+ if (Call.get()->getType().isDestructedType() ==
+ QualType::DK_nontrivial_c_struct)
+ Cleanup.setExprNeedsCleanups(true);
+
return Call;
}
Index: clang/lib/Parse/ParseObjc.cpp
===================================================================
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -3298,14 +3298,19 @@
}
Selector Sel = PP.getSelectorTable().getSelector(nKeys, &KeyIdents[0]);
+ ExprResult Result;
if (SuperLoc.isValid())
- return Actions.ActOnSuperMessage(getCurScope(), SuperLoc, Sel,
- LBracLoc, KeyLocs, RBracLoc, KeyExprs);
+ Result = Actions.ActOnSuperMessage(getCurScope(), SuperLoc, Sel,
+ LBracLoc, KeyLocs, RBracLoc, KeyExprs);
else if (ReceiverType)
- return Actions.ActOnClassMessage(getCurScope(), ReceiverType, Sel,
- LBracLoc, KeyLocs, RBracLoc, KeyExprs);
- return Actions.ActOnInstanceMessage(getCurScope(), ReceiverExpr, Sel,
- LBracLoc, KeyLocs, RBracLoc, KeyExprs);
+ Result = Actions.ActOnClassMessage(getCurScope(), ReceiverType, Sel,
+ LBracLoc, KeyLocs, RBracLoc, KeyExprs);
+ else
+ Result = Actions.ActOnInstanceMessage(getCurScope(), ReceiverExpr, Sel,
+ LBracLoc, KeyLocs, RBracLoc,
+ KeyExprs);
+ Actions.ActOnFinishObjCMessageSend(Result);
+ return Result;
}
ExprResult Parser::ParseObjCStringLiteral(SourceLocation AtLoc) {
Index: clang/lib/CodeGen/CGObjC.cpp
===================================================================
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -623,6 +623,14 @@
}
}
+ if (Return.requiresDestruction()) {
+ assert(E->getType().isDestructedType() ==
+ QualType::DK_nontrivial_c_struct &&
+ "non-trivial C struct expected");
+ pushDestroy(QualType::DK_nontrivial_c_struct, result.getAggregateAddress(),
+ E->getType());
+ }
+
// For delegate init calls in ARC, implicitly store the result of
// the call back into self. This takes ownership of the value.
if (isDelegateInit) {
Index: clang/lib/CodeGen/CGExprAgg.cpp
===================================================================
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -245,7 +245,7 @@
const Expr *E, llvm::function_ref<RValue(ReturnValueSlot)> EmitCall) {
QualType RetTy = E->getType();
bool RequiresDestruction =
- Dest.isIgnored() &&
+ !Dest.isExternallyDestructed() &&
RetTy.isDestructedType() == QualType::DK_nontrivial_c_struct;
// If it makes no observable difference, save a memcpy + temporary.
@@ -282,11 +282,8 @@
}
}
- RValue Src =
- EmitCall(ReturnValueSlot(RetAddr, Dest.isVolatile(), IsResultUnused));
-
- if (RequiresDestruction)
- CGF.pushDestroy(RetTy.isDestructedType(), Src.getAggregateAddress(), RetTy);
+ RValue Src = EmitCall(ReturnValueSlot(RetAddr, Dest.isVolatile(),
+ IsResultUnused, RequiresDestruction));
if (!UseTemp)
return;
@@ -811,7 +808,16 @@
// into existence.
if (E->getSubExpr()->getType().isVolatileQualified()) {
EnsureDest(E->getType());
- return Visit(E->getSubExpr());
+ Visit(E->getSubExpr());
+
+ if (!Dest.isExternallyDestructed() &&
+ E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) {
+ CGF.pushDestroy(QualType::DK_nontrivial_c_struct, Dest.getAddress(),
+ E->getType());
+ Dest.setExternallyDestructed();
+ }
+
+ return;
}
LLVM_FALLTHROUGH;
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4487,35 +4487,48 @@
// Expression Emission
//===--------------------------------------------------------------------===//
-RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
- ReturnValueSlot ReturnValue) {
+static RValue emitCallExpr(const CallExpr *E, ReturnValueSlot ReturnValue,
+ CodeGenFunction &CGF) {
// Builtins never have block type.
if (E->getCallee()->getType()->isBlockPointerType())
- return EmitBlockCallExpr(E, ReturnValue);
+ return CGF.EmitBlockCallExpr(E, ReturnValue);
if (const auto *CE = dyn_cast<CXXMemberCallExpr>(E))
- return EmitCXXMemberCallExpr(CE, ReturnValue);
+ return CGF.EmitCXXMemberCallExpr(CE, ReturnValue);
if (const auto *CE = dyn_cast<CUDAKernelCallExpr>(E))
- return EmitCUDAKernelCallExpr(CE, ReturnValue);
+ return CGF.EmitCUDAKernelCallExpr(CE, ReturnValue);
if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(E))
if (const CXXMethodDecl *MD =
dyn_cast_or_null<CXXMethodDecl>(CE->getCalleeDecl()))
- return EmitCXXOperatorMemberCallExpr(CE, MD, ReturnValue);
+ return CGF.EmitCXXOperatorMemberCallExpr(CE, MD, ReturnValue);
- CGCallee callee = EmitCallee(E->getCallee());
+ CGCallee callee = CGF.EmitCallee(E->getCallee());
- if (callee.isBuiltin()) {
- return EmitBuiltinExpr(callee.getBuiltinDecl(), callee.getBuiltinID(),
- E, ReturnValue);
- }
+ if (callee.isBuiltin())
+ return CGF.EmitBuiltinExpr(callee.getBuiltinDecl(), callee.getBuiltinID(),
+ E, ReturnValue);
+
+ if (callee.isPseudoDestructor())
+ return CGF.EmitCXXPseudoDestructorExpr(callee.getPseudoDestructorExpr());
+
+ return CGF.EmitCall(E->getCallee()->getType(), callee, E, ReturnValue);
+}
+
+RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
+ ReturnValueSlot ReturnValue) {
+ RValue R = emitCallExpr(E, ReturnValue, *this);
- if (callee.isPseudoDestructor()) {
- return EmitCXXPseudoDestructorExpr(callee.getPseudoDestructorExpr());
+ if (ReturnValue.requiresDestruction()) {
+ assert(E->getType().isDestructedType() ==
+ QualType::DK_nontrivial_c_struct &&
+ "non-trivial C struct expected");
+ pushDestroy(QualType::DK_nontrivial_c_struct, R.getAggregateAddress(),
+ E->getType());
}
- return EmitCall(E->getCallee()->getType(), callee, E, ReturnValue);
+ return R;
}
/// Emit a CallExpr without considering whether it might be a subclass.
@@ -4637,7 +4650,10 @@
}
LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E) {
- RValue RV = EmitCallExpr(E);
+ bool RequiresDestruction =
+ E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct;
+ RValue RV = EmitCallExpr(E, ReturnValueSlot(Address::invalid(), false, false,
+ RequiresDestruction));
if (!RV.isScalar())
return MakeAddrLValue(RV.getAggregateAddress(), E->getType(),
@@ -4688,7 +4704,11 @@
}
LValue CodeGenFunction::EmitObjCMessageExprLValue(const ObjCMessageExpr *E) {
- RValue RV = EmitObjCMessageExpr(E);
+ bool RequiresDestruction =
+ E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct;
+ RValue RV =
+ EmitObjCMessageExpr(E, ReturnValueSlot(Address::invalid(), false, false,
+ RequiresDestruction));
if (!RV.isScalar())
return MakeAddrLValue(RV.getAggregateAddress(), E->getType(),
Index: clang/lib/CodeGen/CGCall.h
===================================================================
--- clang/lib/CodeGen/CGCall.h
+++ clang/lib/CodeGen/CGCall.h
@@ -361,20 +361,23 @@
/// ReturnValueSlot - Contains the address where the return value of a
/// function can be stored, and whether the address is volatile or not.
class ReturnValueSlot {
- llvm::PointerIntPair<llvm::Value *, 2, unsigned int> Value;
+ llvm::PointerIntPair<llvm::Value *, 3, unsigned int> Value;
CharUnits Alignment;
// Return value slot flags
enum Flags {
IS_VOLATILE = 0x1,
IS_UNUSED = 0x2,
+ REQUIRES_DESTRUCTION = 0x4,
};
public:
ReturnValueSlot() {}
- ReturnValueSlot(Address Addr, bool IsVolatile, bool IsUnused = false)
+ ReturnValueSlot(Address Addr, bool IsVolatile, bool IsUnused = false,
+ bool RequiresDestruction = false)
: Value(Addr.isValid() ? Addr.getPointer() : nullptr,
- (IsVolatile ? IS_VOLATILE : 0) | (IsUnused ? IS_UNUSED : 0)),
+ (IsVolatile ? IS_VOLATILE : 0) | (IsUnused ? IS_UNUSED : 0) |
+ (RequiresDestruction ? REQUIRES_DESTRUCTION : 0)),
Alignment(Addr.isValid() ? Addr.getAlignment() : CharUnits::Zero()) {}
bool isNull() const { return !getValue().isValid(); }
@@ -382,6 +385,9 @@
bool isVolatile() const { return Value.getInt() & IS_VOLATILE; }
Address getValue() const { return Address(Value.getPointer(), Alignment); }
bool isUnused() const { return Value.getInt() & IS_UNUSED; }
+ bool requiresDestruction() const {
+ return Value.getInt() & REQUIRES_DESTRUCTION;
+ }
};
} // end namespace CodeGen
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -8576,6 +8576,8 @@
SourceLocation RBracLoc,
MultiExprArg Args);
+ void ActOnFinishObjCMessageSend(ExprResult Result);
+
ExprResult BuildObjCBridgedCast(SourceLocation LParenLoc,
ObjCBridgeCastKind Kind,
SourceLocation BridgeKeywordLoc,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
