Stabilize -Zstack-protector as -Cstack-protector

I propose stabilizing `-Cstack-protector` as `-Zstack-protector`. This PR adds a new `-Cstack-protector` flag, leaving the unstable `-Z` flag as is to ease the transition period. The `-Z` flag will be removed in the future.

No RFC/MCP, this flag was added in 84197 and was not deemed large enough to require additional process.

The tracking issue for this feature is 114903.

The `-Cstack-protector=strong` mode uses the same underlying heuristics as Clang's `-fstack-protector-strong`.
These heuristics weren't designed for Rust, and may be over-conservative in some cases - for example, if
Rust stores a field's data in an alloca using an LLVM array type, LLVM regard the alloca as meaning
that the function has a C array, and enable stack overflow canaries even if the function accesses
the alloca in a safe way. Some people thought we should wait on stabilization until there are better
heuristics, but I didn't hear about any concrete case where this unduly harms performance, and I think
that when a need comes, we can improve the heuristics in LLVM after stabilization.

The heuristics do seem to not be under-conservative, so this should not be a security risk.

The `-Cstack-protector=basic` mode (`-fstack-protector`) uses heuristics that are specifically designed
to catch old-C-style string manipulation. This is not a good fit to Rust, which does not perform much
unsafe C-style string manipulation. As far as I can tell, nobody has been asking for it,
and few people are using it even in today's C - modern distros (e.g. [Debian]) tend to use
`-fstack-protector-strong`.

Therefore, `-Cstack-protector=basic` has been **removed**. If anyone is interested in it, they
are welcome to add it back as an unstable option.

[Debian]: https://wiki.debian.org/Hardening#DEB_BUILD_HARDENING_STACKPROTECTOR_.28gcc.2Fg.2B-.2B-_-fstack-protector-strong.29

Most implementation was done in <#84197>. The command-line
attribute enables the relevant LLVM attribute on all functions in
<https://github.com/rust-lang/rust/blob/68baa87ba6f03f8b6af2a368690161f1601e4040/compiler/rustc_codegen_llvm/src/attributes.rs#L267-L276>.

Each target can indicate that it does not support stack canaries - currently,
the GPU platforms `nvptx64-nvidia-cuda` and `amdgcn-amd-amdhsa`. On these
platforms, use of `-Cstack-protector` causes an error.

The feature has tests that make sure that the LLVM heuristic gives reasonable
results for several functions, by checking for `__security_check_cookie` (on Windows)
or `__stack_chk_fail` (on Linux). See
<https://github.com/rust-lang/rust/tree/68baa87ba6f03f8b6af2a368690161f1601e4040/tests/assembly-llvm/stack-protector>

No call-for-testing has been conducted, but the feature seems to be in use.

No reported bugs seem to exist.

- bbjornse was the original implementor at 84197
- mrcnski documented it at 111722
- wesleywiser added tests for Windows at 116037
- davidtwco worked on the feature at 121742
- nikic provided support from the LLVM side (on Zulip on <https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841> and elsewhere),
  thanks nikic!

No FIXMEs related to this feature.

This feature cannot cause undefined behavior.

No changes to reference/spec, docs added to the codegen docs as part of the stabilization PR.

No.

None.

No support needed for rustdoc, clippy, rust-analyzer, rustfmt or rustup.

Cargo could expose this as an option in build profiles but I would expect the decision as to what version should be used would
be made for the entire crate graph at build time rather than by individual package authors.

`-C stack-protector` is propagated to C compilers using cc-rs via rust-lang/cc-rs issue 1550

Author: Ariel Ben-Yehuda

bootstrap.example.toml

@@ -760,10 +760,9 @@
 #rust.frame-pointers = false
 
 # Indicates whether stack protectors should be used
-# via the unstable option `-Zstack-protector`.
+# via `-Cstack-protector`.
 #
-# Valid options are : `none`(default),`basic`,`strong`, or `all`.
-# `strong` and `basic` options may be buggy and are not recommended, see rust-lang/rust#114903.
+# Valid options are : `none`(default), `strong`, or `all`.
 #rust.stack-protector = "none"
 
 # Prints each test name as it is executed, to help debug issues in the test harness itself.

compiler/rustc_codegen_llvm/src/attributes.rs

@@ -306,7 +306,6 @@ fn stackprotector_attr<'ll>(cx: &SimpleCx<'ll>, sess: &Session) -> Option<&'ll A
         StackProtector::None => return None,
         StackProtector::All => AttributeKind::StackProtectReq,
         StackProtector::Strong => AttributeKind::StackProtectStrong,
-        StackProtector::Basic => AttributeKind::StackProtect,
     };
 
     Some(sspattr.create_attr(cx.llcx))

compiler/rustc_codegen_llvm/src/lib.rs

