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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits