From 791ee45beb81238894285ef10d5f6c441b87b764 Mon Sep 17 00:00:00 2001 From: Kajetan Puchalski Date: Fri, 1 Nov 2024 16:29:48 +0000 Subject: [PATCH] build: Inherit flags from rustc Where applicable, detect which RUSTFLAGS were set for rustc and convert them into their corresponding cc flags in order to ensure consistent codegen across Rust and non-Rust modules. --- src/flags.rs | 476 +++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 45 ++++- tests/rustflags.rs | 29 +++ 3 files changed, 548 insertions(+), 2 deletions(-) create mode 100644 src/flags.rs create mode 100644 tests/rustflags.rs diff --git a/src/flags.rs b/src/flags.rs new file mode 100644 index 00000000..f4b21260 --- /dev/null +++ b/src/flags.rs @@ -0,0 +1,476 @@ +use crate::target::TargetInfo; +use crate::{Build, Error, ErrorKind, ToolFamily}; +use std::borrow::Cow; +use std::ffi::OsString; +use std::path::PathBuf; + +#[derive(Debug, PartialEq, Default)] +pub struct RustcCodegenFlags { + branch_protection: Option, + code_model: Option, + no_vectorize_loops: bool, + no_vectorize_slp: bool, + profile_generate: Option, + profile_use: Option, + control_flow_guard: Option, + lto: Option, + relocation_model: Option, + embed_bitcode: Option, + force_frame_pointers: Option, + link_dead_code: Option, + no_redzone: Option, + soft_float: Option, +} + +impl RustcCodegenFlags { + // Parse flags obtained from CARGO_ENCODED_RUSTFLAGS + pub fn parse(rustflags_env: &str) -> Result { + fn is_flag_prefix(flag: &str) -> bool { + match flag { + "-Z" | "-C" | "--codegen" | "-L" | "-l" | "-o" => true, + "-W" | "--warn" | "-A" | "--allow" | "-D" | "--deny" | "-F" | "--forbid" => true, + _ => false, + } + } + + fn handle_flag_prefix<'a>(prev: &'a str, curr: &'a str) -> Cow<'a, str> { + match prev { + "--codegen" | "-C" => Cow::from(format!("-C{}", curr)), + // Handle flags passed like --codegen=code-model=small + _ if curr.starts_with("--codegen=") => Cow::from(format!("-C{}", &curr[10..])), + "-Z" => Cow::from(format!("-Z{}", curr)), + "-L" | "-l" | "-o" => Cow::from(format!("{}{}", prev, curr)), + // Handle lint flags + "-W" | "--warn" => Cow::from(format!("-W{}", curr)), + "-A" | "--allow" => Cow::from(format!("-A{}", curr)), + "-D" | "--deny" => Cow::from(format!("-D{}", curr)), + "-F" | "--forbid" => Cow::from(format!("-F{}", curr)), + _ => Cow::from(curr), + } + } + + let mut codegen_flags = Self::default(); + + let mut prev_prefix = None; + for curr in rustflags_env.split("\u{1f}") { + let prev = if let Some(prev) = prev_prefix { + prev_prefix = None; + prev + } else { + // Do not process prefixes on their own + if is_flag_prefix(curr) { + prev_prefix = Some(curr); + continue; + } + "" + }; + + let rustc_flag = handle_flag_prefix(prev, curr); + codegen_flags.set_rustc_flag(&rustc_flag)?; + } + + Ok(codegen_flags) + } + + fn set_rustc_flag(&mut self, flag: &str) -> Result<(), Error> { + // Convert a textual representation of a bool-like rustc flag argument into an actual bool + fn arg_to_bool(arg: impl AsRef) -> Option { + match arg.as_ref() { + "y" | "yes" | "on" | "true" => Some(true), + "n" | "no" | "off" | "false" => Some(false), + _ => None, + } + } + + let (flag, value) = if let Some((flag, value)) = flag.split_once('=') { + (flag, Some(value.to_owned())) + } else { + (flag, None) + }; + + fn flag_ok_or(flag: Option, msg: &'static str) -> Result { + flag.ok_or(Error::new(ErrorKind::InvalidFlag, msg)) + } + + match flag { + // https://doc.rust-lang.org/rustc/codegen-options/index.html#code-model + "-Ccode-model" => { + self.code_model = Some(flag_ok_or(value, "-Ccode-model must have a value")?); + } + // https://doc.rust-lang.org/rustc/codegen-options/index.html#no-vectorize-loops + "-Cno-vectorize-loops" => self.no_vectorize_loops = true, + // https://doc.rust-lang.org/rustc/codegen-options/index.html#no-vectorize-slp + "-Cno-vectorize-slp" => self.no_vectorize_slp = true, + // https://doc.rust-lang.org/rustc/codegen-options/index.html#profile-generate + "-Cprofile-generate" => { + self.profile_generate = + Some(flag_ok_or(value, "-Cprofile-generate must have a value")?); + } + // https://doc.rust-lang.org/rustc/codegen-options/index.html#profile-use + "-Cprofile-use" => { + self.profile_use = Some(flag_ok_or(value, "-Cprofile-use must have a value")?); + } + // https://doc.rust-lang.org/rustc/codegen-options/index.html#control-flow-guard + "-Ccontrol-flow-guard" => self.control_flow_guard = value.or(Some("true".into())), + // https://doc.rust-lang.org/rustc/codegen-options/index.html#lto + "-Clto" => self.lto = value.or(Some("true".into())), + // https://doc.rust-lang.org/rustc/codegen-options/index.html#relocation-model + "-Crelocation-model" => { + self.relocation_model = + Some(flag_ok_or(value, "-Crelocation-model must have a value")?); + } + // https://doc.rust-lang.org/rustc/codegen-options/index.html#embed-bitcode + "-Cembed-bitcode" => self.embed_bitcode = value.map_or(Some(true), arg_to_bool), + // https://doc.rust-lang.org/rustc/codegen-options/index.html#force-frame-pointers + "-Cforce-frame-pointers" => { + self.force_frame_pointers = value.map_or(Some(true), arg_to_bool) + } + // https://doc.rust-lang.org/rustc/codegen-options/index.html#link-dead-code + "-Clink-dead-code" => self.link_dead_code = value.map_or(Some(true), arg_to_bool), + // https://doc.rust-lang.org/rustc/codegen-options/index.html#no-redzone + "-Cno-redzone" => self.no_redzone = value.map_or(Some(true), arg_to_bool), + // https://doc.rust-lang.org/rustc/codegen-options/index.html#soft-float + // Note: This flag is now deprecated in rustc. + "-Csoft-float" => self.soft_float = value.map_or(Some(true), arg_to_bool), + // https://doc.rust-lang.org/beta/unstable-book/compiler-flags/branch-protection.html + // FIXME: Drop the -Z variant and update the doc link once the option is stabilised + "-Zbranch-protection" | "-Cbranch-protection" => { + self.branch_protection = + Some(flag_ok_or(value, "-Zbranch-protection must have a value")?); + } + _ => {} + } + Ok(()) + } + + // Rust and clang/cc don't agree on what equivalent flags should look like. + pub fn cc_flags( + &self, + build: &Build, + path: &PathBuf, + family: ToolFamily, + target: &TargetInfo, + flags: &mut Vec, + ) { + // Push `flag` to `flags` if it is supported by the currently used CC + let mut push_if_supported = |flag: OsString| { + if build + .is_flag_supported_inner(&flag, path, target) + .unwrap_or(false) + { + flags.push(flag); + } else { + build.cargo_output.print_warning(&format!( + "Inherited flag {:?} is not supported by the currently used CC", + flag + )); + } + }; + + match family { + ToolFamily::Clang { .. } | ToolFamily::Gnu => { + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mbranch-protection + if let Some(value) = &self.branch_protection { + push_if_supported( + format!("-mbranch-protection={}", value.replace(",", "+")).into(), + ); + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mcmodel + if let Some(value) = &self.code_model { + push_if_supported(format!("-mcmodel={value}").into()); + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fno-vectorize + if self.no_vectorize_loops { + push_if_supported("-fno-vectorize".into()); + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fno-slp-vectorize + if self.no_vectorize_slp { + push_if_supported("-fno-slp-vectorize".into()); + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fprofile-generate + if let Some(value) = &self.profile_generate { + push_if_supported(format!("-fprofile-generate={value}").into()); + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fprofile-use + if let Some(value) = &self.profile_use { + push_if_supported(format!("-fprofile-use={value}").into()); + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mguard + if let Some(value) = &self.control_flow_guard { + let cc_val = match value.as_str() { + "y" | "yes" | "on" | "true" | "checks" => Some("cf"), + "nochecks" => Some("cf-nochecks"), + "n" | "no" | "off" | "false" => Some("none"), + _ => None, + }; + if let Some(cc_val) = cc_val { + push_if_supported(format!("-mguard={cc_val}").into()); + } + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-flto + if let Some(value) = &self.lto { + let cc_val = match value.as_str() { + "y" | "yes" | "on" | "true" | "fat" => Some("full"), + "thin" => Some("thin"), + _ => None, + }; + if let Some(cc_val) = cc_val { + push_if_supported(format!("-flto={cc_val}").into()); + } + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fPIC + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fPIE + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mdynamic-no-pic + if let Some(value) = &self.relocation_model { + let cc_flag = match value.as_str() { + "pic" => Some("-fPIC"), + "pie" => Some("-fPIE"), + "dynamic-no-pic" => Some("-mdynamic-no-pic"), + _ => None, + }; + if let Some(cc_flag) = cc_flag { + push_if_supported(cc_flag.into()); + } + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fembed-bitcode + if let Some(value) = &self.embed_bitcode { + let cc_val = if *value { "all" } else { "off" }; + push_if_supported(format!("-fembed-bitcode={cc_val}").into()); + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fno-omit-frame-pointer + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fomit-frame-pointer + if let Some(value) = &self.force_frame_pointers { + let cc_flag = if *value { + "-fno-omit-frame-pointer" + } else { + "-fomit-frame-pointer" + }; + push_if_supported(cc_flag.into()); + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-dead_strip + if let Some(value) = &self.link_dead_code { + if !value { + push_if_supported("-dead_strip".into()); + } + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mno-red-zone + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mred-zone + if let Some(value) = &self.no_redzone { + let cc_flag = if *value { + "-mno-red-zone" + } else { + "-mred-zone" + }; + push_if_supported(cc_flag.into()); + } + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-msoft-float + // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mno-soft-float + if let Some(value) = &self.soft_float { + let cc_flag = if *value { + "-msoft-float" + } else { + "-mno-soft-float" + }; + push_if_supported(cc_flag.into()); + } + } + ToolFamily::Msvc { .. } => { + // https://learn.microsoft.com/en-us/cpp/build/reference/guard-enable-control-flow-guard + if let Some(value) = &self.control_flow_guard { + let cc_val = match value.as_str() { + "y" | "yes" | "on" | "true" | "checks" => Some("cf"), + "n" | "no" | "off" | "false" => Some("cf-"), + _ => None, + }; + if let Some(cc_val) = cc_val { + push_if_supported(format!("/guard:{cc_val}").into()); + } + } + // https://learn.microsoft.com/en-us/cpp/build/reference/oy-frame-pointer-omission + if let Some(value) = &self.force_frame_pointers { + let cc_flag = if *value { "/Oy-" } else { "/Oy" }; + push_if_supported(cc_flag.into()); + } + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[track_caller] + fn check(env: &str, expected: &RustcCodegenFlags) { + let actual = RustcCodegenFlags::parse(env).unwrap(); + assert_eq!(actual, *expected); + } + + #[test] + fn codegen_type() { + let expected = RustcCodegenFlags { + code_model: Some("tiny".into()), + ..RustcCodegenFlags::default() + }; + check("-Ccode-model=tiny", &expected); + check("-C\u{1f}code-model=tiny", &expected); + check("--codegen\u{1f}code-model=tiny", &expected); + check("--codegen=code-model=tiny", &expected); + } + + #[test] + fn precedence() { + check( + "-ccode-model=tiny\u{1f}-Ccode-model=small", + &RustcCodegenFlags { + code_model: Some("small".into()), + ..RustcCodegenFlags::default() + }, + ); + } + + #[test] + fn two_valid_prefixes() { + let expected = RustcCodegenFlags::default(); + check("-L\u{1f}-Clto", &expected); + } + + #[test] + fn three_valid_prefixes() { + let expected = RustcCodegenFlags { + lto: Some("true".into()), + ..RustcCodegenFlags::default() + }; + check("-L\u{1f}-L\u{1f}-Clto", &expected); + } + + #[test] + fn all_rustc_flags() { + // Throw all possible flags at the parser to catch false positives + let flags = [ + // Set all the flags we recognise first + "-Ccode-model=tiny", + "-Ccontrol-flow-guard=yes", + "-Cembed-bitcode=no", + "-Cforce-frame-pointers=yes", + "-Clto=false", + "-Clink-dead-code=yes", + "-Cno-redzone=yes", + "-Cno-vectorize-loops", + "-Cno-vectorize-slp", + "-Cprofile-generate=fooprofile", + "-Cprofile-use=fooprofile", + "-Crelocation-model=pic", + "-Csoft-float=yes", + "-Zbranch-protection=bti,pac-ret,leaf", + // Set flags we don't recognise but rustc supports next + // rustc flags + "--cfg", + "a", + "--check-cfg 'cfg(verbose)", + "-L", + "/usr/lib/foo", + "-l", + "static:+whole-archive=mylib", + "--crate-type=dylib", + "--crate-name=foo", + "--edition=2021", + "--emit=asm", + "--print=crate-name", + "-g", + "-O", + "-o", + "foooutput", + "--out-dir", + "foooutdir", + "--target", + "aarch64-unknown-linux-gnu", + "-W", + "missing-docs", + "-D", + "unused-variables", + "--force-warn", + "dead-code", + "-A", + "unused", + "-F", + "unused", + "--cap-lints", + "warn", + "--version", + "--verbose", + "-v", + "--extern", + "foocrate", + "--sysroot", + "fooroot", + "--error-format", + "human", + "--color", + "auto", + "--diagnostic-width", + "80", + "--remap-path-prefix", + "foo=bar", + "--json=artifact", + // Codegen flags + "-Car", + "-Ccodegen-units=1", + "-Ccollapse-macro-debuginfo=yes", + "-Cdebug-assertions=yes", + "-Cdebuginfo=1", + "-Cdefault-linker-libraries=yes", + "-Cdlltool=foo", + "-Cextra-filename=foo", + "-Cforce-unwind-tables=yes", + "-Cincremental=foodir", + "-Cinline-threshold=6", + "-Cinstrument-coverage", + "-Clink-arg=-foo", + "-Clink-args=-foo", + "-Clink-self-contained=yes", + "-Clinker=lld", + "-Clinker-flavor=ld.lld", + "-Clinker-plugin-lto=yes", + "-Cllvm-args=foo", + "-Cmetadata=foo", + "-Cno-prepopulate-passes", + "-Cno-stack-check", + "-Copt-level=3", + "-Coverflow-checks=yes", + "-Cpanic=abort", + "-Cpasses=foopass", + "-Cprefer-dynamic=yes", + "-Crelro-level=partial", + "-Cremark=all", + "-Crpath=yes", + "-Csave-temps=yes", + "-Csplit-debuginfo=packed", + "-Cstrip=symbols", + "-Csymbol-mangling-version=v0", + "-Ctarget-cpu=native", + "-Ctarget-feature=+sve", + // Unstable options + "-Ztune-cpu=machine", + ]; + check( + &flags.join("\u{1f}"), + &RustcCodegenFlags { + code_model: Some("tiny".into()), + control_flow_guard: Some("yes".into()), + embed_bitcode: Some(false), + force_frame_pointers: Some(true), + link_dead_code: Some(true), + lto: Some("false".into()), + no_redzone: Some(true), + no_vectorize_loops: true, + no_vectorize_slp: true, + profile_generate: Some("fooprofile".into()), + profile_use: Some("fooprofile".into()), + relocation_model: Some("pic".into()), + soft_float: Some(true), + branch_protection: Some("bti,pac-ret,leaf".into()), + }, + ); + } +} diff --git a/src/lib.rs b/src/lib.rs index 187eb705..e9891725 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -261,6 +261,9 @@ mod tempfile; mod utilities; use utilities::*; +mod flags; +use flags::*; + #[derive(Debug, Eq, PartialEq, Hash)] struct CompilerFlag { compiler: Box, @@ -328,6 +331,7 @@ pub struct Build { emit_rerun_if_env_changed: bool, shell_escaped_flags: Option, build_cache: Arc, + inherit_rustflags: bool, } /// Represents the types of errors that may occur while using cc-rs. @@ -343,10 +347,12 @@ enum ErrorKind { ToolNotFound, /// One of the function arguments failed validation. InvalidArgument, - /// No known macro is defined for the compiler when discovering tool family + /// No known macro is defined for the compiler when discovering tool family. ToolFamilyMacroNotFound, - /// Invalid target + /// Invalid target. InvalidTarget, + /// Invalid rustc flag. + InvalidFlag, #[cfg(feature = "parallel")] /// jobserver helpthread failure JobserverHelpThreadError, @@ -449,6 +455,7 @@ impl Build { emit_rerun_if_env_changed: true, shell_escaped_flags: None, build_cache: Arc::default(), + inherit_rustflags: true, } } @@ -677,6 +684,7 @@ impl Build { .debug(false) .cpp(self.cpp) .cuda(self.cuda) + .inherit_rustflags(false) .emit_rerun_if_env_changed(self.emit_rerun_if_env_changed); if let Some(target) = &self.target { cfg.target(target); @@ -1333,6 +1341,15 @@ impl Build { self } + /// Configure whether cc should automatically inherit compatible flags passed to rustc + /// from `CARGO_ENCODED_RUSTFLAGS`. + /// + /// This option defaults to `true`. + pub fn inherit_rustflags(&mut self, inherit_rustflags: bool) -> &mut Build { + self.inherit_rustflags = inherit_rustflags; + self + } + #[doc(hidden)] pub fn __set_env(&mut self, a: A, b: B) -> &mut Build where @@ -1928,6 +1945,11 @@ impl Build { cmd.args.push((**flag).into()); } + // Add cc flags inherited from matching rustc flags + if self.inherit_rustflags { + self.add_inherited_rustflags(&mut cmd, &target)?; + } + for flag in self.flags_supported.iter() { if self .is_flag_supported_inner(flag, &cmd.path, &target) @@ -2365,6 +2387,25 @@ impl Build { Ok(()) } + fn add_inherited_rustflags(&self, cmd: &mut Tool, target: &TargetInfo) -> Result<(), Error> { + let env_os = match self.getenv("CARGO_ENCODED_RUSTFLAGS") { + Some(env) => env, + // No encoded RUSTFLAGS -> nothing to do + None => return Ok(()), + }; + + let Tool { + family, path, args, .. + } = cmd; + + let env = env_os.to_string_lossy(); + let codegen_flags = RustcCodegenFlags::parse(&env)?; + let mut cc_flags_args: Vec = vec![]; + codegen_flags.cc_flags(&self, path, *family, target, &mut cc_flags_args); + args.extend(cc_flags_args); + Ok(()) + } + fn has_flags(&self) -> bool { let flags_env_var_name = if self.cpp { "CXXFLAGS" } else { "CFLAGS" }; let flags_env_var_value = self.getenv_with_target_prefixes(flags_env_var_name); diff --git a/tests/rustflags.rs b/tests/rustflags.rs new file mode 100644 index 00000000..c9c6fc14 --- /dev/null +++ b/tests/rustflags.rs @@ -0,0 +1,29 @@ +use crate::support::Test; +mod support; + +/// This test is in its own module because it modifies the environment and would affect other tests +/// when run in parallel with them. +#[test] +#[cfg(not(windows))] +fn inherits_rustflags() { + // Sanity check - no flags + std::env::set_var("CARGO_ENCODED_RUSTFLAGS", ""); + let test = Test::gnu(); + test.gcc().file("foo.c").compile("foo"); + test.cmd(0) + .must_not_have("-fno-omit-frame-pointer") + .must_not_have("-mcmodel=small") + .must_not_have("-msoft-float"); + + // Correctly inherits flags from rustc + std::env::set_var( + "CARGO_ENCODED_RUSTFLAGS", + "-Cforce-frame-pointers=true\u{1f}-Ccode-model=small\u{1f}-Csoft-float", + ); + let test = Test::gnu(); + test.gcc().file("foo.c").compile("foo"); + test.cmd(0) + .must_have("-fno-omit-frame-pointer") + .must_have("-mcmodel=small") + .must_have("-msoft-float"); +}