bug #37251 from clang 7 still exists in 14

[martinbonaciugome]

#37251 still exists in clang-cl 14.0.6 but is also happening in debug build. My memory blew on initializing an inline global thread pool in a dll, as it was creating 1000s of instances of pools during program initialization. Same code works perfectly as a framework and .so on iOS/macOS and android/linux

[topperc]

@zmodem @rnk @jyu2-git

[rnk]

Please provide more info. I don't think we need a full executable reproducer, but the details of the class, the static data member, it's inline initializer, etc, these are the things that matter. The DLL structure and the export annotations probably also matter.

[martinbonaciugome]

I don't have a DllMain function, and this dll is just regularly linked using standard CMake target_link_libraries. This is my class and global declaration. Lemme know if you need more

  struct TaskScheduler final
  {
    TaskScheduler(uint count) noexcept;

    bool IsWaiting() const noexcept
    {
      return WaitFlags.Load(MemoryOrder::Relaxed);
    }
    auto ThreadCount() const noexcept
    {
      return Workers.Count();
    }

    Action operator co_await() const noexcept;

    void Suspend() noexcept;
    void Resume() noexcept;

  private:
#pragma mark - Worker
    struct Worker
    {
      TaskScheduler& Scheduler;
      uintptr_t WaitMask;
      EventQueue Queue;
      Thread Thread;

      Worker(TaskScheduler& scheduler, uintptr_t waitMask) noexcept;
      ~Worker();

      void Run() noexcept;
    };

#pragma mark -
    alignas(CacheLine) mutable Atomic<uint> Counter = 0;
    alignas(CacheLine) Atomic<uintptr_t> WaitFlags = 0;
    alignas(CacheLine) List<Worker> Workers;
    Mutex Mutex;
    Condition Condition;
    bool Pause = false;
  };

  inline auto& Continuation(TaskScheduler const& scheduler) noexcept
  {
    return scheduler;
  }

  namespace global
  {
#if IGL_SERVER_INPLACE
    __declspec(dllimport) extern TaskScheduler BlockingTaskScheduler;
#else
    __declspec(dllexport) inline TaskScheduler BlockingTaskScheduler{min(HardwareThreadCount(), uint(sizeof(uintptr_t) * 8))};
#endif
  }

[rnk]

I see, the bug looks pretty straightforward:

struct TaskScheduler final { TaskScheduler(int); };
__declspec(dllexport) inline TaskScheduler BlockingTaskScheduler{52};

Marking things exported makes them non-discardable, so this condition does the wrong thing:
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGDeclCXX.cpp#L561

  } else if (isTemplateInstantiation(D->getTemplateSpecializationKind()) ||
             getContext().GetGVALinkageForVariable(D) == GVA_DiscardableODR ||
             D->hasAttr<SelectAnyAttr>()) {

We should also use this codepath for GVA_StrongODR. If anyone wants to dig into code review comments on this code, I'm sure they can find some past opinions.

[rnk]

I should add, thank you for the code, it was very helpful. :)

[martinbonaciugome]

No thank you for such a quick response. Very refreshing with bug reporting

[llvmbot]

@llvm/issue-subscribers-clang-codegen

[pthurau]

Unfortunately, the bug still seems to exist:
We encountered a segmentation fault in executables compiled with LLVM on Windows. The issue was traced to static inline members of an exported class, which caused multiple constructor and destructor calls, leading to memory corruption. This issue only occurs when the static inline member is used in multiple translation units (TUs). The number of constructor and destructor calls equals the number of TUs.

Details:

• Environment: Windows, LLVM 16 (also tested with LLVM 18)
• Reproduction Steps:

1. Define a class with a static inline member.
2. Export the class.
3. Use the member in multiple translation units (TUs).

• Observed Behavior: Multiple calls to CTor and DTor with same "this" pointer -> double free.
• Workaround: Replacing static inline with static and defining the member in a source file resolves the issue.
• The issue was confirmed to be Windows-specific.
• The problem is resolved if LLVM is patched to include GVA_StrongODR at this location: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.6/clang/lib/CodeGen/CGDeclCXX.cpp#L572, like @rnk suggested. However, the potential effects of this change are completely unknown, so we do not want to use a self-patched version.

Example:
repro.zip

Please find example code attached, run CMake:
cmake -DCMAKE_C_COMPILER=clang-cl -DCMAKE_CXX_COMPILER=clang-cl -G "NMake Makefiles" <source_dir>

Output:

>main.exe
Constructor: this = 00007FFAD75A5034
Constructor: this = 00007FFAD75A5034
Destructor: this = 00007FFAD75A5034
Destructor: this = 00007FFAD75A5034