Give externally-provided symbol declarations default visibility - #2136
Open
rvandermeulen wants to merge 1 commit into
Open
Give externally-provided symbol declarations default visibility#2136rvandermeulen wants to merge 1 commit into
rvandermeulen wants to merge 1 commit into
Conversation
Abseil forward-declares a handful of symbols that are provided by something outside the object being linked: the compiler-based sanitizer runtime supplies the Annotate* race annotations and __tsan_read1, and Bionic supplies __mmap2. None of these have a public header to include, so declaring them locally is the only option, but the declarations inherit whatever visibility is in effect. When a translation unit is compiled with -fvisibility=hidden and abseil is linked into a shared library, those references become hidden undefined symbols and cannot be resolved from the runtime or from libc, so the link fails: ld.lld: error: undefined hidden symbol: AnnotateIgnoreReadsBegin ld.lld: error: undefined hidden symbol: AnnotateIgnoreWritesBegin ld.lld: error: undefined hidden symbol: __tsan_read1 ld.lld: error: undefined hidden symbol: __mmap2 Wrap the declarations in a visibility push/pop so they stay resolvable. Every site is already inside a Bionic or sanitizer guard, so this does not affect platforms where the attribute is unavailable.
rvandermeulen
force-pushed
the
sanitizer-and-mmap2-visibility
branch
from
August 22, 2026 22:08
7c3e130 to
ab78d3a
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.
Abseil forward-declares a few symbols that are provided by something outside the object being linked:
Annotate*race annotationsabsl/base/dynamic_annotations.h__tsan_read1absl/synchronization/mutex.cc__mmap2absl/base/internal/direct_mmap.hNone of these has a public header to include — none of the
Annotate*functions nor__tsan_read1appears in any compiler-rtsanitizer/*.h, and__mmap2is a private Bionic symbol — so declaring them locally is the only option. But a local declaration inherits whatever visibility is in effect at that point.When the translation unit is compiled with
-fvisibility=hiddenand abseil is linked into a shared library, those references become hidden undefined symbols. They then cannot be resolved from the sanitizer runtime (which lives in the executable) or from libc, and the link fails:This wraps each declaration in
#pragma GCC visibility push(default)/pop.Verification
Compiling all ~163 abseil sources with
-fsanitize=threadand a tree-wide#pragma GCC visibility push(hidden), then scanning every object for hidden undefined symbols: before this change the fourAnnotate*and__tsan_read1areGLOBAL HIDDEN UND; after, every__tsan_*andAnnotate*reference isGLOBAL DEFAULT UND. The only hidden undefined symbols remaining areAbslInternal*ones that abseil defines itself, which is correct.For
__mmap2, on a 32-bit Android shared-library link:Notes
__mmap2path only compiles on 32-bit architectures, which is why this is rarely hit — Chromium's Android builds are 64-bit. We hit it on 32-bit ARM.-fvisibility=hiddentree-wide.