Use Default visibility for rustc-generated C symbol declarations #131519
Conversation
This file contains 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
|
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
rustbot
added
S-waiting-on-review
Urgau
reviewed
Oct 10, 2024
Non-default visibilities should only be used for definitions, not declarations, otherwise linking can fail. Co-authored-by: Collin Baker <collinbaker@chromium.org>
davidlattimore
force-pushed
the
intrinsics-default-vis
branch
from
2acf902
to
42c0494
Compare
|
r? compiler |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 11, 2024
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#129079 (Create `_imp__` symbols also when doing ThinLTO) - rust-lang#131208 (ABI: Pass aggregates by value on AIX) - rust-lang#131394 (fix(rustdoc): add space between struct fields and their descriptions) - rust-lang#131519 (Use Default visibility for rustc-generated C symbol declarations) - rust-lang#131541 (compiletest: Extract auxiliary-crate properties to their own module/struct) - rust-lang#131542 (next-solver: remove outdated FIXMEs) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 11, 2024
Rollup merge of rust-lang#131519 - davidlattimore:intrinsics-default-vis, r=Urgau Use Default visibility for rustc-generated C symbol declarations Non-default visibilities should only be used for definitions, not declarations, otherwise linking can fail. This is based on rust-lang#123994. Issue rust-lang#123427 When I changed `default-hidden-visibility` to `default-visibility` in rust-lang#130005, I updated all places in the code that used `default-hidden-visibility`, replicating the hidden-visibility bug to also happen for protected visibility. Without this change, trying to build rustc with `-Z default-visibility=protected` fails with a link error.
This was referenced Oct 25, 2024
github-actions bot
pushed a commit
to kaidokert/chrome_base_mirror
that referenced
this pull request
Oct 30, 2024
This depended on rust-lang/rust#131519 rolling into the Rust compiler to fix builtins/intrinisics, which was merged in 33b1264540b. Remove ASAN ODR exceptions for Rust globals as they are no longer walked twice incorrectly when they are hidden. Disable the rust logging test in component builds for all platforms not just for Windows, now that symbol visibility is the same for all platforms. See https://crbug.com/374023535#comment13 Bug: 41484256, 40274892, 374023535 Fixed: 40278279 Cq-Include-Trybots: luci.chromium.try:linux-official,mac-official,win-official,win32-official Cq-Include-Trybots: luci.chromium.try:linux-asan-rel,mac-asan-rel,win-asan-rel Cq-Include-Trybots: luci.chromium.try:linux_chromium_dbg_ng Change-Id: I976ef679961e9db5a91bcc74f559ebbcc0bf058b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5966273 Commit-Queue: danakj <danakj@chromium.org> Reviewed-by: Arthur Eubanks <aeubanks@google.com> Cr-Commit-Position: refs/heads/main@{#1375460} NOKEYCHECK=True GitOrigin-RevId: ee3900fd57b3c580aefff15c64052904d81b7760
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.
Non-default visibilities should only be used for definitions, not declarations, otherwise linking can fail.
This is based on #123994.
Issue #123427
When I changed
default-hidden-visibilitytodefault-visibilityin #130005, I updated all places in the code that useddefault-hidden-visibility, replicating the hidden-visibility bug to also happen for protected visibility.Without this change, trying to build rustc with
-Z default-visibility=protectedfails with a link error.