[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/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;