[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, rsmith, jfb.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

Previously, we didn't mark an array's element type's destructor referenced when 
it was annotated with no_destroy. This is not correct: we still need the 
destructor if we need to do any cleanup if an element's constructor throws. 
This was leading to crashes and linker errors. The fix is just to mark the 
destructor referenced in the array case.

This leads to an inconsistency with access control: If the array element type's 
destructor is used, then we really ought check its access. However, that would 
be a source breaking change. This patch ignores access checking, which is a bit 
unfortunate, but I think its the best we can do if we're not willing to break 
source compatibility.

Fixes rdar://48462498

Thanks!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D61165

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/no_destroy.cpp
  clang/test/SemaCXX/no_destroy.cpp


Index: clang/test/SemaCXX/no_destroy.cpp
===================================================================
--- clang/test/SemaCXX/no_destroy.cpp
+++ clang/test/SemaCXX/no_destroy.cpp
@@ -6,6 +6,9 @@
   // expected-note@+2 4 {{private}}
 #endif
 private: ~SecretDestructor(); // expected-note 2 {{private}}
+#ifndef NO_DTORS
+  // expected-note@-2 3 {{private}}
+#endif
 };
 
 SecretDestructor sd1;
@@ -44,3 +47,32 @@
 
 [[clang::no_destroy(0)]] int no_args; // expected-error{{'no_destroy' attribute takes no arguments}}
 [[clang::always_destroy(0)]] int no_args2; // expected-error{{'always_destroy' attribute takes no arguments}}
+
+SecretDestructor arr[10];
+#ifndef NO_DTORS
+// expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+
+void local_arrays() {
+  static SecretDestructor arr2[10];
+#ifndef NO_DTORS
+  // expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+  thread_local SecretDestructor arr3[10];
+#ifndef NO_DTORS
+  // expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+}
+
+struct Base {
+  ~Base();
+};
+struct Derived1 {
+  Derived1(int);
+  Base b;
+};
+struct Derived2 {
+  Derived1 b;
+};
+
+[[clang::no_destroy]] Derived2 d2[] = {0, 0};
Index: clang/test/CodeGenCXX/no_destroy.cpp
===================================================================
--- clang/test/CodeGenCXX/no_destroy.cpp
+++ clang/test/CodeGenCXX/no_destroy.cpp
@@ -1,11 +1,14 @@
 // RUN: %clang_cc1 %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s
+// RUN: %clang_cc1 -fexceptions %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s --check-prefixes=CHECK,EXCEPTIONS
 
 struct NonTrivial {
   ~NonTrivial();
 };
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK-NOT: __cxa_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::no_destroy]] NonTrivial nt1;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK-NOT: _tlv_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::no_destroy]] thread_local NonTrivial nt2;
 
@@ -13,11 +16,14 @@
   ~NonTrivial2();
 };
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: __cxa_atexit{{.*}}_ZN11NonTrivial2D1Ev
 NonTrivial2 nt21;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: _tlv_atexit{{.*}}_ZN11NonTrivial2D1Ev
 thread_local NonTrivial2 nt22;
 
+// CHECK-LABEL: define void @_Z1fv
 void f() {
   // CHECK: __cxa_atexit{{.*}}_ZN11NonTrivial2D1Ev
   static NonTrivial2 nt21;
@@ -25,7 +31,21 @@
   thread_local NonTrivial2 nt22;
 }
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: __cxa_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::always_destroy]] NonTrivial nt3;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: _tlv_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::always_destroy]] thread_local NonTrivial nt4;
+
+
+struct NonTrivial3 {
+  NonTrivial3();
+  ~NonTrivial3();
+};
+[[clang::no_destroy]] NonTrivial3 arr[10];
+
+// CHECK-LABEL: define internal void @__cxx_global_var_init
+// CHECK: {{invoke|call}} void @_ZN11NonTrivial3C1Ev
+// EXCEPTIONS: call void @_ZN11NonTrivial3D1Ev
+// CHECK-NOT: call i32 @__cxa_atexit
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13108,11 +13108,16 @@
   if (ClassDecl->hasIrrelevantDestructor()) return;
   if (ClassDecl->isDependentContext()) return;
 
-  if (VD->isNoDestroy(getASTContext()))
+  bool IsNoDestroy = VD->isNoDestroy(getASTContext());
+  if (IsNoDestroy && !VD->getType()->isArrayType())
     return;
 
   CXXDestructorDecl *Destructor = LookupDestructor(ClassDecl);
   MarkFunctionReferenced(VD->getLocation(), Destructor);
+
+  if (IsNoDestroy)
+    return;
+
   CheckDestructorAccess(VD->getLocation(), Destructor,
                         PDiag(diag::err_access_dtor_var)
                         << VD->getDeclName()

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