[PATCH] D64464: [CodeGen] Emit destructor calls for non-trivial C structs

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added subscribers: dexonsmith, jkorous.

This patch fixes a bug in IRGen where it wasn't calling the destructors for 
non-trivial C structs in the following cases:

- member access of structs that are returned from a function.
- compound literal.
- load from volatile types.

rdar://problem/51867864


Repository:
  rC Clang

https://reviews.llvm.org/D64464

Files:
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  test/CodeGenObjC/strong-in-c-struct.m


Index: test/CodeGenObjC/strong-in-c-struct.m
===================================================================
--- test/CodeGenObjC/strong-in-c-struct.m
+++ test/CodeGenObjC/strong-in-c-struct.m
@@ -521,7 +521,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 +720,42 @@
   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]])
+
+void test_member_access(void) {
+  id t = getStrongSmall().f1;
+}
+
+// CHECK: define void @test_compound_literal(
+// CHECK: %[[_COMPOUNDLITERAL:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V0:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[_COMPOUNDLITERAL]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V0]])
+
+void test_compound_literal(void) {
+  StrongSmall *p = &(StrongSmall){ 1, 0 };
+}
+
+// CHECK: define void @test_compound_literal2(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V0:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V0]])
+
+void test_compound_literal2(void) {
+  (void)(StrongSmall){ 1, 0 };
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %{{.*}})
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+  (void)*a;
+}
+
 #endif /* USESTRUCT */
Index: lib/CodeGen/CGExprAgg.cpp
===================================================================
--- lib/CodeGen/CGExprAgg.cpp
+++ 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.
@@ -656,6 +656,16 @@
   }
 
   AggValueSlot Slot = EnsureSlot(E->getType());
+
+  // Push a destructor cleanup if this is a non-trivial C struct. A compound
+  // literal object that doesn't have a file scope has automatic
+  // storage duration.
+  if (!Dest.isExternallyDestructed() &&
+      E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) {
+    CGF.pushDestroy(QualType::DK_nontrivial_c_struct, Slot.getAddress(),
+                    E->getType());
+    Dest.setExternallyDestructed();
+  }
   CGF.EmitAggExpr(E->getInitializer(), Slot);
 }
 
@@ -792,6 +802,12 @@
     // into existence.
     if (E->getSubExpr()->getType().isVolatileQualified()) {
       EnsureDest(E->getType());
+      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 Visit(E->getSubExpr());
     }
 
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -4093,6 +4093,12 @@
   EmitAnyExprToMem(InitExpr, DeclPtr, E->getType().getQualifiers(),
                    /*Init*/ true);
 
+  // Push a destructor cleanup if this is a non-trivial C struct. A compound
+  // literal object that doesn't have a file scope has automatic
+  // storage duration.
+  if (E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+    pushDestroy(QualType::DK_nontrivial_c_struct, DeclPtr, E->getType());
+
   return Result;
 }
 
@@ -4574,6 +4580,10 @@
 LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E) {
   RValue RV = EmitCallExpr(E);
 
+  if (E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+    pushDestroy(QualType::DK_nontrivial_c_struct, RV.getAggregateAddress(),
+                E->getType());
+
   if (!RV.isScalar())
     return MakeAddrLValue(RV.getAggregateAddress(), E->getType(),
                           AlignmentSource::Decl);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits