[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

(cherry picked from commit d35a454)

Conflicts:
	clang/test/CodeGenObjC/arc.m
	clang/test/CodeGenObjC/strong-in-c-struct.m

clang/lib/AST/Expr.cpp

@@ -3211,6 +3211,9 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef,
 
   switch (getStmtClass()) {
   default: break;
+  case Stmt::ExprWithCleanupsClass:
+    return cast<ExprWithCleanups>(this)->getSubExpr()->isConstantInitializer(
+        Ctx, IsForRef, Culprit);
   case StringLiteralClass:
   case ObjCEncodeExprClass:
     return true;

clang/lib/CodeGen/CGCall.cpp

@@ -4692,6 +4692,11 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
   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;
 }
 

clang/lib/CodeGen/CGCall.h

@@ -401,27 +401,26 @@ class FunctionArgList : public SmallVector<const VarDecl *, 16> {};
 /// 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()) {}
-
-  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

clang/lib/CodeGen/CGClass.cpp

@@ -2874,7 +2874,9 @@ void CodeGenFunction::EmitForwardingCallToLambda(
   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

clang/lib/CodeGen/CGExprAgg.cpp

@@ -249,7 +249,7 @@ void AggExprEmitter::withReturnValueSlot(
     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 @@ void AggExprEmitter::withReturnValueSlot(
   }
 
   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 @@ void AggExprEmitter::VisitCastExpr(CastExpr *E) {
     // 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;

clang/lib/CodeGen/CGExprConstant.cpp

@@ -1167,9 +1167,7 @@ class ConstExprEmitter :
   }
 
   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,

clang/lib/CodeGen/CGVTables.cpp

@@ -364,7 +364,8 @@ void CodeGenFunction::EmitCallAndReturnForThunk(llvm::FunctionCallee Callee,
   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;

clang/lib/Sema/SemaDecl.cpp

@@ -11380,6 +11380,9 @@ bool Sema::DeduceVariableDeclarationType(VarDecl *VDecl, bool DirectInit,
 
 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();
 

clang/lib/Sema/SemaExpr.cpp

@@ -637,6 +637,9 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
   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;

clang/lib/Sema/SemaExprCXX.cpp

@@ -6475,6 +6475,9 @@ ExprResult Sema::MaybeBindToTemporary(Expr *E) {
                                     VK_RValue);
   }
 
+  if (E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+    Cleanup.setExprNeedsCleanups(true);
+
   if (!getLangOpts().CPlusPlus)
     return E;
 

clang/test/CodeGenObjC/arc.m

@@ -1536,23 +1536,20 @@ void test70(id i) {
 
 // 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 align 8 %[[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 align 8 %[[TMP2]])
   // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP2]] to i8**
   // 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();
 }

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 @@ void test_destructor_ignored_result(void) {
   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 @@ void test_copy_assignment_StrongBlock(StrongBlock *d, StrongBlock *s) {
 
 // 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,6 +829,64 @@ void test_compound_literal2(int c, StrongSmall *p) {
   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);
+}
+
 struct ZeroBitfield {
   int : 0;
   id strong;