[CodeGen] Emit destructor calls to destruct non-trivial C struct objects
returned by function calls or loaded from volatile objects
rdar://problem/51867864
Differential Revision: https://reviews.llvm.org/D66094
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 8045521..a2a024a 100644
--- 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<ExprWithCleanups>(this)->getSubExpr()->isConstantInitializer(
+ Ctx, IsForRef, Culprit);
case StringLiteralClass:
case ObjCEncodeExprClass:
return true;
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index ad8ebd2..c582b9a 100644
--- 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/CGCall.h b/clang/lib/CodeGen/CGCall.h
index 28121af..509ca43a 100644
--- 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<llvm::Value *, 2, unsigned int> 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()) {}
+ 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 !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; }
+ 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/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index acc9a9e..73c522a 100644
--- 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
index 9881d28..df576de 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -249,7 +249,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.
@@ -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
index aa9c8f9..e17c1c5f 100644
--- 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
index 412fc69..6a0a848 100644
--- 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
index 7b39736..b33596e 100644
--- 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<ExprWithCleanups>(Init))
+ Init = EWC->getSubExpr();
+
if (auto *CE = dyn_cast<ConstantExpr>(Init))
Init = CE->getSubExpr();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index e3cc486..a176125 100644
--- 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
index 24e312e..365e6a2 100644
--- 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;