[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

[Kristina Brooks via Phabricator via cfe-commits]

kristina updated this revision to Diff 173503.
kristina set the repository for this revision to rC Clang.
kristina added a comment.

Revised, I think I worked out what was happening, there's an explanation in the comment in the test. This is a relatively new attribute so the coverage for it did not test this specific corner case, I've managed to manually narrow it down and write a reliable regression test. The core issue boils down to having a non-trivially destructible member in a superclass that lacks a destructor and a subclass that lacks a destructor, in which case, a different path was taken to balance out the static storage duration class' initializer call and the `__cxa_atexit` registration. This adds an explicit check for the attribute and avoids balancing out the constructor as intended by the attribute.

The new test successfully replicates the assertion failure and should fail for the above cases in assertion builds. Without assertions the generated code produces undefined behavior if the above conditions are met. There is a separate test for this attribute that provides the coverage for its functionality, this is a much more narrower test for a very specific scenario in which it was possible to cause an improperly balanced constructor call followed by a emission of a call to a destructor that would never be emitted due to the attribute, thus tripping the assert. Now no attempt to call the destructor is made if the declaration is marked as `no_destroy`.

`Test without patch:`

  => ninja check-clang-semacxx
  
  FAIL: Clang :: SemaCXX/attr-no-destroy-d54344.cpp (164 of 818)
  ******************** TEST 'Clang :: SemaCXX/attr-no-destroy-d54344.cpp' FAILED ********************
  
  (--- stack trace and the expected assertion failure ---)
  
  /q/org.llvm.caches/llvm-8.0/4141/tools/clang/test/SemaCXX/Output/attr-no-destroy-d54344.cpp.script: line 1: 31356 Aborted (core dumped)
  
  --
  
  ********************
  Testing Time: 5.03s
  ********************
  Failing Tests (1):
      Clang :: SemaCXX/attr-no-destroy-d54344.cpp
  
    Expected Passes    : 816
    Expected Failures  : 1
    Unexpected Failures: 1
  FAILED: tools/clang/test/CMakeFiles/check-clang-semacxx
  
  ninja: build stopped: subcommand failed.

`Patch applied:`

  => ninja check-clang-semacxx
  
  Testing Time: 6.28s
    Expected Passes    : 817
    Expected Failures  : 1


Repository:
  rC Clang

https://reviews.llvm.org/D54344

Files:
  lib/CodeGen/CGDeclCXX.cpp
  test/SemaCXX/attr-no-destroy-d54344.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54344.173503.patch
Type: text/x-patch
Size: 3208 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181110/dc32db4c/attachment.bin>