This revision was automatically updated to reflect the committed changes. Closed by commit rL346628: [CodeGen][CXX]: Fix no_destroy CG bug under specific circumstances (authored by kristina, committed by ). Herald added a subscriber: llvm-commits.
Changed prior to commit: https://reviews.llvm.org/D54344?vs=173602&id=173605#toc Repository: rL LLVM https://reviews.llvm.org/D54344 Files: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp Index: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp =================================================================== --- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp +++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp @@ -64,6 +64,15 @@ /// static storage duration. static void EmitDeclDestroy(CodeGenFunction &CGF, const VarDecl &D, ConstantAddress Addr) { + // Honor __attribute__((no_destroy)) and bail instead of attempting + // to emit a reference to a possibly nonexistent destructor, which + // in turn can cause a crash. This will result in a global constructor + // that isn't balanced out by a destructor call as intended by the + // attribute. This also checks for -fno-c++-static-destructors and + // bails even if the attribute is not present. + if (D.isNoDestroy(CGF.getContext())) + return; + CodeGenModule &CGM = CGF.CGM; // FIXME: __attribute__((cleanup)) ? Index: cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp =================================================================== --- cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp +++ cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp @@ -0,0 +1,42 @@ +// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -DNOATTR %s -o - | FileCheck %s +// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=CHECK-ATTR +// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -DNOATTR -fno-c++-static-destructors %s -o - | FileCheck %s --check-prefix=CHECK-FLAG + +// Regression test for D54344. Class with no user-defined destructor +// that has an inherited member that has a non-trivial destructor +// and a non-default constructor will attempt to emit a destructor +// despite being marked as __attribute((no_destroy)) in which case +// it would trigger an assertion due to an incorrect assumption. + +// This test is more reliable with asserts to work as without +// the crash may (unlikely) could generate working but semantically +// incorrect code. + +class a { +public: + ~a(); +}; +class logger_base { + a d; +}; +class e : logger_base {}; +#ifndef NOATTR +__attribute((no_destroy)) +#endif +e g; + +// In the absence of the attribute and flag, both ctor and dtor should +// be emitted, check for that. +// CHECK: @__cxx_global_var_init +// CHECK: @__cxa_atexit + +// When attribute is enabled, the constructor should not be balanced +// by a destructor. Make sure we have the ctor but not the dtor +// registration. +// CHECK-ATTR: @__cxx_global_var_init +// CHECK-ATTR-NOT: @__cxa_atexit + +// Same scenario except with global flag (-fno-c++-static-destructors) +// supressing it instead of the attribute. +// CHECK-FLAG: @__cxx_global_var_init +// CHECK-FLAG-NOT: @__cxa_atexit
Index: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp =================================================================== --- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp +++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp @@ -64,6 +64,15 @@ /// static storage duration. static void EmitDeclDestroy(CodeGenFunction &CGF, const VarDecl &D, ConstantAddress Addr) { + // Honor __attribute__((no_destroy)) and bail instead of attempting + // to emit a reference to a possibly nonexistent destructor, which + // in turn can cause a crash. This will result in a global constructor + // that isn't balanced out by a destructor call as intended by the + // attribute. This also checks for -fno-c++-static-destructors and + // bails even if the attribute is not present. + if (D.isNoDestroy(CGF.getContext())) + return; + CodeGenModule &CGM = CGF.CGM; // FIXME: __attribute__((cleanup)) ? Index: cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp =================================================================== --- cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp +++ cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp @@ -0,0 +1,42 @@ +// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -DNOATTR %s -o - | FileCheck %s +// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=CHECK-ATTR +// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -DNOATTR -fno-c++-static-destructors %s -o - | FileCheck %s --check-prefix=CHECK-FLAG + +// Regression test for D54344. Class with no user-defined destructor +// that has an inherited member that has a non-trivial destructor +// and a non-default constructor will attempt to emit a destructor +// despite being marked as __attribute((no_destroy)) in which case +// it would trigger an assertion due to an incorrect assumption. + +// This test is more reliable with asserts to work as without +// the crash may (unlikely) could generate working but semantically +// incorrect code. + +class a { +public: + ~a(); +}; +class logger_base { + a d; +}; +class e : logger_base {}; +#ifndef NOATTR +__attribute((no_destroy)) +#endif +e g; + +// In the absence of the attribute and flag, both ctor and dtor should +// be emitted, check for that. +// CHECK: @__cxx_global_var_init +// CHECK: @__cxa_atexit + +// When attribute is enabled, the constructor should not be balanced +// by a destructor. Make sure we have the ctor but not the dtor +// registration. +// CHECK-ATTR: @__cxx_global_var_init +// CHECK-ATTR-NOT: @__cxa_atexit + +// Same scenario except with global flag (-fno-c++-static-destructors) +// supressing it instead of the attribute. +// CHECK-FLAG: @__cxx_global_var_init +// CHECK-FLAG-NOT: @__cxa_atexit
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits