erik.pilkington updated this revision to Diff 197453.
erik.pilkington added a comment.
Address review comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61165/new/
https://reviews.llvm.org/D61165
Files:
clang/include/clang/Basic/AttrDocs.td
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGValue.h
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
@@ -1,11 +1,13 @@
-// RUN: %clang_cc1 -DNO_DTORS -fno-c++-static-destructors -verify %s
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -DNO_DTORS -DNO_EXCEPTIONS -fno-c++-static-destructors -verify %s
+// RUN: %clang_cc1 -DNO_EXCEPTIONS -verify %s
+// RUN: %clang_cc1 -DNO_DTORS -fexceptions -fno-c++-static-destructors -verify %s
+// RUN: %clang_cc1 -fexceptions -verify %s
struct SecretDestructor {
#ifndef NO_DTORS
// expected-note@+2 4 {{private}}
#endif
-private: ~SecretDestructor(); // expected-note 2 {{private}}
+private: ~SecretDestructor(); // expected-note + {{private}}
};
SecretDestructor sd1;
@@ -44,3 +46,36 @@
[[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];
+#if !defined(NO_DTORS) || !defined(NO_EXCEPTIONS)
+ // expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+ thread_local SecretDestructor arr3[10];
+#if !defined(NO_DTORS) || !defined(NO_EXCEPTIONS)
+ // 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;
+};
+
+void dontcrash() {
+ [[clang::no_destroy]] static Derived2 d2[] = {0, 0};
+}
+
+[[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 %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s --check-prefixes=CHECK,NO_EXCEPTIONS
+// 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,50 @@
thread_local NonTrivial2 nt22;
}
+// CHECK-LABEL: define void @_Z1gv
+void g() {
+ // CHECK-NOT: __cxa_atexit
+ [[clang::no_destroy]] static NonTrivial2 nt21;
+ // CHECK-NOT: _tlv_atexit
+ [[clang::no_destroy]] 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
+// CHECK-NOT: call void @_ZN11NonTrivial3D1Ev
+// CHECK-NOT: call i32 @__cxa_atexit
+
+void h() {
+ [[clang::no_destroy]] static NonTrivial3 slarr[10];
+}
+
+// CHECK-LABEL: define void @_Z1hv
+// CHECK: {{invoke|call}} void @_ZN11NonTrivial3C1Ev
+// EXCEPTIONS: call void @_ZN11NonTrivial3D1Ev
+// NO_EXCEPTIONS-NOT: call void @_ZN11NonTrivial3D1Ev
+// CHECK-NOT: call i32 @__cxa_atexit
+
+void i() {
+ [[clang::no_destroy]] thread_local NonTrivial3 tlarr[10];
+}
+
+// CHECK-LABEL: define void @_Z1iv
+// CHECK: {{invoke|call}} void @_ZN11NonTrivial3C1Ev
+// EXCEPTIONS: call void @_ZN11NonTrivial3D1Ev
+// NO_EXCEPTIONS-NOT: call void @_ZN11NonTrivial3D1Ev
+// CHECK-NOT: _tlv_atexit
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13113,7 +13113,13 @@
if (ClassDecl->hasIrrelevantDestructor()) return;
if (ClassDecl->isDependentContext()) return;
- if (VD->isNoDestroy(getASTContext()))
+ // If a variable is no_destroy, either due to an attribute or flag, don't emit
+ // the destructor. As a corner case, we still need the destructor if the
+ // variable is static local array and exceptions are enabled, since then we
+ // need to clean up the elements.
+ if (VD->isNoDestroy(getASTContext()) &&
+ !(VD->getType()->isArrayType() && getLangOpts().Exceptions &&
+ VD->isStaticLocal()))
return;
CXXDestructorDecl *Destructor = LookupDestructor(ClassDecl);
Index: clang/lib/CodeGen/CGValue.h
===================================================================
--- clang/lib/CodeGen/CGValue.h
+++ clang/lib/CodeGen/CGValue.h
@@ -485,6 +485,12 @@
/// them.
bool SanitizerCheckedFlag : 1;
+ /// If set to true, this slot corresponds to a static or thread local global
+ /// variable that was annotated with the no_destroy attribute. These have
+ /// slightly different semantics from no_destroy static locals, where
+ /// exceptions are recoverable.
+ bool NoDestroyGlobalFlag : 1;
+
public:
enum IsAliased_t { IsNotAliased, IsAliased };
enum IsDestructed_t { IsNotDestructed, IsDestructed };
@@ -492,6 +498,7 @@
enum Overlap_t { DoesNotOverlap, MayOverlap };
enum NeedsGCBarriers_t { DoesNotNeedGCBarriers, NeedsGCBarriers };
enum IsSanitizerChecked_t { IsNotSanitizerChecked, IsSanitizerChecked };
+ enum IsNoDestroyGlobal_t { IsNotNoDestroyGlobal, IsNoDestroyGlobal };
/// ignored - Returns an aggregate value slot indicating that the
/// aggregate value is being ignored.
@@ -510,14 +517,12 @@
/// for calling destructors on this object
/// \param needsGC - true if the slot is potentially located
/// somewhere that ObjC GC calls should be emitted for
- static AggValueSlot forAddr(Address addr,
- Qualifiers quals,
- IsDestructed_t isDestructed,
- NeedsGCBarriers_t needsGC,
- IsAliased_t isAliased,
- Overlap_t mayOverlap,
- IsZeroed_t isZeroed = IsNotZeroed,
- IsSanitizerChecked_t isChecked = IsNotSanitizerChecked) {
+ static AggValueSlot
+ forAddr(Address addr, Qualifiers quals, IsDestructed_t isDestructed,
+ NeedsGCBarriers_t needsGC, IsAliased_t isAliased,
+ Overlap_t mayOverlap, IsZeroed_t isZeroed = IsNotZeroed,
+ IsSanitizerChecked_t isChecked = IsNotSanitizerChecked,
+ IsNoDestroyGlobal_t isNoDestroyGlobal = IsNotNoDestroyGlobal) {
AggValueSlot AV;
if (addr.isValid()) {
AV.Addr = addr.getPointer();
@@ -533,18 +538,19 @@
AV.AliasedFlag = isAliased;
AV.OverlapFlag = mayOverlap;
AV.SanitizerCheckedFlag = isChecked;
+ AV.NoDestroyGlobalFlag = isNoDestroyGlobal;
return AV;
}
- static AggValueSlot forLValue(const LValue &LV,
- IsDestructed_t isDestructed,
- NeedsGCBarriers_t needsGC,
- IsAliased_t isAliased,
- Overlap_t mayOverlap,
- IsZeroed_t isZeroed = IsNotZeroed,
- IsSanitizerChecked_t isChecked = IsNotSanitizerChecked) {
+ static AggValueSlot
+ forLValue(const LValue &LV, IsDestructed_t isDestructed,
+ NeedsGCBarriers_t needsGC, IsAliased_t isAliased,
+ Overlap_t mayOverlap, IsZeroed_t isZeroed = IsNotZeroed,
+ IsSanitizerChecked_t isChecked = IsNotSanitizerChecked,
+ IsNoDestroyGlobal_t isNoDestroyGlobal = IsNotNoDestroyGlobal) {
return forAddr(LV.getAddress(), LV.getQuals(), isDestructed, needsGC,
- isAliased, mayOverlap, isZeroed, isChecked);
+ isAliased, mayOverlap, isZeroed, isChecked,
+ isNoDestroyGlobal);
}
IsDestructed_t isExternallyDestructed() const {
@@ -603,6 +609,8 @@
return SanitizerCheckedFlag;
}
+ bool isNoDestroyGlobal() const { return NoDestroyGlobalFlag; }
+
RValue asRValue() const {
if (isIgnored()) {
return RValue::getIgnored();
Index: clang/lib/CodeGen/CGExprAgg.cpp
===================================================================
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -513,7 +513,7 @@
Address endOfInit = Address::invalid();
EHScopeStack::stable_iterator cleanup;
llvm::Instruction *cleanupDominator = nullptr;
- if (CGF.needsEHCleanup(dtorKind)) {
+ if (CGF.needsEHCleanup(dtorKind) && !Dest.isNoDestroyGlobal()) {
// In principle we could tell the cleanup where we are more
// directly, but the control flow can get so varied here that it
// would actually be quite complex. Therefore we go through an
Index: clang/lib/CodeGen/CGDeclCXX.cpp
===================================================================
--- clang/lib/CodeGen/CGDeclCXX.cpp
+++ clang/lib/CodeGen/CGDeclCXX.cpp
@@ -52,13 +52,19 @@
case TEK_Complex:
CGF.EmitComplexExprIntoLValue(Init, lv, /*isInit*/ true);
return;
- case TEK_Aggregate:
- CGF.EmitAggExpr(Init, AggValueSlot::forLValue(lv,AggValueSlot::IsDestructed,
- AggValueSlot::DoesNotNeedGCBarriers,
- AggValueSlot::IsNotAliased,
- AggValueSlot::DoesNotOverlap));
+ case TEK_Aggregate: {
+ auto IsNoDestroy = !D.isStaticLocal() && D.isNoDestroy(CGF.getContext())
+ ? AggValueSlot::IsNoDestroyGlobal
+ : AggValueSlot::IsNotNoDestroyGlobal;
+ auto Slot = AggValueSlot::forLValue(
+ lv, AggValueSlot::IsDestructed, AggValueSlot::DoesNotNeedGCBarriers,
+ AggValueSlot::IsNotAliased, AggValueSlot::DoesNotOverlap,
+ AggValueSlot::IsNotZeroed, AggValueSlot::IsNotSanitizerChecked,
+ IsNoDestroy);
+ CGF.EmitAggExpr(Init, Slot);
return;
}
+ }
llvm_unreachable("bad evaluation kind");
}
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3880,6 +3880,32 @@
storage duration shouldn't have its exit-time destructor run. Annotating every
static and thread duration variable with this attribute is equivalent to
invoking clang with -fno-c++-static-destructors.
+
+If a variable is declared with this attribute, clang doesn't access check or
+generate the type's destructor. If you have a type that you only want to be
+annotated with ``no_destroy``, you can therefore declare the destructor private:
+
+.. code-block:: c++
+
+ struct only_no_destroy {
+ private:
+ ~only_no_destroy();
+ }
+
+ [[clang::no_destroy]] only_no_destroy global; // fine!
+
+This works in almost all cases, but if ``no_destroy`` is applied to a ``static``
+or ``thread_local`` local builtin array variable and exceptions are enabled, the
+destructor is still needed to perform cleanup if the construction of an element
+of the array throws. For instance:
+
+.. code-block::c++
+
+ void f() {
+ [[clang::no_destroy]]
+ static only_no_destroy array[10]; // error, only_no_destroy has a private destructor.
+
+
}];
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
