From f9df1ad4f252531d0d66c97beb0d25c0deb3e07d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 28 Oct 2023 09:26:39 +1100 Subject: [PATCH 1/4] Check for profiler support via a flag, instead of an environment var --- src/bootstrap/src/core/build_steps/test.rs | 2 +- src/tools/compiletest/src/common.rs | 4 ++++ src/tools/compiletest/src/header/needs.rs | 2 +- src/tools/compiletest/src/header/tests.rs | 18 ++++++++++++++++++ src/tools/compiletest/src/lib.rs | 3 +++ 5 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 3ae3af38bf85b..f757bdb001eba 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1976,7 +1976,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the } if builder.config.profiler_enabled(target) { - cmd.env("RUSTC_PROFILER_SUPPORT", "1"); + cmd.arg("--profiler-support"); } cmd.env("RUST_TEST_TMPDIR", builder.tempdir()); diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index e85f6319936bf..4cf5a710586c1 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -387,6 +387,10 @@ pub struct Config { // Needed both to construct build_helper::git::GitConfig pub git_repository: String, pub nightly_branch: String, + + /// True if the profiler runtime is enabled for this target. + /// Used by the "needs-profiler-support" header in test files. + pub profiler_support: bool, } impl Config { diff --git a/src/tools/compiletest/src/header/needs.rs b/src/tools/compiletest/src/header/needs.rs index 4a40fb55f5c16..9b22b2112a8c0 100644 --- a/src/tools/compiletest/src/header/needs.rs +++ b/src/tools/compiletest/src/header/needs.rs @@ -238,7 +238,7 @@ impl CachedNeedsConditions { sanitizer_memtag: sanitizers.contains(&Sanitizer::Memtag), sanitizer_shadow_call_stack: sanitizers.contains(&Sanitizer::ShadowCallStack), sanitizer_safestack: sanitizers.contains(&Sanitizer::Safestack), - profiler_support: std::env::var_os("RUSTC_PROFILER_SUPPORT").is_some(), + profiler_support: config.profiler_support, xray: config.target_cfg().xray, // For tests using the `needs-rust-lld` directive (e.g. for `-Clink-self-contained=+linker`), diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 295134c78dcb1..c6d63f7419fed 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -62,6 +62,7 @@ struct ConfigBuilder { llvm_version: Option, git_hash: bool, system_llvm: bool, + profiler_support: bool, } impl ConfigBuilder { @@ -100,6 +101,11 @@ impl ConfigBuilder { self } + fn profiler_support(&mut self, s: bool) -> &mut Self { + self.profiler_support = s; + self + } + fn build(&mut self) -> Config { let args = &[ "compiletest", @@ -142,6 +148,9 @@ impl ConfigBuilder { if self.system_llvm { args.push("--system-llvm".to_owned()); } + if self.profiler_support { + args.push("--profiler-support".to_owned()); + } args.push("--rustc-path".to_string()); // This is a subtle/fragile thing. On rust-lang CI, there is no global @@ -340,6 +349,15 @@ fn sanitizers() { assert!(check_ignore(&config, "// needs-sanitizer-thread")); } +#[test] +fn profiler_support() { + let config: Config = cfg().profiler_support(false).build(); + assert!(check_ignore(&config, "// needs-profiler-support")); + + let config: Config = cfg().profiler_support(true).build(); + assert!(!check_ignore(&config, "// needs-profiler-support")); +} + #[test] fn asm_support() { let asms = [ diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 5a80b9121f063..60dd15841b766 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -142,6 +142,7 @@ pub fn parse_config(args: Vec) -> Config { .optflag("", "force-rerun", "rerun tests even if the inputs are unchanged") .optflag("", "only-modified", "only run tests that result been modified") .optflag("", "nocapture", "") + .optflag("", "profiler-support", "is the profiler runtime enabled for this target") .optflag("h", "help", "show this message") .reqopt("", "channel", "current Rust channel", "CHANNEL") .optflag("", "git-hash", "run tests which rely on commit version being compiled into the binaries") @@ -315,6 +316,8 @@ pub fn parse_config(args: Vec) -> Config { git_repository: matches.opt_str("git-repository").unwrap(), nightly_branch: matches.opt_str("nightly-branch").unwrap(), + + profiler_support: matches.opt_present("profiler-support"), } } From aa4bf0bbf0078ca0f9b2cc1a9c0edea0b551af35 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 30 Nov 2023 17:45:03 +1100 Subject: [PATCH 2/4] Allow tests to ignore individual test modes Normally, each test in `tests/coverage` is automatically run in both `coverage-map` mode and `coverage-run` mode. This new family of directives allows an individual test to specify that it should not be run in a particular mode. --- src/tools/compiletest/src/header/cfg.rs | 13 +++++++++++- src/tools/compiletest/src/header/tests.rs | 26 +++++++++++++++++++++-- tests/coverage/ignore_map.coverage | 4 ++++ tests/coverage/ignore_map.rs | 3 +++ tests/coverage/ignore_run.cov-map | 8 +++++++ tests/coverage/ignore_run.rs | 3 +++ 6 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 tests/coverage/ignore_map.coverage create mode 100644 tests/coverage/ignore_map.rs create mode 100644 tests/coverage/ignore_run.cov-map create mode 100644 tests/coverage/ignore_run.rs diff --git a/src/tools/compiletest/src/header/cfg.rs b/src/tools/compiletest/src/header/cfg.rs index e2a04b7e55882..df8c804705029 100644 --- a/src/tools/compiletest/src/header/cfg.rs +++ b/src/tools/compiletest/src/header/cfg.rs @@ -1,4 +1,4 @@ -use crate::common::{CompareMode, Config, Debugger}; +use crate::common::{CompareMode, Config, Debugger, Mode}; use crate::header::IgnoreDecision; use std::collections::HashSet; @@ -208,6 +208,17 @@ pub(super) fn parse_cfg_name_directive<'a>( }, message: "when comparing with {name}", } + // Coverage tests run the same test file in multiple modes. + // If a particular test should not be run in one of the modes, ignore it + // with "ignore-mode-coverage-map" or "ignore-mode-coverage-run". + condition! { + name: format!("mode-{}", config.mode.to_str()), + allowed_names: ContainsPrefixed { + prefix: "mode-", + inner: Mode::STR_VARIANTS, + }, + message: "when the test mode is {name}", + } if prefix == "ignore" && outcome == MatchOutcome::Invalid { // Don't error out for ignore-tidy-* diretives, as those are not handled by compiletest. diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index c6d63f7419fed..8882f1582acc7 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -1,7 +1,8 @@ use std::io::Read; use std::path::Path; +use std::str::FromStr; -use crate::common::{Config, Debugger}; +use crate::common::{Config, Debugger, Mode}; use crate::header::{parse_normalization_string, EarlyProps, HeadersCache}; fn make_test_description( @@ -55,6 +56,7 @@ fn test_parse_normalization_string() { #[derive(Default)] struct ConfigBuilder { + mode: Option, channel: Option, host: Option, target: Option, @@ -66,6 +68,11 @@ struct ConfigBuilder { } impl ConfigBuilder { + fn mode(&mut self, s: &str) -> &mut Self { + self.mode = Some(s.to_owned()); + self + } + fn channel(&mut self, s: &str) -> &mut Self { self.channel = Some(s.to_owned()); self @@ -109,7 +116,8 @@ impl ConfigBuilder { fn build(&mut self) -> Config { let args = &[ "compiletest", - "--mode=ui", + "--mode", + self.mode.as_deref().unwrap_or("ui"), "--suite=ui", "--compile-lib-path=", "--run-lib-path=", @@ -548,3 +556,17 @@ fn families() { assert!(!check_ignore(&config, &format!("// ignore-{other}"))); } } + +#[test] +fn ignore_mode() { + for &mode in Mode::STR_VARIANTS { + // Indicate profiler support so that "coverage-run" tests aren't skipped. + let config: Config = cfg().mode(mode).profiler_support(true).build(); + let other = if mode == "coverage-run" { "coverage-map" } else { "coverage-run" }; + assert_ne!(mode, other); + assert_eq!(config.mode, Mode::from_str(mode).unwrap()); + assert_ne!(config.mode, Mode::from_str(other).unwrap()); + assert!(check_ignore(&config, &format!("// ignore-mode-{mode}"))); + assert!(!check_ignore(&config, &format!("// ignore-mode-{other}"))); + } +} diff --git a/tests/coverage/ignore_map.coverage b/tests/coverage/ignore_map.coverage new file mode 100644 index 0000000000000..04bcb5bec6ec2 --- /dev/null +++ b/tests/coverage/ignore_map.coverage @@ -0,0 +1,4 @@ + LL| |// ignore-mode-coverage-map + LL| | + LL| 1|fn main() {} + diff --git a/tests/coverage/ignore_map.rs b/tests/coverage/ignore_map.rs new file mode 100644 index 0000000000000..71b82e8fc9def --- /dev/null +++ b/tests/coverage/ignore_map.rs @@ -0,0 +1,3 @@ +// ignore-mode-coverage-map + +fn main() {} diff --git a/tests/coverage/ignore_run.cov-map b/tests/coverage/ignore_run.cov-map new file mode 100644 index 0000000000000..9865efae0a1ef --- /dev/null +++ b/tests/coverage/ignore_run.cov-map @@ -0,0 +1,8 @@ +Function name: ignore_run::main +Raw bytes (9): 0x[01, 01, 00, 01, 01, 03, 01, 00, 0d] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 3, 1) to (start + 0, 13) + diff --git a/tests/coverage/ignore_run.rs b/tests/coverage/ignore_run.rs new file mode 100644 index 0000000000000..87108867a057d --- /dev/null +++ b/tests/coverage/ignore_run.rs @@ -0,0 +1,3 @@ +// ignore-mode-coverage-run + +fn main() {} From 9ab8c632ee2ede9a350104278864154d933ed1db Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 4 Jan 2024 11:08:01 +1100 Subject: [PATCH 3/4] Extract a `split_flags` helper in header directive parsing --- src/tools/compiletest/src/header.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index f85f9e674ab12..a9f022664cfd9 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -321,16 +321,23 @@ impl TestProps { |r| r, ); - if let Some(flags) = config.parse_name_value_directive(ln, COMPILE_FLAGS) { - self.compile_flags.extend( - flags - .split("'") - .enumerate() - .flat_map(|(i, f)| { + fn split_flags(flags: &str) -> Vec { + // Individual flags can be single-quoted to preserve spaces; see + // . + flags + .split("'") + .enumerate() + .flat_map( + |(i, f)| { if i % 2 == 1 { vec![f] } else { f.split_whitespace().collect() } - }) - .map(|s| s.to_owned()), - ); + }, + ) + .map(move |s| s.to_owned()) + .collect::>() + } + + if let Some(flags) = config.parse_name_value_directive(ln, COMPILE_FLAGS) { + self.compile_flags.extend(split_flags(&flags)); } if config.parse_name_value_directive(ln, INCORRECT_COMPILER_FLAGS).is_some() { panic!("`compiler-flags` directive should be spelled `compile-flags`"); From 731ba80a6b53e3397eadeda37b18bd8fb3016aad Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 16 Dec 2023 13:30:47 +1100 Subject: [PATCH 4/4] Allow coverage tests to enable `llvm-cov --use-color` --- src/tools/compiletest/src/header.rs | 9 +++++++++ src/tools/compiletest/src/runtest.rs | 2 ++ tests/coverage/color.coverage | 13 +++++++++++++ tests/coverage/color.rs | 11 +++++++++++ 4 files changed, 35 insertions(+) create mode 100644 tests/coverage/color.coverage create mode 100644 tests/coverage/color.rs diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index a9f022664cfd9..e70e01e8757e0 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -178,6 +178,9 @@ pub struct TestProps { // Whether to tell `rustc` to remap the "src base" directory to a fake // directory. pub remap_src_base: bool, + /// Extra flags to pass to `llvm-cov` when producing coverage reports. + /// Only used by the "coverage-run" test mode. + pub llvm_cov_flags: Vec, } mod directives { @@ -216,6 +219,7 @@ mod directives { pub const MIR_UNIT_TEST: &'static str = "unit-test"; pub const REMAP_SRC_BASE: &'static str = "remap-src-base"; pub const COMPARE_OUTPUT_LINES_BY_SUBSET: &'static str = "compare-output-lines-by-subset"; + pub const LLVM_COV_FLAGS: &'static str = "llvm-cov-flags"; // This isn't a real directive, just one that is probably mistyped often pub const INCORRECT_COMPILER_FLAGS: &'static str = "compiler-flags"; } @@ -265,6 +269,7 @@ impl TestProps { stderr_per_bitwidth: false, mir_unit_test: None, remap_src_base: false, + llvm_cov_flags: vec![], } } @@ -495,6 +500,10 @@ impl TestProps { COMPARE_OUTPUT_LINES_BY_SUBSET, &mut self.compare_output_lines_by_subset, ); + + if let Some(flags) = config.parse_name_value_directive(ln, LLVM_COV_FLAGS) { + self.llvm_cov_flags.extend(split_flags(&flags)); + } }); } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 1f5f77839de41..b258b748ca876 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -575,6 +575,8 @@ impl<'test> TestCx<'test> { cmd.arg("--object"); cmd.arg(bin); } + + cmd.args(&self.props.llvm_cov_flags); }); if !proc_res.status.success() { self.fatal_proc_rec("llvm-cov show failed!", &proc_res); diff --git a/tests/coverage/color.coverage b/tests/coverage/color.coverage new file mode 100644 index 0000000000000..bc49fff9cb75d --- /dev/null +++ b/tests/coverage/color.coverage @@ -0,0 +1,13 @@ + LL| |// edition: 2021 + LL| |// ignore-mode-coverage-map + LL| |// ignore-windows + LL| |// llvm-cov-flags: --use-color + LL| | + LL| |// Verify that telling `llvm-cov` to use colored output actually works. + LL| |// Ignored on Windows because we can't tell the tool to use ANSI escapes. + LL| | + LL| 1|fn main() { + LL| 1| for _i in 0..0 {} + ^0 ^0 + LL| 1|} + diff --git a/tests/coverage/color.rs b/tests/coverage/color.rs new file mode 100644 index 0000000000000..bd727946c781b --- /dev/null +++ b/tests/coverage/color.rs @@ -0,0 +1,11 @@ +// edition: 2021 +// ignore-mode-coverage-map +// ignore-windows +// llvm-cov-flags: --use-color + +// Verify that telling `llvm-cov` to use colored output actually works. +// Ignored on Windows because we can't tell the tool to use ANSI escapes. + +fn main() { + for _i in 0..0 {} +}