Hide libunwind's _Unwind_* symbols under Bazel to fix bthread_tracer crash#3297
Open
chenBright wants to merge 1 commit into
Open
Hide libunwind's _Unwind_* symbols under Bazel to fix bthread_tracer crash#3297chenBright wants to merge 1 commit into
chenBright wants to merge 1 commit into
Conversation
…crash libunwind ships its `src/unwind/*.c` (the GCC `_Unwind_*` ABI compatibility layer) as exported symbols of `libexternal_S~libunwind.so`. At runtime the dynamic loader resolves `_Unwind_*` lookups (from `pthread_exit`, libstdc++'s `__gxx_personality_v0`, etc.) to libunwind's DWARF-based implementation instead of `libgcc_s.so.1`, hitting an uninitialized internal context and crashing on the no-return cleanup chain triggered by pthread_exit / C++ exception unwinding -- e.g. it makes BthreadTest.bthread_exit segfault deterministically when `--define=with_bthread_tracer=true` is on. This is purely an ELF runtime symbol-resolution-order issue and reproduces identically on GCC and Clang, since both default to `libstdc++ + libgcc_s` on Linux.
9bb354a to
18be781
Compare
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.
What problem does this PR solve?
Issue Number: resolve
Problem Summary:
When
--define=with_bthread_tracer=trueis enabled under Bazel(
bazel test //test:bthread_unittest), tests such asBthreadTest.bthread_exitsegfault insidepthread_exit/__gxx_personality_v0, with a stack like:Root cause: libunwind's
src/unwind/*.cexports the_Unwind_*GCC ABIsymbols, which collide with the same symbols in
libgcc_s.so.1. Bazel'sdefault fastbuild mode wraps each
cc_libraryinto an intermediate.so,so
libexternal_S~libunwind.soends up earlier in the binary'sDT_NEEDEDthanlibgcc_s.so.1, and the runtime dynamic linker resolvesthe
_Unwind_*calls coming frompthread_exit/ libstdc++ tolibunwind's DWARF-based implementation instead of libgcc_s.
This is a runtime symbol-resolution-order issue independent of the
compiler -- Clang's default toolchain on Linux uses the same
libstdc++ + libgcc_sruntime and reproduces the exact same crash.The make / cmake CI does not hit this because it links the
autoconf-built libunwind which hides
_Unwind_*by default.Distro packages (
libunwind-dev,libunwind-devel) typicallyalso re-export the symbols, so they have the same risk.
What is changed and the side effects?
Changed:
registry/modules/libunwind/, hosting two variants of libunwind:1.8.1.brpc-no-unwindand1.8.3.brpc-no-unwind. Both are forks ofthe corresponding modules in Bazel Central Registry, Apache-2.0 licensed.
License attribution and a per-file change list are added at the bottom
of the top-level
LICENSEfile.hide_unwind_symbolsconfig_settinggated by
--define=libunwind_hide_unwind_symbols=true. When the switchis on, the
src/unwind/*.cglob is dropped from the build viaselect(), so libunwind no longer provides_Unwind_*symbols andlibgcc_s.so.1wins the runtime lookup.Side effects:
Performance effects:
Breaking backward compatibility:
Check List: