Skip to content

Commit

Permalink
Default to -Z plt=yes
Browse files Browse the repository at this point in the history
6009da0 defaulted to `-Z plt=no` (like
`clang -fno-plt`) which a not a useful default[1].

On x86-64, if the target symbol is preemptible, there is an
`R_X86_64_GLOB_DAT` relocation, and the (very minor) optimization works as
intended. However, if the target is non-preemptible, i.e. the target is
resolved to the same component, this is actually a pessimization due to
the longer instruction.

On RISC architectures, there is typically no single instruction which
can load a GOT entry and perform an indirect call. `-fno-plt` has a longer
code quence. For example, AArch64 needs 3 instructions:

    adrp    x0, _GLOBAL_OFFSET_TABLE_
    ldr     x0, [x0, #:gotpage_lo15:bar]
    br      x0

This does not end up with a serious code size issue, because LLVM
"RtLibUseGOT" is not implemented for non-x86 targets.

On x86-32, very new lld[2] (2022-12-31) is needed to support
general-dynamic/local-dynamic TLS models.

`-Z plt=no` is not an appropriate default, so just default to true for
all targets.

[1] https://maskray.me/blog/2021-09-19-all-about-procedure-linkage-table#fno-plt
[2] llvm/llvm-project@8dc7366
  • Loading branch information
MaskRay committed Jan 11, 2023
1 parent b22c152 commit 2b35723
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 39 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ fn test_unstable_options_tracking_hash() {
tracked!(panic_abort_tests, true);
tracked!(panic_in_drop, PanicStrategy::Abort);
tracked!(pick_stable_methods_before_any_unstable, false);
tracked!(plt, Some(true));
tracked!(plt, false);
tracked!(polonius, true);
tracked!(precise_enum_drop_elaboration, false);
tracked!(print_fuel, Some("abc".to_string()));
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1479,10 +1479,10 @@ options! {
"print some performance-related statistics (default: no)"),
pick_stable_methods_before_any_unstable: bool = (true, parse_bool, [TRACKED],
"try to pick stable methods first before picking any unstable methods (default: yes)"),
plt: Option<bool> = (None, parse_opt_bool, [TRACKED],
"whether to use the PLT when calling into shared libraries;
only has effect for PIC code on systems with ELF binaries
(default: PLT is disabled if full relro is enabled)"),
plt: bool = (true, parse_bool, [TRACKED],
"if false, use GOT-generating code sequences for external function calls. \
This results in longer code sequences, but may avoid a PLT if the function is not bound locally. \
Only has effect on ELF systems (default: yes)"),
polonius: bool = (false, parse_bool, [TRACKED],
"enable polonius-based borrow-checker (default: no)"),
polymorphize: bool = (false, parse_bool, [TRACKED],
Expand Down
19 changes: 2 additions & 17 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use rustc_span::edition::Edition;
use rustc_span::source_map::{FileLoader, RealFileLoader, SourceMap, Span};
use rustc_span::{sym, SourceFileHashAlgorithm, Symbol};
use rustc_target::asm::InlineAsmArch;
use rustc_target::spec::{CodeModel, PanicStrategy, RelocModel, RelroLevel};
use rustc_target::spec::{CodeModel, PanicStrategy, RelocModel};
use rustc_target::spec::{
DebuginfoKind, SanitizerSet, SplitDebuginfo, StackProtector, Target, TargetTriple, TlsModel,
};
Expand Down Expand Up @@ -908,22 +908,7 @@ impl Session {

/// Returns `true` if we cannot skip the PLT for shared library calls.
pub fn needs_plt(&self) -> bool {
// Check if the current target usually needs PLT to be enabled.
// The user can use the command line flag to override it.
let needs_plt = self.target.needs_plt;

let dbg_opts = &self.opts.unstable_opts;

let relro_level = dbg_opts.relro_level.unwrap_or(self.target.relro_level);

// Only enable this optimization by default if full relro is also enabled.
// In this case, lazy binding was already unavailable, so nothing is lost.
// This also ensures `-Wl,-z,now` is supported by the linker.
let full_relro = RelroLevel::Full == relro_level;

// If user didn't explicitly forced us to use / skip the PLT,
// then try to skip it where possible.
dbg_opts.plt.unwrap_or(needs_plt || !full_relro)
self.opts.unstable_opts.plt
}

/// Checks if LLVM lifetime markers should be emitted.
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1549,9 +1549,6 @@ pub struct TargetOptions {
pub position_independent_executables: bool,
/// Executables that are both statically linked and position-independent are supported.
pub static_position_independent_executables: bool,
/// Determines if the target always requires using the PLT for indirect
/// library calls or not. This controls the default value of the `-Z plt` flag.
pub needs_plt: bool,
/// Either partial, full, or off. Full RELRO makes the dynamic linker
/// resolve all symbols at startup and marks the GOT read-only before
/// starting the program, preventing overwriting the GOT.
Expand Down Expand Up @@ -1871,7 +1868,6 @@ impl Default for TargetOptions {
no_default_libraries: true,
position_independent_executables: false,
static_position_independent_executables: false,
needs_plt: false,
relro_level: RelroLevel::None,
pre_link_objects: Default::default(),
post_link_objects: Default::default(),
Expand Down Expand Up @@ -2544,7 +2540,6 @@ impl Target {
key!(no_default_libraries, bool);
key!(position_independent_executables, bool);
key!(static_position_independent_executables, bool);
key!(needs_plt, bool);
key!(relro_level, RelroLevel)?;
key!(archive_format);
key!(allow_asm, bool);
Expand Down Expand Up @@ -2798,7 +2793,6 @@ impl ToJson for Target {
target_option_val!(no_default_libraries);
target_option_val!(position_independent_executables);
target_option_val!(static_position_independent_executables);
target_option_val!(needs_plt);
target_option_val!(relro_level);
target_option_val!(archive_format);
target_option_val!(allow_asm);
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_target/src/spec/x86_64_unknown_linux_gnux32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ pub fn target() -> Target {
base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-mx32"]);
base.stack_probes = StackProbeType::X86;
base.has_thread_local = false;
// BUG(GabrielMajeri): disabling the PLT on x86_64 Linux with x32 ABI
// breaks code gen. See LLVM bug 36743
base.needs_plt = true;

Target {
llvm_target: "x86_64-unknown-linux-gnux32".into(),
Expand Down
4 changes: 2 additions & 2 deletions tests/assembly/pic-relocation-model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ trait Sized {}
trait Copy {}

// CHECK-LABEL: call_other_fn:
// CHECK: {{(jmpq|callq)}} *other_fn@GOTPCREL(%rip)
// CHECK: {{(jmp|callq)}} other_fn@PLT
#[no_mangle]
pub fn call_other_fn() -> u8 {
unsafe {
Expand All @@ -23,7 +23,7 @@ pub fn call_other_fn() -> u8 {
}

// CHECK-LABEL: other_fn:
// CHECK: callq *foreign_fn@GOTPCREL(%rip)
// CHECK: callq foreign_fn@PLT
#[no_mangle]
#[inline(never)]
pub fn other_fn() -> u8 {
Expand Down
4 changes: 1 addition & 3 deletions tests/assembly/pie-relocation-model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ pub fn call_other_fn() -> u8 {
}

// CHECK-LABEL: other_fn:
// External functions are still called through GOT, since we don't know if the symbol
// is defined in the binary or in the shared library.
// CHECK: callq *foreign_fn@GOTPCREL(%rip)
// CHECK: callq foreign_fn@PLT
#[no_mangle]
#[inline(never)]
pub fn other_fn() -> u8 {
Expand Down
4 changes: 1 addition & 3 deletions tests/rustdoc-ui/z-help.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@
-Z parse-only=val -- parse only; do not compile, assemble, or link (default: no)
-Z perf-stats=val -- print some performance-related statistics (default: no)
-Z pick-stable-methods-before-any-unstable=val -- try to pick stable methods first before picking any unstable methods (default: yes)
-Z plt=val -- whether to use the PLT when calling into shared libraries;
only has effect for PIC code on systems with ELF binaries
(default: PLT is disabled if full relro is enabled)
-Z plt=val -- if false, use GOT-generating code sequences for external function calls. This results in longer code sequences, but may avoid a PLT if the function is not bound locally. Only has effect on ELF systems (default: yes)
-Z polonius=val -- enable polonius-based borrow-checker (default: no)
-Z polymorphize=val -- perform polymorphization analysis
-Z pre-link-arg=val -- a single extra argument to prepend the linker invocation (can be used several times)
Expand Down

0 comments on commit 2b35723

Please sign in to comment.