fix: use compiled spdlog with hidden visibility to prevent symbol collision#332
Merged
Merged
Conversation
…lision Both MORI and rocroller (loaded via hipblaslt/TransformerEngine) statically link spdlog. When TransformerEngine loads libtransformer_engine.so with RTLD_GLOBAL, rocroller's spdlog symbols enter the global namespace and interpose MORI's spdlog calls (e.g. registry::initialize_logger dispatches into rocroller's set_formatter), causing heap corruption (free(): invalid size / SIGABRT). Switch from spdlog::spdlog_header_only to the compiled spdlog::spdlog target and set CXX_VISIBILITY_PRESET=hidden + VISIBILITY_INLINES_HIDDEN=ON. This compiles spdlog into a static archive with hidden visibility; when linked into MORI's .so files, the spdlog symbols are not exported to the dynamic symbol table and cannot be interposed. The previous header-only mode (introduced in aceb70c for an XLA build fix) made target_compile_options on the spdlog target ineffective because MORI never linked the compiled spdlog library — the visibility flags were applied to an unused target. The XLA build concern no longer applies after the header decoupling work in commits 53a8513 and 40c0820. Tested: TE (all 3 init orders PASS), JAX/XLA (BUILD_XLA_FFI_OPS=ON PASS).
jhchouuu
force-pushed
the
fix/spdlog-visibility-te-collision
branch
from
6d0a579 to
a805e11
Compare
There was a problem hiding this comment.
Pull request overview
This PR adjusts MORI’s build configuration to prevent runtime crashes caused by cross-library spdlog symbol interposition when other dependencies (e.g., rocroller via hipblaslt/TransformerEngine loaded with RTLD_GLOBAL) also statically link spdlog.
Changes:
- Switch MORI’s logging dependency from
spdlog::spdlog_header_onlyto the compiledspdlog::spdlogtarget. - Apply hidden symbol visibility settings to the
spdlogtarget viaCXX_VISIBILITY_PRESET=hiddenandVISIBILITY_INLINES_HIDDEN=ON. - Add detailed build-system documentation in
CMakeLists.txtexplaining the symbol-collision failure mode being addressed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pemeliya
approved these changes
May 21, 2026
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.
Summary
spdlog::spdlog_header_only) to compiled static library (spdlog::spdlog) withCXX_VISIBILITY_PRESET=hiddenandVISIBILITY_INLINES_HIDDEN=ON, preventing spdlog symbols from leaking into MORI's dynamic symbol table.Problem
When TransformerEngine is imported before MORI shmem init, the process crashes with
free(): invalid size(SIGABRT).Root cause: Both MORI and rocroller (loaded transitively via
libtransformer_engine.so→libhipblaslt.so→librocroller.so) statically link spdlog. TE loads its core library withctypes.RTLD_GLOBAL, which promotes rocroller's spdlog symbols into the global namespace. MORI's spdlog calls (e.g.registry::initialize_logger) are then interposed by rocroller's copy, and cross-library object lifetime mismatch causes heap corruption.GDB backtrace confirms the interposition:
#11 spdlog::details::registry::initialize_logger() ← from libmori_application.so #10 spdlog::sinks::ansicolor_sink::set_formatter() ← from librocroller.so (!) #7 spdlog::details::full_formatter::~full_formatter() ← from librocroller.so #6 free() → SIGABRTFix
target_compile_options(spdlog PRIVATE -fvisibility=hidden ...)set_target_properties(spdlog PROPERTIES CXX_VISIBILITY_PRESET hidden VISIBILITY_INLINES_HIDDEN ON)spdlog::spdlog_header_onlyspdlog::spdlogWith compiled mode, spdlog is built into
libspdlog.awith hidden visibility. When linked into MORI's.sofiles, the spdlog symbols are present but hidden from the dynamic symbol table — they cannot be interposed by rocroller's copy.Test plan
mori-only→ PASSmori-then-te→ PASSte-then-mori→ PASS (was CRASH)BUILD_XLA_FFI_OPS=ON pip install -e .→ PASS.hsacokernel generation → PASS