Hlms: give msThreadId default visibility so a static build links into a PIE - #586
Merged
eugenegff merged 1 commit intoAug 1, 2026
Merged
Conversation
… a PIE
OGRE_SHADER_THREADING_USE_TLS is a static-build setting, and there _OgreExport
is visibility("hidden"). Hlms::msThreadId is declared in OgreHlms.h and defined
constant-initialized in OgreHlms.cpp, so a translation unit that sees only the
declaration emits the Itanium ABI thread-local access wrapper and its weak
reference to the init function _ZTHN4Ogre4Hlms10msThreadIdE - a symbol a
constant-initialized variable never defines. Resolving that dangling weak
reference to zero needs a GOT entry, which the compiler only emits for a symbol
that may bind externally; hidden, clang addresses it directly and leaves
R_X86_64_PC32, which GNU ld refuses to link into a position-independent
executable.
Only optimized builds hit it: at -O0 the relocation stays inside the wrapper's
own COMDAT section, which the linker discards in favour of the clean copy
OgreHlms.cpp contributes, while inlining moves it into ordinary .text where it
survives to the final link. GCC reaches the symbol through the GOT regardless
and AArch64 does too, so only clang on x86-64 in Release is affected.
Declaring the member with explicit default visibility restores the indirection
every other configuration already uses. There is no runtime cost: the linker
relaxes the resulting general-dynamic TLS access back to local-exec because the
variable is defined in the executable. MSVC keeps the byte-identical old
declaration.
Member
|
Looks alright. @eugenegff any comments? |
Member
|
The investigation is impressive, the fix is safe, 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.
A static Ogre-Next cannot be linked into a Release position-independent executable with clang on x86-64:
Why
Hlms::msThreadIdis declaredthread_localinOgreHlms.hand defined constant-initialized (= 0u) inOgreHlms.cpp. A translation unit that sees only the declaration cannot know the initialization is constant, so it emits the Itanium ABI thread-local access wrapper_ZTW..., which weakly references the thread-local init function_ZTHN4Ogre4Hlms10msThreadIdE— a symbol that a constant-initialized variable never defines anywhere.Resolving that dangling weak reference to zero is the linker's job, and it needs the indirection of a GOT entry. The compiler only emits that for a symbol that may bind externally.
OGRE_SHADER_THREADING_USE_TLSis set only for static builds, where_OgreExportexpands tovisibility("hidden")— so clang addresses the symbol directly, and the resultingR_X86_64_PC32cannot go into a PIE.Three things hide it, which is presumably why it has gone unnoticed:
-O0the reference lives in.text._ZTWN4Ogre4Hlms10msThreadIdE, the wrapper's own COMDAT, which the linker discards in favour of the clean copyOgreHlms.cppcontributes (and that object precedesOgreRenderQueue.cpp.oin the archive). At-O3clang inlines the wrapper into plain.text, where the relocation cannot be discarded.R_AARCH64_ADR_GOT_PAGE), which links fine.R_X86_64_GOTPCRELhere unconditionally.The change
Declare the member with explicit
visibility("default"), guarded by Ogre's ownOGRE_GCC_VISIBILITY. That restores the GOT indirection every other configuration already uses. MSVC keeps the byte-identical old declaration.There is no runtime cost: the linker relaxes the resulting general-dynamic TLS access back to local-exec, because the variable is defined in the executable.
Alternative considered and rejected
static inline thread_local uint32 msThreadId = 0u;removes the wrapper entirely and is arguably the more modern spelling — but the inline/non-inline choice must be uniform across the library boundary, and it can only be guarded on__cpp_inline_variables. Ogre-Next compiles at the compiler default (gnu++17 under clang, C++14 under MSVC) while a consumer may compile at a different standard, so the guard would evaluate differently on the two sides of the boundary: on MSVC that pairs a COMDAT definition in the consumer's objects with a strong out-of-line one here, and a C++11 consumer would lose the symbol altogether. A visibility attribute is standard-independent and therefore uniform by construction.Verification
Compiling the real
OgreRenderQueue.cppforx86_64-linux-gnuwith the release flags:WEAK HIDDEN UND _ZTHN4Ogre4Hlms10msThreadIdEwith threeR_X86_64_PC32relocations, and GNU ld reproduces the error above verbatim;WEAK DEFAULT UNDwithR_X86_64_GOTPCREL, and zero PIE relocation errors.A full static Release build then links both a player and an editor executable as PIE.
Found while building Ogre-Next statically into the editor of orkige, whose Release Linux binaries were the first to ask for that combination.