-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Description
With #26486 we started using TLS (C11 _Thread_local) to store the current local.
This seems to have a bad interaction this thinglto due to some kind LLVM issue.
Gemini says:
✦ Triage of recent CI failures identified a regression in ThinLTO builds using pthreads, specifically failing in test_pthread_dylink and other related tests. The failures were triggered by a recent Emscripten change (#26486 (#26486)) that introduced a _Thread_local variable for locale support in musl.
The root cause was a bug in the LLVM WebAssembly backend's CoalesceFeaturesAndStripAtomics pass. This pass determines the union of features for a module based only on its defined functions. In ThinLTO builds, if a function like __uselocale is inlined into all its callers, its original module may be left with no defined functions but still contain the thread_local variable definition. In such cases, the pass would fail to
enable the bulk-memory feature, leading it to incorrectly strip the TLS attribute from the variable or lower it to a non-TLS section. This resulted in wasm-ld errors about disallowed shared memory or relocation mismatches.
I have implemented a fix in llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp that ensures coalesceFeatures also inspects global variables for the thread_local attribute. If found, it explicitly enables the bulk-memory and atomics features for the module, preventing the incorrect stripping of TLS. This fix was verified to resolve both a minimal reproduction case and the original test_pthread_dylink failure.
Changes:
- Modified llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp to include thread_local globals in the feature coalescence logic.
- Verified the fix with emscripten/test/runner.py thinltoz.test_pthread_dylink.