Work around ORC emulated-TLS weak-discard crash on the in-process JIT#1045
Open
conrade-ctc wants to merge 1 commit into
Open
Work around ORC emulated-TLS weak-discard crash on the in-process JIT#1045conrade-ctc wants to merge 1 commit into
conrade-ctc wants to merge 1 commit into
Conversation
Contributor
Author
|
@vgvassilev, here's a compatability patch/work-around for the bug that I just posted a PR to clang for. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1045 +/- ##
==========================================
+ Coverage 86.73% 86.77% +0.04%
==========================================
Files 23 23
Lines 5970 5990 +20
==========================================
+ Hits 5178 5198 +20
Misses 792 792
🚀 New features to boost your workflow:
|
af53887 to
7dc5507
Compare
vgvassilev
reviewed
Jul 2, 2026
7dc5507 to
6f78b14
Compare
6f78b14 to
cedb7ec
Compare
cedb7ec to
8977039
Compare
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
8977039 to
fa9c5cc
Compare
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
fa9c5cc to
835969d
Compare
835969d to
33714af
Compare
Demote duplicate weak thread_local definitions to available_externally before they reach the JIT, sidestepping the buggy IRMaterializationUnit::discard (fixed upstream by llvm/llvm-project#207161; compiles out on clang >= 23). Cover every weak thread_local regardless of initializer: one with a non-trivial ctor is zeroinitializer in IR (its real init runs in a __tls_init function) yet still emits the emulated-TLS companion that trips discard. Adds regression tests for both non-zero- and zero-init weak thread_locals. Scoped to ELF: on COFF the demotion orphans comdat-associated members and on Mach-O it breaks the paired _ZTH/_ZTW emission -- and on both, the JIT fails to resolve __emutls_get_address before the discard bug is reachable, so the tests skip there. Co-developed-with-the-help-of: Claude Code (Claude Opus 4.8, human in the loop)
33714af to
2ef5452
Compare
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On the in-process JIT, two incremental modules that each define the same
weak/
linkonce_odrthread_localwith a non-zero initializer crash theinterpreter. This happens whenever an
inlinefunction (or template static)that odr-uses such a
thread_localis pulled into twoPartialTranslationUnits— exactly what
MakeFunctionCallableand repeatedprocess()calls do.Root cause is in LLVM ORC:
IRMaterializationUnit's constructor registers__emutls_t.<var>(emitted for non-zero-init emulated-TLS globals) inSymbolFlagsbut not inSymbolToDefinition. When the second modulere-defines the weak symbol,
JITDylib::defineImpldiscards the duplicate viaIRMaterializationUnit::discard(), which looks the symbol up inSymbolToDefinition, doesn't find it, and dereferencesend()— an assertionin
+Assertsbuilds, heap corruption otherwise.Fixed upstream in llvm/llvm-project#207161, but that only helps once the
LLVM baseline carries the fix (LLVM ≥ 23). This protects CppInterOp users on
current releases (21.x/22.x) in the meantime.
Fix
compat::dedupeWeakEmulatedTLS()(inCompatibility.h) runs on each modulebefore it reaches the JIT. For every weak/
linkonce_odrnon-zero-initthread_localdefinition seen more than once, it demotes the duplicate toavailable_externally. TheIRMaterializationUnitconstructor skipsavailable_externallyglobals, so the duplicate contributes no symbol, thebuggy
discard()is never reached, and the first module remains the soledefinition that later modules resolve against within the JITDylib — i.e. the
dedup
discard()was trying to do, done in IR before the crash.It's wired into
process()and the no-valueParseAndExecute(), covering bothuser
process()and wrapper compilation. Blast radius is limited to non-zero-initweak
thread_locals; every other weak/COMDAT symbol keeps the normal path,where
discard()already works.Gated
#if LLVM_VERSION_MAJOR < 23, so it compiles out once the fix is in thebaseline (mirroring the existing
static_assert(LLVM_VERSION_MAJOR < 23, …)deprecation pattern in
Compatibility.h).Test
FunctionReflection_DiscardDuplicateWeakEmulatedTLSreproduces the two-moduleduplicate-weak-TLS scenario. Verified against a deliberately-unpatched
libLLVMOrcJIT.a: passes with the workaround, aborts without it (negativecontrol). Full suite stays green.
🤖 Done with the help of Claude Code (Claude Opus 4.8, human in the loop)