@@ -281,18 +281,19 @@ impl CodegenBackend for LlvmCodegenBackend {
         Generate stack canaries in all functions.
 
     strong
-        Generate stack canaries in a function if it either:
-        - has a local variable of `[T; N]` type, regardless of `T` and `N`
-        - takes the address of a local variable.
-
-          (Note that a local variable being borrowed is not equivalent to its
-          address being taken: e.g. some borrows may be removed by optimization,
-          while by-value argument passing may be implemented with reference to a
-          local stack variable in the ABI.)
-
-    basic
-        Generate stack canaries in functions with local variables of `[T; N]`
-        type, where `T` is byte-sized and `N` >= 8.
+        Generate stack canaries for all functions, unless the compiler
+        can prove these functions can't be the source of a stack
+        buffer overflow (even in the presence of undefined behavior).
+
+        This provides similar security guarantees to Clang's
+        `-fstack-protector-strong`.
+
+        The exact rules are unstable and subject to change, but
+        currently, it generates stack protectors for functions that,
+        *post-optimization*, contain LLVM allocas (which
+        include all stack allocations - including fixed-size
+        allocations - that are used in a way that is not completely
+        determined by static control flow).
 
     none
         Do not generate stack canaries.

compiler/rustc_interface/src/tests.rs

@@ -662,6 +662,7 @@ fn test_codegen_options_tracking_hash() {
     tracked!(relocation_model, Some(RelocModel::Pic));
     tracked!(relro_level, Some(RelroLevel::Full));
     tracked!(split_debuginfo, Some(SplitDebuginfo::Packed));
+    tracked!(stack_protector, Some(StackProtector::All));
     tracked!(symbol_mangling_version, Some(SymbolManglingVersion::V0));
     tracked!(target_cpu, Some(String::from("abc")));
     tracked!(target_feature, String::from("all the features, all of them"));
@@ -892,7 +893,7 @@ fn test_unstable_options_tracking_hash() {
     tracked!(small_data_threshold, Some(16));
     tracked!(split_lto_unit, Some(true));
     tracked!(src_hash_algorithm, Some(SourceFileHashAlgorithm::Sha1));
-    tracked!(stack_protector, StackProtector::All);
+    tracked!(stack_protector, Some(StackProtector::All));
     tracked!(staticlib_hide_internal_symbols, true);
     tracked!(teach, true);
     tracked!(thinlto, Some(true));

compiler/rustc_session/src/errors.rs

@@ -364,7 +364,7 @@ pub(crate) struct EmbedSourceRequiresDebugInfo;
 
 #[derive(Diagnostic)]
 #[diag(
-    "`-Z stack-protector={$stack_protector}` is not supported for target {$target_triple} and will be ignored"
+    "`-C stack-protector={$stack_protector}` is not supported for target {$target_triple} and will be ignored"
 )]
 pub(crate) struct StackProtectorNotSupportedForTarget<'a> {
     pub(crate) stack_protector: StackProtector,

compiler/rustc_session/src/options.rs

@@ -828,8 +828,7 @@ mod desc {
     pub(crate) const parse_polonius: &str = "either no value or `legacy` (the default), or `next`";
     pub(crate) const parse_annotate_moves: &str =
         "either a boolean (`yes`, `no`, `on`, `off`, etc.), or a size limit in bytes";
-    pub(crate) const parse_stack_protector: &str =
-        "one of (`none` (default), `basic`, `strong`, or `all`)";
+    pub(crate) const parse_stack_protector: &str = "one of (`none` (default), `strong`, or `all`)";
     pub(crate) const parse_branch_protection: &str = "a `,` separated combination of `bti`, `gcs`, `pac-ret`, (optionally with `pc`, `b-key`, `leaf` if `pac-ret` is set)";
     pub(crate) const parse_proc_macro_execution_strategy: &str =
         "one of supported execution strategies (`same-thread`, or `cross-thread`)";
@@ -1879,9 +1878,12 @@ pub mod parse {
         true
     }
 
-    pub(crate) fn parse_stack_protector(slot: &mut StackProtector, v: Option<&str>) -> bool {
+    pub(crate) fn parse_stack_protector(
+        slot: &mut Option<StackProtector>,
+        v: Option<&str>,
+    ) -> bool {
         match v.and_then(|s| StackProtector::from_str(s).ok()) {
-            Some(ssp) => *slot = ssp,
+            Some(ssp) => *slot = Some(ssp),
             _ => return false,
         }
         true
@@ -2185,6 +2187,9 @@ options! {
     #[rustc_lint_opt_deny_field_access("use `Session::split_debuginfo` instead of this field")]
     split_debuginfo: Option<SplitDebuginfo> = (None, parse_split_debuginfo, [TRACKED],
         "how to handle split-debuginfo, a platform-specific option"),
+    #[rustc_lint_opt_deny_field_access("use `Session::stack_protector` instead of this field")]
+    stack_protector: Option<StackProtector> = (None, parse_stack_protector, [TRACKED] { MITIGATION: StackProtector },
+        "control stack smashing protection strategy (`rustc --print stack-protector-strategies` for details)"),
     strip: Strip = (Strip::None, parse_strip, [UNTRACKED],
         "tell the linker which information to strip (`none` (default), `debuginfo` or `symbols`)"),
     symbol_mangling_version: Option<SymbolManglingVersion> = (None,
@@ -2692,8 +2697,8 @@ written to standard error output)"),
     src_hash_algorithm: Option<SourceFileHashAlgorithm> = (None, parse_src_file_hash, [TRACKED],
         "hash algorithm of source files in debug info (`md5`, `sha1`, or `sha256`)"),
     #[rustc_lint_opt_deny_field_access("use `Session::stack_protector` instead of this field")]
-    stack_protector: StackProtector = (StackProtector::None, parse_stack_protector, [TRACKED] { MITIGATION: StackProtector },
-        "control stack smash protection strategy (`rustc --print stack-protector-strategies` for details)"),
+    stack_protector: Option<StackProtector> = (None, parse_stack_protector, [TRACKED] { MITIGATION: StackProtector },
+        "control stack smashing protection strategy (`rustc --print stack-protector-strategies` for details)"),
     staticlib_allow_rdylib_deps: bool = (false, parse_bool, [TRACKED],
         "allow staticlibs to have rust dylib dependencies"),
     staticlib_hide_internal_symbols: bool = (false, parse_bool, [TRACKED],

compiler/rustc_session/src/options/mitigation_coverage.rs

@@ -20,7 +20,6 @@ impl DeniedPartialMitigationLevel {
     pub fn level_str(&self) -> &'static str {
         match self {
             DeniedPartialMitigationLevel::StackProtector(StackProtector::All) => "=all",
-            DeniedPartialMitigationLevel::StackProtector(StackProtector::Basic) => "=basic",
             DeniedPartialMitigationLevel::StackProtector(StackProtector::Strong) => "=strong",
             // currently `=disabled` should not appear
             DeniedPartialMitigationLevel::Enabled(false) => "=disabled",
@@ -36,9 +35,6 @@ impl std::fmt::Display for DeniedPartialMitigationLevel {
             DeniedPartialMitigationLevel::StackProtector(StackProtector::All) => {
                 write!(f, "all")
             }
-            DeniedPartialMitigationLevel::StackProtector(StackProtector::Basic) => {
-                write!(f, "basic")
-            }
             DeniedPartialMitigationLevel::StackProtector(StackProtector::Strong) => {
                 write!(f, "strong")
             }
@@ -203,6 +199,7 @@ denied_partial_mitigations! {
     enum DeniedPartialMitigationKind {
         // The mitigation name should match the option name in rustc_session::options,
         // to allow for resetting the mitigation
+
         (StackProtector, "stack-protector", EditionFuture, self.stack_protector()),
         (ControlFlowGuard, "control-flow-guard", EditionFuture, self.opts.cg.control_flow_guard == CFGuard::Checks)
     }

compiler/rustc_session/src/session.rs

@@ -772,11 +772,12 @@ impl Session {
     }
 
     pub fn stack_protector(&self) -> StackProtector {
-        if self.target.options.supports_stack_protector {
-            self.opts.unstable_opts.stack_protector
-        } else {
-            StackProtector::None
-        }
+        // -C stack-protector overwrites -Z stack-protector, default to StackProtector::None
+        self.opts
+            .cg
+            .stack_protector
+            .or(self.opts.unstable_opts.stack_protector)
+            .unwrap_or(StackProtector::None)
     }
 
     pub fn must_emit_unwind_tables(&self) -> bool {
@@ -1287,10 +1288,10 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
         }
     }
 
-    if sess.opts.unstable_opts.stack_protector != StackProtector::None {
+    if sess.stack_protector() != StackProtector::None {
         if !sess.target.options.supports_stack_protector {
-            sess.dcx().emit_warn(errors::StackProtectorNotSupportedForTarget {
-                stack_protector: sess.opts.unstable_opts.stack_protector,
+            sess.dcx().emit_err(errors::StackProtectorNotSupportedForTarget {
+                stack_protector: sess.stack_protector(),
                 target_triple: &sess.opts.target_triple,
             });
         }

compiler/rustc_target/src/spec/mod.rs

@@ -1336,12 +1336,6 @@ crate::target_spec_enum! {
         /// Disable stack canary generation.
         None = "none",
 
-        /// On LLVM, mark all generated LLVM functions with the `ssp` attribute (see
-        /// llvm/docs/LangRef.rst). This triggers stack canary generation in
-        /// functions which contain an array of a byte-sized type with more than
-        /// eight elements.
-        Basic = "basic",
-
         /// On LLVM, mark all generated LLVM functions with the `sspstrong`
         /// attribute (see llvm/docs/LangRef.rst). This triggers stack canary
         /// generation in functions which either contain an array, or which take

src/bootstrap/src/core/builder/cargo.rs

@@ -967,7 +967,7 @@ impl Builder<'_> {
         cargo.env(profile_var("STRIP"), self.config.rust_strip.to_string());
 
         if let Some(stack_protector) = &self.config.rust_stack_protector {
-            rustflags.arg(&format!("-Zstack-protector={stack_protector}"));
+            rustflags.arg(&format!("-Cstack-protector={stack_protector}"));
         }
 
         let debuginfo_level = match mode {