Skip to content

Commit

Permalink
Auto merge of #131634 - davidlattimore:lld-protected, r=<try>
Browse files Browse the repository at this point in the history
Use protected visibility when LLD feature is enabled and enable it when building rustc

rust-lang/compiler-team#782

I wasn't sure about having two commits in a PR, but I figured, at least initially it might make sense to discuss these commits together. Happy to squash, or move the second commit to a separate PR.

I contemplated trying to enable protected visibility for more cases when LLD will be used other than just `-Zlinker-features=+lld`, but that would be more a complex change that probably still wouldn't cover all cases when LLD is used, so went with the simplest option of just checking if the linker-feature is enabled.

r? lqd
  • Loading branch information
bors committed Oct 13, 2024
2 parents 2aa26d8 + 1e1bf2b commit 6ad0952
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 7 deletions.
13 changes: 11 additions & 2 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use rustc_span::source_map::{FilePathMapping, SourceMap};
use rustc_span::{FileNameDisplayPreference, RealFileName, Span, Symbol};
use rustc_target::asm::InlineAsmArch;
use rustc_target::spec::{
CodeModel, DebuginfoKind, PanicStrategy, RelocModel, RelroLevel, SanitizerSet,
CodeModel, DebuginfoKind, LinkerFeatures, PanicStrategy, RelocModel, RelroLevel, SanitizerSet,
SmallDataThresholdSupport, SplitDebuginfo, StackProtector, SymbolVisibility, Target,
TargetTriple, TlsModel,
};
Expand Down Expand Up @@ -622,11 +622,20 @@ impl Session {

/// Returns the default symbol visibility.
pub fn default_visibility(&self) -> SymbolVisibility {
// For now, we default to using protected only if the LLD linker features is enabled. This
// is to avoid link errors that are likely if using protected visibility with GNU ld < 2.40.
let fallback =
if self.opts.unstable_opts.linker_features.enabled.contains(LinkerFeatures::LLD) {
SymbolVisibility::Protected
} else {
SymbolVisibility::Interposable
};

self.opts
.unstable_opts
.default_visibility
.or(self.target.options.default_visibility)
.unwrap_or(SymbolVisibility::Interposable)
.unwrap_or(fallback)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/utils/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ pub fn linker_flags(
) -> Vec<String> {
let mut args = vec![];
if !builder.is_lld_direct_linker(target) && builder.config.lld_mode.is_used() {
args.push(String::from("-Clink-arg=-fuse-ld=lld"));
args.push(String::from("-Zlinker-features=+lld"));

if matches!(lld_threads, LldThreads::No) {
args.push(format!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@ setting.

This option only affects building of shared objects and should have no effect on executables.

Visibility an be set to one of three options:
Visibility can be set to one of three options:

* protected
* hidden
* interposable

The default value for this flag is `interposable` unless `-Zlinker-features=+lld` is specified, in
which case the default is `protected`.

## Hidden visibility

Using `-Zdefault-visibility=hidden` is roughly equivalent to Clang's
Expand Down
10 changes: 7 additions & 3 deletions tests/codegen/default-visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
// also https://github.com/rust-lang/rust/issues/73295 and
// https://github.com/rust-lang/rust/issues/37530.

//@ revisions:DEFAULT HIDDEN PROTECTED INTERPOSABLE
//@ revisions:NO_LLD LLD HIDDEN PROTECTED INTERPOSABLE
//@[HIDDEN] compile-flags: -Zdefault-visibility=hidden
//@[PROTECTED] compile-flags: -Zdefault-visibility=protected
//@[INTERPOSABLE] compile-flags: -Zdefault-visibility=interposable
//@[NO_LLD] compile-flags: -Zlinker-features=-lld
//@[LLD] compile-flags: -Zlinker-features=+lld

// The test scenario is specifically about visibility of symbols exported out of dynamically linked
// libraries.
Expand All @@ -30,7 +32,8 @@ pub static tested_symbol: [u8; 6] = *b"foobar";
// HIDDEN: @{{.*}}default_visibility{{.*}}tested_symbol{{.*}} = hidden constant
// PROTECTED: @{{.*}}default_visibility{{.*}}tested_symbol{{.*}} = protected constant
// INTERPOSABLE: @{{.*}}default_visibility{{.*}}tested_symbol{{.*}} = constant
// DEFAULT: @{{.*}}default_visibility{{.*}}tested_symbol{{.*}} = constant
// NO_LLD: @{{.*}}default_visibility{{.*}}tested_symbol{{.*}} = constant
// LLD: @{{.*}}default_visibility{{.*}}tested_symbol{{.*}} = protected constant

pub fn do_memcmp(left: &[u8], right: &[u8]) -> i32 {
left.cmp(right) as i32
Expand All @@ -46,4 +49,5 @@ pub fn do_memcmp(left: &[u8], right: &[u8]) -> i32 {
// HIDDEN: declare i32 @memcmp
// PROTECTED: declare i32 @memcmp
// INTERPOSABLE: declare i32 @memcmp
// DEFAULT: declare i32 @memcmp
// NO_LLD: declare i32 @memcmp
// LLD: declare i32 @memcmp

0 comments on commit 6ad0952

Please sign in to comment.