kristina updated this revision to Diff 173507. kristina added a comment. Revised, added a newline to the testcase.
https://reviews.llvm.org/D54344 Files: lib/CodeGen/CGDeclCXX.cpp test/SemaCXX/attr-no-destroy-d54344.cpp
Index: test/SemaCXX/attr-no-destroy-d54344.cpp =================================================================== --- test/SemaCXX/attr-no-destroy-d54344.cpp +++ test/SemaCXX/attr-no-destroy-d54344.cpp @@ -0,0 +1,92 @@ +// RUN: %clang_cc1 -std=c++2a -emit-llvm -S -O0 %s -o - > /dev/null +// REQUIRES: asserts + +// 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 requires asserts to reliably work as without the crash +// it generates working but semantically incorrect code. + +namespace util { + template <typename T> + class test_vector + { + public: + test_vector(unsigned c) + : Used(0), TotalSize(sizeof(T) * c) + { + // Intentional + Storage = (T*)(new T[TotalSize]); + } + ~test_vector() { + delete[] Storage; + } + char* data() { + return Storage; + } + unsigned size() { + return TotalSize; + } + void push_back(T one_thing) { + Storage[Used++] = one_thing; + } + private: + unsigned Used; + unsigned TotalSize; + T* Storage; + }; +} + +volatile int do_not_optimize_me = 0; + +namespace os { + using char_vec_t = util::test_vector<char>; + + class logger_base + { + public: + constexpr logger_base() = default; + protected: + char_vec_t* NamePtr = nullptr; + char_vec_t Name = char_vec_t(10); + }; + + class logger : public logger_base + { + public: + void stuff(const char* fmt, int something) { + do_not_optimize_me = something + fmt[0]; + } + logger() + { + Name = char_vec_t(8); + Name.push_back('a'); + } + + logger(const char* name) + { + Name = util::test_vector<char>(__builtin_strlen(name)); + Name.push_back(name[0]); + Name.push_back(name[1]); + } + }; +} + +#define TOPLEVEL_LOGGER(_var_name, _category_const) \ + namespace { constexpr char $__##_var_name[] = _category_const; \ + __attribute((no_destroy)) os::logger _var_name ($__##_var_name) ; } + +TOPLEVEL_LOGGER(s_log, "something"); + +class some_class { +public: + some_class(int some_value) { + do_not_optimize_me = some_value; + s_log.stuff("wawawa", 5 + do_not_optimize_me); + } + ~some_class() = default; +}; + +static some_class s_ctor(1); Index: lib/CodeGen/CGDeclCXX.cpp =================================================================== --- lib/CodeGen/CGDeclCXX.cpp +++ lib/CodeGen/CGDeclCXX.cpp @@ -64,6 +64,14 @@ /// 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 non-existant 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 (D54344). + if (D.hasAttr<NoDestroyAttr>()) + return; + CodeGenModule &CGM = CGF.CGM; // FIXME: __attribute__((cleanup)) ?
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits