diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -3009,6 +3009,9 @@ switch (getStmtClass()) { default: break; + case Stmt::ExprWithCleanupsClass: + return cast(this)->getSubExpr()->isConstantInitializer( + Ctx, IsForRef, Culprit); case StringLiteralClass: case ObjCEncodeExprClass: return true; diff --git a/clang/lib/CodeGen/CGCall.h b/clang/lib/CodeGen/CGCall.h --- a/clang/lib/CodeGen/CGCall.h +++ b/clang/lib/CodeGen/CGCall.h @@ -358,27 +358,26 @@ /// 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 Value; - CharUnits Alignment; + Address Addr = Address::invalid(); // Return value slot flags - enum Flags { - IS_VOLATILE = 0x1, - IS_UNUSED = 0x2, - }; + unsigned IsVolatile : 1; + unsigned IsUnused : 1; + unsigned IsExternallyDestructed : 1; public: - ReturnValueSlot() {} - ReturnValueSlot(Address Addr, bool IsVolatile, bool IsUnused = false) - : Value(Addr.isValid() ? Addr.getPointer() : nullptr, - (IsVolatile ? IS_VOLATILE : 0) | (IsUnused ? IS_UNUSED : 0)), - Alignment(Addr.isValid() ? Addr.getAlignment() : CharUnits::Zero()) {} - - bool isNull() const { return !getValue().isValid(); } - - bool isVolatile() const { return Value.getInt() & IS_VOLATILE; } - Address getValue() const { return Address(Value.getPointer(), Alignment); } - bool isUnused() const { return Value.getInt() & IS_UNUSED; } + ReturnValueSlot() + : IsVolatile(false), IsUnused(false), IsExternallyDestructed(false) {} + ReturnValueSlot(Address Addr, bool IsVolatile, bool IsUnused = false, + bool IsExternallyDestructed = false) + : Addr(Addr), IsVolatile(IsVolatile), IsUnused(IsUnused), + IsExternallyDestructed(IsExternallyDestructed) {} + + bool isNull() const { return !Addr.isValid(); } + bool isVolatile() const { return IsVolatile; } + Address getValue() const { return Addr; } + bool isUnused() const { return IsUnused; } + bool isExternallyDestructed() const { return IsExternallyDestructed; } }; } // end namespace CodeGen diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -4805,6 +4805,11 @@ for (CallLifetimeEnd &LifetimeEnd : CallLifetimeEndAfterCall) LifetimeEnd.Emit(*this, /*Flags=*/{}); + if (!ReturnValue.isExternallyDestructed() && + RetTy.isDestructedType() == QualType::DK_nontrivial_c_struct) + pushDestroy(QualType::DK_nontrivial_c_struct, Ret.getAggregateAddress(), + RetTy); + return Ret; } diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -2869,7 +2869,9 @@ if (!resultType->isVoidType() && calleeFnInfo.getReturnInfo().getKind() == ABIArgInfo::Indirect && !hasScalarEvaluationKind(calleeFnInfo.getReturnType())) - returnSlot = ReturnValueSlot(ReturnValue, resultType.isVolatileQualified()); + returnSlot = + ReturnValueSlot(ReturnValue, resultType.isVolatileQualified(), + /*IsUnused=*/false, /*IsExternallyDestructed=*/true); // We don't need to separately arrange the call arguments because // the call can't be variadic anyway --- it's impossible to forward diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -249,7 +249,7 @@ const Expr *E, llvm::function_ref 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. @@ -287,10 +287,8 @@ } RValue Src = - EmitCall(ReturnValueSlot(RetAddr, Dest.isVolatile(), IsResultUnused)); - - if (RequiresDestruction) - CGF.pushDestroy(RetTy.isDestructedType(), Src.getAggregateAddress(), RetTy); + EmitCall(ReturnValueSlot(RetAddr, Dest.isVolatile(), IsResultUnused, + Dest.isExternallyDestructed())); if (!UseTemp) return; @@ -827,8 +825,19 @@ // If we're loading from a volatile type, force the destination // into existence. if (E->getSubExpr()->getType().isVolatileQualified()) { + bool Destruct = + !Dest.isExternallyDestructed() && + E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct; + if (Destruct) + Dest.setExternallyDestructed(); EnsureDest(E->getType()); - return Visit(E->getSubExpr()); + Visit(E->getSubExpr()); + + if (Destruct) + CGF.pushDestroy(QualType::DK_nontrivial_c_struct, Dest.getAddress(), + E->getType()); + + return; } LLVM_FALLTHROUGH; diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp --- a/clang/lib/CodeGen/CGExprConstant.cpp +++ b/clang/lib/CodeGen/CGExprConstant.cpp @@ -1167,9 +1167,7 @@ } llvm::Constant *VisitExprWithCleanups(ExprWithCleanups *E, QualType T) { - if (!E->cleanupsHaveSideEffects()) - return Visit(E->getSubExpr(), T); - return nullptr; + return Visit(E->getSubExpr(), T); } llvm::Constant *VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E, diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -364,7 +364,8 @@ ReturnValueSlot Slot; if (!ResultType->isVoidType() && CurFnInfo->getReturnInfo().getKind() == ABIArgInfo::Indirect) - Slot = ReturnValueSlot(ReturnValue, ResultType.isVolatileQualified()); + Slot = ReturnValueSlot(ReturnValue, ResultType.isVolatileQualified(), + /*IsUnused=*/false, /*IsExternallyDestructed=*/true); // Now emit our call. llvm::CallBase *CallOrInvoke; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -11533,6 +11533,9 @@ void Sema::checkNonTrivialCUnionInInitializer(const Expr *Init, SourceLocation Loc) { + if (auto *EWC = dyn_cast(Init)) + Init = EWC->getSubExpr(); + if (auto *CE = dyn_cast(Init)) Init = CE->getSubExpr(); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -673,6 +673,9 @@ if (E->getType().getObjCLifetime() == Qualifiers::OCL_Weak) Cleanup.setExprNeedsCleanups(true); + if (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; diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -6851,6 +6851,9 @@ VK_RValue); } + if (E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) + Cleanup.setExprNeedsCleanups(true); + if (!getLangOpts().CPlusPlus) return E; diff --git a/clang/test/CodeGenObjC/arc.m b/clang/test/CodeGenObjC/arc.m --- a/clang/test/CodeGenObjC/arc.m +++ b/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(); } diff --git a/clang/test/CodeGenObjC/strong-in-c-struct.m b/clang/test/CodeGenObjC/strong-in-c-struct.m --- a/clang/test/CodeGenObjC/strong-in-c-struct.m +++ b/clang/test/CodeGenObjC/strong-in-c-struct.m @@ -89,6 +89,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] } @@ -476,6 +483,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( @@ -520,7 +539,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 @@ -808,4 +829,62 @@ func(0); } +// 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 */