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

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

This seems to be a peculiar side effect of weird combinations of previous uses of attributes `no_destroy`, `used`, and `section`, as well as complex macros used to create metaclass-like systems (through the use of said attributes). It seems that there is a serious bug somewhere, the repeated use of metaclass-like macros seem to break Clang's lexer/parser and leave it in a state where it starts behaving erratically, with this being a major symptom of it.

The trace state at the time of the crash usually resembles this:

  1.      /SourceCache/ips-1.0.0/os/credentials.cpp:20:41: current parser token ';'
  2.      LLVM IR generation of declaration '<using-directive>'
  3.      /SourceCache/ips-1.0.0/os/credentials.cpp:20:1 <Spelling=/SourceCache/ips-1.0.0/os/credentials.cpp:20:17>: Generating code for declaration '(anonymous namespace)::s_log'

When using `static` instead of an anonymous namespace the last token parsed is `static` instead of `;`. I also seem to have found a bug in implementation of `no_destroy`, fixing which would be considered an actual "fix" instead of a "workaround". I gave several other approaches of replicating this state, closest one being:

  class global_ns_class {
  public:
  	global_ns_class(int some_value) { SomeValue[2] = some_value; }
  	~global_ns_class() { SomeValue[2] = 1; }
  protected:
  	volatile int SomeValue[10];
  };
  
  #define MAKE_GVAR(_sfx, _init_val) \
  global_ns_class g_var##_sfx(_init_val); \
  __attribute((no_destroy, used, section("namedsect"))) \
  global_ns_class* g_var##_sfx##_ref = &g_var##_sfx;
  
  MAKE_GVAR(ONE,   1);
  MAKE_GVAR(TWO,   2);
  MAKE_GVAR(THREE, 3);
  
  namespace my_ns {
  	class with_nontrivial {
  	public:
  		with_nontrivial() { SomeValue[2] = 3; }
  		~with_nontrivial() { SomeValue[2] = 4; }
  	protected:
  		int SomeValue[10];
  	};
  	
  	class my_class : public with_nontrivial {
  	public:
  		my_class(const char* cstr) {}
  	};
  }
  
  #define MAKE_VAR(_x,_y) \
  	namespace { constexpr char _$__##_x[] = _y; \
  	__attribute((no_destroy)) my_ns::my_class _x (_$__##_x); }
  	
  MAKE_VAR(s_var, "test str lit");

Which seems to force Clang to take an incorrect code path but not trip the assert, unlike in the project where the assert is reliably tripped likely due to a manifestation of another, far more bening bug which is far beyond the scope of my understanding of Clang internals (it seems to be rooted in the preprocessor<->sema interactions). The 400k line preprocessed file is able to trigger it with 100% reliability, the preconditions for triggering the bug require the code to be semantically valid, which makes it hard to fuzz.

There seem to be too many variables to take into account to reliably reproduce it with the assert firing off, some manifestations just generate subtly-incorrect code (by emitting `__cxa_atexit` even with attribute present). The libc `atexit` fallback in handling of that attribute also seems like it could lead to issue but at this point this is just speculation.

I'm honestly not sure what to make of it at this point. Considering it's an immensely useful attribute for eliminating destructors of objects with "infinite" lifetime (I first hit this bug when I added it to all top level loggers which are created through a macro similar to one above), given that the x86_64 test suite does pass with it, I still think some kind of workaround is worth it (with reference to this issue)? I'd be happy to revert it if/once a concrete cause is found but so far I seem to be hitting dead ends.

I added a logger for when this specific issue is triggered to my downstream fork and it seems in most cases it causes "bening" behavior of attribute being ineffective. The assert is tripped when the attribute is partially ineffective, leading to Clang trying to emit code to register a destructor that doesn't exist with cxxabi.

Considering this is something I will likely have to dig through by hand, can anyone help me brainstorm how to approach this besides fuzzing it which I don't think will work due to the precondition of the code being semantically valid (not being diagnosed with an error)?


https://reviews.llvm.org/D54344