From 5237e38fddf1699ea6186400c17c0a32b71fb836 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 24 Sep 2026 17:48:28 +0300 Subject: [PATCH 1/2] Support -Z merge-functions with gcc and add stack-protector asm tests This is a part of the stack-protector work. Adding support for merge-functions is not strictly needed, but it does prevent breakage if the GCC function merging logic changes. Not adding tests for -Z merge-functions since I don't see any such tests for the LLVM side of it. --- compiler/rustc_codegen_gcc/src/gcc_util.rs | 11 +- compiler/rustc_codegen_llvm/src/llvm_util.rs | 2 +- compiler/rustc_codegen_ssa/src/back/write.rs | 7 +- compiler/rustc_session/src/options.rs | 1 + compiler/rustc_session/src/session.rs | 10 +- src/bootstrap/src/core/build_steps/test.rs | 6 + .../builder/cli_paths/snapshots/x_test.snap | 3 + .../snapshots/x_test_skip_coverage.snap | 3 + .../snapshots/x_test_skip_coverage_map.snap | 3 + .../snapshots/x_test_skip_coverage_run.snap | 3 + .../snapshots/x_test_skip_tests_coverage.snap | 3 + .../cli_paths/snapshots/x_test_tests.snap | 3 + .../snapshots/x_test_tests_skip_coverage.snap | 3 + src/bootstrap/src/core/builder/mod.rs | 1 + src/bootstrap/src/core/builder/tests.rs | 2 + src/tools/compiletest/src/common.rs | 1 + src/tools/compiletest/src/runtest.rs | 3 +- .../stack-protector-heuristics-effect.rs | 331 ++++++++++++++++++ 18 files changed, 384 insertions(+), 12 deletions(-) create mode 100644 tests/assembly-gcc/stack-protector/stack-protector-heuristics-effect.rs diff --git a/compiler/rustc_codegen_gcc/src/gcc_util.rs b/compiler/rustc_codegen_gcc/src/gcc_util.rs index 0628171e488b3..4850a5fc5ca31 100644 --- a/compiler/rustc_codegen_gcc/src/gcc_util.rs +++ b/compiler/rustc_codegen_gcc/src/gcc_util.rs @@ -9,7 +9,9 @@ use rustc_codegen_ssa::target_features; use rustc_data_structures::smallvec::{SmallVec, smallvec}; use rustc_session::config::NATIVE_CPU; use rustc_session::{EarlySession, Session}; -use rustc_target::spec::{Arch, RelocModel, StackProbeType, StackProtector, Target}; +use rustc_target::spec::{ + Arch, MergeFunctions, RelocModel, StackProbeType, StackProtector, Target, +}; fn gcc_features_by_flags(sess: &EarlySession, features: &mut Vec) { target_features::retpoline_features_by_flags(sess, features); @@ -220,6 +222,13 @@ pub fn new_context<'gcc>(sess: &Session) -> Context<'gcc> { StackProtector::None => (), } + match sess.merge_functions() { + MergeFunctions::Disabled | MergeFunctions::Trampolines => { + context.add_command_line_option("-fno-ipa-icf-functions"); + } + MergeFunctions::Aliases => {} + } + match sess.target.stack_probes { StackProbeType::None => (), StackProbeType::Inline | StackProbeType::InlineOrCall { .. } => { diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 9e37205f230ac..9cfa74ffd019d 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -120,7 +120,7 @@ unsafe fn configure_llvm(sess: &EarlySession) { add("-generate-arange-section", false); } - match sess.opts.unstable_opts.merge_functions.unwrap_or(sess.target.merge_functions) { + match sess.merge_functions() { MergeFunctions::Disabled | MergeFunctions::Trampolines => {} MergeFunctions::Aliases => { add("-mergefunc-use-aliases", false); diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 147e4c43c7103..f87bfa34ddc8b 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -244,12 +244,7 @@ impl ModuleConfig { // backends (again, NVPTX). Therefore, allow targets to opt out of // the MergeFunctions pass, but otherwise keep the pass enabled (at // O2 and O3) since it can be useful for reducing code size. - merge_functions: match sess - .opts - .unstable_opts - .merge_functions - .unwrap_or(sess.target.merge_functions) - { + merge_functions: match sess.merge_functions() { MergeFunctions::Disabled => false, MergeFunctions::Trampolines | MergeFunctions::Aliases => { use config::OptLevel::*; diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 13fc598d86bfa..61b7d792d9a08 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -2643,6 +2643,7 @@ options! { maximal_hir_to_mir_coverage: bool = (false, parse_bool, [TRACKED], "save as much information as possible about the correspondence between MIR and HIR \ as source scopes (default: no)"), + #[rustc_lint_opt_deny_field_access("use `Session::merge_functions` instead of this field")] merge_functions: Option = (None, parse_merge_functions, [TRACKED], "control the operation of the MergeFunctions LLVM pass, taking \ the same values as the target option of the same name"), diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 3b60a13b8e6f2..060e614e82180 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -28,9 +28,9 @@ use rustc_span::{RealFileName, Span, Symbol}; use rustc_structures::{CrateType, Limit}; use rustc_target::asm::InlineAsmArch; use rustc_target::spec::{ - Arch, CfgAbi, CodeModel, DebuginfoKind, Os, PanicStrategy, RelocModel, RelroLevel, - SanitizerSet, SmallDataThresholdSupport, SplitDebuginfo, StackProtector, SymbolVisibility, - Target, TargetTuple, TlsModel, apple, + Arch, CfgAbi, CodeModel, DebuginfoKind, MergeFunctions, Os, PanicStrategy, RelocModel, + RelroLevel, SanitizerSet, SmallDataThresholdSupport, SplitDebuginfo, StackProtector, + SymbolVisibility, Target, TargetTuple, TlsModel, apple, }; use crate::code_stats::CodeStats; @@ -423,6 +423,10 @@ impl EarlySession { .sanitizer .combine_with_defaults(self.target.options.default_sanitizers) } + + pub fn merge_functions(&self) -> MergeFunctions { + self.opts.unstable_opts.merge_functions.unwrap_or(self.target.merge_functions) + } } /// Some info about the backend, returned by `CodegenBackend::init` and put into the `Session`. diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index cd647eb32f515..3006fddbbe662 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2040,6 +2040,12 @@ test!(BuildStd { default: false }); +test!(AssemblyGcc { + path: "tests/assembly-gcc", + mode: CompiletestMode::Assembly, + suite: "assembly-gcc", + default: true +}); test!(AssemblyLlvm { path: "tests/assembly-llvm", mode: CompiletestMode::Assembly, diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test.snap index 32ed54050dc3d..787686b117cd2 100644 --- a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test.snap +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test.snap @@ -23,6 +23,9 @@ expression: test [Test] test::CodegenUnits targets: [aarch64-unknown-linux-gnu] - Suite(tests/codegen-units) +[Test] test::AssemblyGcc + targets: [aarch64-unknown-linux-gnu] + - Suite(tests/assembly-gcc) [Test] test::AssemblyLlvm targets: [aarch64-unknown-linux-gnu] - Suite(tests/assembly-llvm) diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage.snap index d1cdf81a14e08..879114ffc8170 100644 --- a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage.snap +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage.snap @@ -20,6 +20,9 @@ expression: test --skip=coverage [Test] test::CodegenUnits targets: [aarch64-unknown-linux-gnu] - Suite(tests/codegen-units) +[Test] test::AssemblyGcc + targets: [aarch64-unknown-linux-gnu] + - Suite(tests/assembly-gcc) [Test] test::AssemblyLlvm targets: [aarch64-unknown-linux-gnu] - Suite(tests/assembly-llvm) diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage_map.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage_map.snap index df4b8a2ce1299..a30e3a2407937 100644 --- a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage_map.snap +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage_map.snap @@ -23,6 +23,9 @@ expression: test --skip=coverage-map [Test] test::CodegenUnits targets: [aarch64-unknown-linux-gnu] - Suite(tests/codegen-units) +[Test] test::AssemblyGcc + targets: [aarch64-unknown-linux-gnu] + - Suite(tests/assembly-gcc) [Test] test::AssemblyLlvm targets: [aarch64-unknown-linux-gnu] - Suite(tests/assembly-llvm) diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage_run.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage_run.snap index 75992df8aa616..12e0a3233f7ea 100644 --- a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage_run.snap +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_coverage_run.snap @@ -23,6 +23,9 @@ expression: test --skip=coverage-run [Test] test::CodegenUnits targets: [aarch64-unknown-linux-gnu] - Suite(tests/codegen-units) +[Test] test::AssemblyGcc + targets: [aarch64-unknown-linux-gnu] + - Suite(tests/assembly-gcc) [Test] test::AssemblyLlvm targets: [aarch64-unknown-linux-gnu] - Suite(tests/assembly-llvm) diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_tests_coverage.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_tests_coverage.snap index a44052fac41f9..3b07718b8aecf 100644 --- a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_tests_coverage.snap +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_skip_tests_coverage.snap @@ -20,6 +20,9 @@ expression: test --skip=tests/coverage [Test] test::CodegenUnits targets: [aarch64-unknown-linux-gnu] - Suite(tests/codegen-units) +[Test] test::AssemblyGcc + targets: [aarch64-unknown-linux-gnu] + - Suite(tests/assembly-gcc) [Test] test::AssemblyLlvm targets: [aarch64-unknown-linux-gnu] - Suite(tests/assembly-llvm) diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests.snap index c0105510ae416..6db9443b8543c 100644 --- a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests.snap +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests.snap @@ -20,6 +20,9 @@ expression: test tests [Test] test::CodegenUnits targets: [aarch64-unknown-linux-gnu] - Suite(tests/codegen-units) +[Test] test::AssemblyGcc + targets: [aarch64-unknown-linux-gnu] + - Suite(tests/assembly-gcc) [Test] test::AssemblyLlvm targets: [aarch64-unknown-linux-gnu] - Suite(tests/assembly-llvm) diff --git a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_skip_coverage.snap b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_skip_coverage.snap index d1a90ca426918..ab4e9e6f7d997 100644 --- a/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_skip_coverage.snap +++ b/src/bootstrap/src/core/builder/cli_paths/snapshots/x_test_tests_skip_coverage.snap @@ -17,6 +17,9 @@ expression: test tests --skip=coverage [Test] test::CodegenUnits targets: [aarch64-unknown-linux-gnu] - Suite(tests/codegen-units) +[Test] test::AssemblyGcc + targets: [aarch64-unknown-linux-gnu] + - Suite(tests/assembly-gcc) [Test] test::AssemblyLlvm targets: [aarch64-unknown-linux-gnu] - Suite(tests/assembly-llvm) diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index 0593f30946320..c221a976b2698 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -886,6 +886,7 @@ impl<'a> Builder<'a> { test::MirOpt, test::CodegenLlvm, test::CodegenUnits, + test::AssemblyGcc, test::AssemblyLlvm, test::Incremental, test::Debuginfo, diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index cc6ac39672950..dd1beb65e7fe0 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -1943,6 +1943,7 @@ mod snapshot { [test] compiletest-mir-opt 1 [test] compiletest-codegen-llvm 1 [test] compiletest-codegen-units 1 + [test] compiletest-assembly-gcc 1 [test] compiletest-assembly-llvm 1 [test] compiletest-incremental 1 [test] compiletest-debuginfo 1 @@ -2124,6 +2125,7 @@ mod snapshot { [test] compiletest-mir-opt 2 [test] compiletest-codegen-llvm 2 [test] compiletest-codegen-units 2 + [test] compiletest-assembly-gcc 2 [test] compiletest-assembly-llvm 2 [test] compiletest-incremental 2 [test] compiletest-debuginfo 2 diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 8c580d3520dfd..1968f5eb73d4e 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -60,6 +60,7 @@ impl TestMode { string_enum! { #[derive(Clone, Copy, PartialEq, Debug)] pub(crate) enum TestSuite { + AssemblyGcc => "assembly-gcc", AssemblyLlvm => "assembly-llvm", CodegenLlvm => "codegen-llvm", CodegenUnits => "codegen-units", diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index c728e56cf639d..0bb8a7003589c 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1604,7 +1604,8 @@ impl<'test> TestCx<'test> { // Exhaustively match all other suites. // Note that some suites never actually use this method, so the // return value for those suites is not necessarily meaningful. - TestSuite::AssemblyLlvm + TestSuite::AssemblyGcc + | TestSuite::AssemblyLlvm | TestSuite::BuildStd | TestSuite::CodegenLlvm | TestSuite::CodegenUnits diff --git a/tests/assembly-gcc/stack-protector/stack-protector-heuristics-effect.rs b/tests/assembly-gcc/stack-protector/stack-protector-heuristics-effect.rs new file mode 100644 index 0000000000000..f9ddea2afedbd --- /dev/null +++ b/tests/assembly-gcc/stack-protector/stack-protector-heuristics-effect.rs @@ -0,0 +1,331 @@ +//@ revisions: all strong basic none missing +//@ assembly-output: emit-asm +//@ ignore-apple slightly different policy on stack protection of arrays +//@ ignore-msvc stack check code uses different function names +//@ ignore-nvptx64 stack protector is not supported +//@ ignore-wasm32-unknown-unknown +//@ [all] compile-flags: -Z stack-protector=all +//@ [strong] compile-flags: -Z stack-protector=strong +//@ [basic] compile-flags: -Z stack-protector=basic +//@ [none] compile-flags: -Z stack-protector=none +//@ compile-flags: -C opt-level=2 -Z merge-functions=disabled + +#![crate_type = "lib"] +#![allow(internal_features)] +#![feature(unsized_fn_params)] + +// CHECK-LABEL: "emptyfn" +#[no_mangle] +pub fn emptyfn() { + // all: __stack_chk_fail + // strong-NOT: __stack_chk_fail + // basic-NOT: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// CHECK-LABEL: "array_char" +#[no_mangle] +pub fn array_char(f: fn(*const char)) { + let a = ['c'; 1]; + let b = ['d'; 3]; + let c = ['e'; 15]; + + f(&a as *const _); + f(&b as *const _); + f(&c as *const _); + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// CHECK-LABEL: "array_u8_1" +#[no_mangle] +pub fn array_u8_1(f: fn(*const u8)) { + let a = [0u8; 1]; + f(&a as *const _); + + // The 'strong' heuristic adds stack protection to functions with local + // array variables regardless of their size. + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic-NOT: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// CHECK-LABEL: "array_u8_small" +#[no_mangle] +pub fn array_u8_small(f: fn(*const u8)) { + let a = [0u8; 2]; + let b = [0u8; 7]; + f(&a as *const _); + f(&b as *const _); + + // Small arrays do not lead to stack protection by the 'basic' heuristic. + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic-NOT: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// CHECK-LABEL: "array_u8_large" +#[no_mangle] +pub fn array_u8_large(f: fn(*const u8)) { + let a = [0u8; 9]; + f(&a as *const _); + + // Since `a` is a byte array with size greater than 8, the basic heuristic + // will also protect this function. + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +#[derive(Copy, Clone)] +pub struct ByteSizedNewtype(u8); + +// CHECK-LABEL: "array_bytesizednewtype_9" +#[no_mangle] +pub fn array_bytesizednewtype_9(f: fn(*const ByteSizedNewtype)) { + let a = [ByteSizedNewtype(0); 9]; + f(&a as *const _); + + // Since `a` is a byte array in the GCC output, the basic heuristic will + // also protect this function. + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// CHECK-LABEL: "local_var_addr_used_indirectly" +#[no_mangle] +pub fn local_var_addr_used_indirectly(f: fn(bool)) { + let a = 5; + let a_addr = &a as *const _ as usize; + f(a_addr & 0x10 == 0); + + // This function takes the address of a local variable taken. Although this + // address is never used as a way to refer to stack memory, the `strong` + // heuristic adds stack smash protection. This is also the case in C++: + // ``` + // cat << EOF | g++ -O2 -fstack-protector-strong -S -x c++ - -o - | grep stack_chk + // #include + // void f(void (*g)(bool)) { + // int32_t x; + // g((reinterpret_cast(&x) & 0x10U) == 0); + // } + // EOF + // ``` + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic-NOT: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// CHECK-LABEL: "local_string_addr_taken" +#[no_mangle] +pub fn local_string_addr_taken(f: fn(&String)) { + let x = String::new(); + f(&x); + + // Taking the address of the local variable `x` leads to stack smash + // protection. It does not matter that the reference is not mut. + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +pub trait SelfByRef { + fn f(&self) -> i32; +} + +impl SelfByRef for i32 { + fn f(&self) -> i32 { + return self + 1; + } +} + +// CHECK-LABEL: "local_var_addr_taken_used_locally_only" +#[no_mangle] +pub fn local_var_addr_taken_used_locally_only(factory: fn() -> i32, sink: fn(i32)) { + let x = factory(); + let g = x.f(); + sink(g); + + // Even though the local variable conceptually has its address taken, as + // it's passed by reference to the trait function, the use of the reference + // is easily inlined. There is therefore no stack smash protection even with + // the `strong` heuristic. + + // all: __stack_chk_fail + // strong-NOT: __stack_chk_fail + // basic-NOT: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +pub struct Gigastruct { + does: u64, + not: u64, + have: u64, + array: u64, + members: u64, +} + +// CHECK-LABEL: "local_large_var_moved" +#[no_mangle] +pub fn local_large_var_moved(f: fn(Gigastruct)) { + let x = Gigastruct { does: 0, not: 1, have: 2, array: 3, members: 4 }; + f(x); + + // Even though the local variable conceptually doesn't have its address + // taken, it's so large that the "move" is implemented with a reference to a + // stack-local variable in the ABI. Consequently, this function *is* + // protected. This is also the case for rvalue-references in C++, + // regardless of struct size: + // ``` + // cat < + // #include + // void f(void (*g)(uint64_t&&)) { + // uint64_t x; + // g(std::move(x)); + // } + // EOF + // ``` + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// CHECK-LABEL: "local_large_var_cloned" +#[no_mangle] +pub fn local_large_var_cloned(f: fn(Gigastruct)) { + f(Gigastruct { does: 0, not: 1, have: 2, array: 3, members: 4 }); + + // A new instance of `Gigastruct` is passed to `f()`, without any apparent + // connection to this stack frame. Still, since instances of `Gigastruct` + // are sufficiently large, it is allocated in the caller stack frame and + // passed as a pointer. As such, this function is *also* protected, just + // like `local_large_var_moved`. + // + // This matches clang++ behavior, but not g++ behavior. + // + // In any case, both options are fine from a specification point of view, there + // is no "user-accessible pointer", and there is no strong reason to avoid generating + // a canary in this case, since it doesn't seem to be one of the performance-critical + // cases in which avoiding generating a canary is important. + // + // ``` + // cat < + // #include + // struct Gigastruct { uint64_t a, b, c, d, e; }; + // void f(void (*g)(Gigastruct)) { + // g(Gigastruct{}); + // } + // EOF + // ``` + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +extern "C" { + // Difference between LLVM and GCC: LLVM will not generate stack protection + // for "external" calls to alloca, but gcc will. See the matching test for + // stack-protector-heuristics-effect under assembly-llvm. + // + // This is a difference in heuristics and therefore fine. + // + // Check that rustc_codegen_gcc matches gcc behavior. + + fn alloca(size: usize) -> *mut (); +} + +// CHECK-LABEL: "alloca_small_compile_time_constant_arg" +#[no_mangle] +pub fn alloca_small_compile_time_constant_arg(f: fn(*mut ())) { + f(unsafe { alloca(8) }); + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// CHECK-LABEL: "alloca_large_compile_time_constant_arg" +#[no_mangle] +pub fn alloca_large_compile_time_constant_arg(f: fn(*mut ())) { + f(unsafe { alloca(9) }); + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// CHECK-LABEL: "alloca_dynamic_arg" +#[no_mangle] +pub fn alloca_dynamic_arg(f: fn(*mut ()), n: usize) { + f(unsafe { alloca(n) }); + + // all: __stack_chk_fail + // strong: __stack_chk_fail + // basic: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} + +// The question then is: in what ways can Rust code generate array-`alloca` +// GCC instructions? This appears to only be generated by +// rustc_codegen_ssa::traits::Builder::array_alloca() through +// rustc_codegen_ssa::mir::operand::OperandValue::store_unsized(). FWICT +// this is support for the "unsized locals" unstable feature: +// https://doc.rust-lang.org/unstable-book/language-features/unsized-locals.html. + +// CHECK-LABEL: "unsized_fn_param" +#[no_mangle] +pub fn unsized_fn_param(s: [u8], l: bool, f: fn([u8])) { + let n = if l { 1 } else { 2 }; + f(*Box::<[u8]>::from(&s[0..n])); // slice-copy with Box::from + + // Even though slices are conceptually passed by-value both into this + // function and into `f()`, this is implemented with pass-by-reference + // using a suitably constructed fat-pointer (as if the functions + // accepted &[u8]). This function therefore doesn't need dynamic array + // alloca, and is therefore not protected by the `strong` or `basic` + // heuristics. + + // all: __stack_chk_fail + // strong-NOT: __stack_chk_fail + // basic-NOT: __stack_chk_fail + // none-NOT: __stack_chk_fail + // missing-NOT: __stack_chk_fail +} From c6dc51196d5d387cf44837e9d75212462fb07e4f Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 24 Sep 2026 22:01:39 +0300 Subject: [PATCH 2/2] address comments by bjorn3 --- compiler/rustc_codegen_gcc/src/gcc_util.rs | 7 ++-- compiler/rustc_target/src/spec/mod.rs | 7 ++++ .../stack-protector-heuristics-effect.rs | 32 +++---------------- .../stack-protector-heuristics-effect.rs | 29 ++--------------- 4 files changed, 20 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/gcc_util.rs b/compiler/rustc_codegen_gcc/src/gcc_util.rs index 4850a5fc5ca31..90d08d13a080a 100644 --- a/compiler/rustc_codegen_gcc/src/gcc_util.rs +++ b/compiler/rustc_codegen_gcc/src/gcc_util.rs @@ -223,10 +223,13 @@ pub fn new_context<'gcc>(sess: &Session) -> Context<'gcc> { } match sess.merge_functions() { - MergeFunctions::Disabled | MergeFunctions::Trampolines => { + MergeFunctions::Disabled => { context.add_command_line_option("-fno-ipa-icf-functions"); } - MergeFunctions::Aliases => {} + // GCC always merges functions with trampolines rather than aliases, so we don't + // need to differentiate MergeFunctions::Trampolines and MergeFunctions::Aliases + // as trampolines are allowed either way. + MergeFunctions::Trampolines | MergeFunctions::Aliases => {} } match sess.target.stack_probes { diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 42d817c7daa31..2d7cd049eb61c 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -927,8 +927,15 @@ impl ToJson for SmallDataThresholdSupport { crate::target_spec_enum! { pub enum MergeFunctions { + /// Disable function merging entirely. Disabled = "disabled", + /// Allow function merging via trampolines (i.e., functions can be implemented + /// in terms of a jump to an unrelated function with identical behavior), but + /// not via object-format-level global aliases. Trampolines = "trampolines", + /// (Default) allow function merging via either trampolines or (object format + /// level) global aliases. LLVM will normally use global aliases for merging, but + /// trampolines are also permissible. Aliases = "aliases", } diff --git a/tests/assembly-gcc/stack-protector/stack-protector-heuristics-effect.rs b/tests/assembly-gcc/stack-protector/stack-protector-heuristics-effect.rs index f9ddea2afedbd..c8945ed96b3ee 100644 --- a/tests/assembly-gcc/stack-protector/stack-protector-heuristics-effect.rs +++ b/tests/assembly-gcc/stack-protector/stack-protector-heuristics-effect.rs @@ -235,7 +235,8 @@ pub fn local_large_var_cloned(f: fn(Gigastruct)) { // In any case, both options are fine from a specification point of view, there // is no "user-accessible pointer", and there is no strong reason to avoid generating // a canary in this case, since it doesn't seem to be one of the performance-critical - // cases in which avoiding generating a canary is important. + // cases in which avoiding generating a canary is important, so it seems that + // rustc should keep the clang-like behavior of generating a canary here. // // ``` // cat <::from(&s[0..n])); // slice-copy with Box::from - - // Even though slices are conceptually passed by-value both into this - // function and into `f()`, this is implemented with pass-by-reference - // using a suitably constructed fat-pointer (as if the functions - // accepted &[u8]). This function therefore doesn't need dynamic array - // alloca, and is therefore not protected by the `strong` or `basic` - // heuristics. - - // all: __stack_chk_fail - // strong-NOT: __stack_chk_fail - // basic-NOT: __stack_chk_fail - // none-NOT: __stack_chk_fail - // missing-NOT: __stack_chk_fail -} +// rustc can currently (as of 1.98) not generate variable-sized allocas, except for +// variable-sized scalable vector types, so their interaction with stack-protector +// does not need to be tested. diff --git a/tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect.rs b/tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect.rs index 3728ff3adf105..56cf0035e25bf 100644 --- a/tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect.rs +++ b/tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect.rs @@ -317,29 +317,6 @@ pub fn alloca_dynamic_arg(f: fn(*mut ()), n: usize) { // missing-NOT: __stack_chk_fail } -// The question then is: in what ways can Rust code generate array-`alloca` -// LLVM instructions? This appears to only be generated by -// rustc_codegen_ssa::traits::Builder::array_alloca() through -// rustc_codegen_ssa::mir::operand::OperandValue::store_unsized(). FWICT -// this is support for the "unsized locals" unstable feature: -// https://doc.rust-lang.org/unstable-book/language-features/unsized-locals.html. - -// CHECK-LABEL: unsized_fn_param{{:|\[}} -#[no_mangle] -pub fn unsized_fn_param(s: [u8], l: bool, f: fn([u8])) { - let n = if l { 1 } else { 2 }; - f(*Box::<[u8]>::from(&s[0..n])); // slice-copy with Box::from - - // Even though slices are conceptually passed by-value both into this - // function and into `f()`, this is implemented with pass-by-reference - // using a suitably constructed fat-pointer (as if the functions - // accepted &[u8]). This function therefore doesn't need dynamic array - // alloca, and is therefore not protected by the `strong` or `basic` - // heuristics. - - // all: __stack_chk_fail - // strong-NOT: __stack_chk_fail - // basic-NOT: __stack_chk_fail - // none-NOT: __stack_chk_fail - // missing-NOT: __stack_chk_fail -} +// rustc can currently (as of 1.98) not generate variable-sized allocas, except for +// variable-sized scalable vector types, so their interaction with stack-protector +// does not need to be tested.