summary refs log tree commit diff
path: root/src/bootstrap/test.rs
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-06-14 14:20:59 +0000
committerbors <bors@rust-lang.org>2023-06-14 14:20:59 +0000
commitafa9fef70904bee316d5a73275397d7c4e7c8c4b (patch)
treef4c520b318d66dbbcfd1c1aeb5962ff8154386c2 /src/bootstrap/test.rs
parent7b0eac438ace0ba305b4633328b00474fbbf5120 (diff)
parentf67809ac1dca66e98c0999634212532974e43b97 (diff)
downloadrust-afa9fef70904bee316d5a73275397d7c4e7c8c4b.tar.gz
rust-afa9fef70904bee316d5a73275397d7c4e7c8c4b.zip
Auto merge of #112418 - ferrocene:pa-mir-opt-panic, r=ozkanonur,saethlin
Add support for targets without unwinding in `mir-opt`, and improve `--bless` for it

The main goal of this PR is to add support for targets without unwinding support in the `mir-opt` test suite, by adding the `EMIT_MIR_FOR_EACH_PANIC_STRATEGY` comment. Similarly to 32bit vs 64bit, when that comment is present, blessed output files will have the `.panic-unwind` or `.panic-abort` suffix, and the right one will be chosen depending on the target's panic strategy.

The `EMIT_MIR_FOR_EACH_PANIC_STRATEGY` comment replaced all the `ignore-wasm32` comments in the `mir-opt` test suite, as those comments were added due to `wasm32` being a target without unwinding support. The comment was also added on other tests that were only executed on x86 but were still panic strategy dependent.

The `mir-opt` suite was then blessed, which caused a ton of churn as most of the existing output files had to be renamed and (mostly) duplicated with the abort strategy.

---

After [asking on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/mir-opt.20tests.20and.20panic.3Dabort), the main concern about this change is it'd make blessing the `mir-opt` suite even harder, as you'd need to both bless it with an unwinding target and an aborting target. This exacerbated the current situation, where you'd need to bless it with a 32bit and a 64bit target already.

Because of that, this PR also makes significant enhancements to `--bless` for the `mir-opt` suite, where it will automatically bless the suite four times with different targets, while requiring minimal cross-compilation.

To handle the 32bit vs 64bit blessing, there is now an hardcoded list of target mapping between 32bit and 64bit. The goal of the list is to find a related target that will *probably* work without requiring additional cross-compilation toolchains on the system. If a mapping is found, bootstrap will bless the suite with both targets, otherwise just with the current target.

To handle the panic strategy blessing (abort vs unwind), I had to resort to what I call "synthetic targets". For each of the target we're blessing (so either the current one, or a 32bit and a 64bit depending on the previous paragraph), bootstrap will extract the JSON spec of the target and change it to include `"panic-strategy": "abort"`. It will then build the standard library with this synthetic target, and bless the `mir-opt` suite with it.

As a result of these changes, blessing the `mir-opt` suite will actually bless it two or four times with different targets, ensuring all possible variants are actually blessed.

---

This PR is best reviewed commit-by-commit.

r? `@jyn514`
cc `@saethlin` `@oli-obk`
Diffstat (limited to 'src/bootstrap/test.rs')
-rw-r--r--src/bootstrap/test.rs108
1 files changed, 104 insertions, 4 deletions
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index cde77f4720b..13a10b0d3a5 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -23,6 +23,7 @@ use crate::doc::DocumentationFormat;
 use crate::flags::Subcommand;
 use crate::llvm;
 use crate::render_tests::add_flags_and_try_run_tests;
+use crate::synthetic_targets::MirOptPanicAbortSyntheticTarget;
 use crate::tool::{self, SourceType, Tool};
 use crate::toolstate::ToolState;
 use crate::util::{self, add_link_lib_path, dylib_path, dylib_path_var, output, t, up_to_date};
@@ -30,6 +31,22 @@ use crate::{envify, CLang, DocTests, GitRepo, Mode};
 
 const ADB_TEST_DIR: &str = "/data/local/tmp/work";
 
+// mir-opt tests have different variants depending on whether a target is 32bit or 64bit, and
+// blessing them requires blessing with each target. To aid developers, when blessing the mir-opt
+// test suite the corresponding target of the opposite pointer size is also blessed.
+//
+// This array serves as the known mappings between 32bit and 64bit targets. If you're developing on
+// a target where a target with the opposite pointer size exists, feel free to add it here.
+const MIR_OPT_BLESS_TARGET_MAPPING: &[(&str, &str)] = &[
+    // (32bit, 64bit)
+    ("i686-unknown-linux-gnu", "x86_64-unknown-linux-gnu"),
+    ("i686-unknown-linux-musl", "x86_64-unknown-linux-musl"),
+    ("i686-pc-windows-msvc", "x86_64-pc-windows-msvc"),
+    ("i686-pc-windows-gnu", "x86_64-pc-windows-gnu"),
+    ("i686-apple-darwin", "x86_64-apple-darwin"),
+    ("i686-apple-darwin", "aarch64-apple-darwin"),
+];
+
 fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool {
     if !builder.fail_fast {
         if !builder.try_run(cmd) {
@@ -1261,8 +1278,6 @@ default_test!(RunPassValgrind {
     suite: "run-pass-valgrind"
 });
 
-default_test!(MirOpt { path: "tests/mir-opt", mode: "mir-opt", suite: "mir-opt" });
-
 default_test!(Codegen { path: "tests/codegen", mode: "codegen", suite: "codegen" });
 
 default_test!(CodegenUnits {
@@ -1299,6 +1314,91 @@ host_test!(RunMakeFullDeps {
 
 default_test!(Assembly { path: "tests/assembly", mode: "assembly", suite: "assembly" });
 
+// For the mir-opt suite we do not use macros, as we need custom behavior when blessing.
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+pub struct MirOpt {
+    pub compiler: Compiler,
+    pub target: TargetSelection,
+}
+
+impl Step for MirOpt {
+    type Output = ();
+    const DEFAULT: bool = true;
+    const ONLY_HOSTS: bool = false;
+
+    fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
+        run.suite_path("tests/mir-opt")
+    }
+
+    fn make_run(run: RunConfig<'_>) {
+        let compiler = run.builder.compiler(run.builder.top_stage, run.build_triple());
+        run.builder.ensure(MirOpt { compiler, target: run.target });
+    }
+
+    fn run(self, builder: &Builder<'_>) {
+        let run = |target| {
+            builder.ensure(Compiletest {
+                compiler: self.compiler,
+                target: target,
+                mode: "mir-opt",
+                suite: "mir-opt",
+                path: "tests/mir-opt",
+                compare_mode: None,
+            })
+        };
+
+        // We use custom logic to bless the mir-opt suite: mir-opt tests have multiple variants
+        // (32bit vs 64bit, and panic=abort vs panic=unwind), and all of them needs to be blessed.
+        // When blessing, we try best-effort to also bless the other variants, to aid developers.
+        if builder.config.cmd.bless() {
+            let targets = MIR_OPT_BLESS_TARGET_MAPPING
+                .iter()
+                .filter(|(target_32bit, target_64bit)| {
+                    *target_32bit == &*self.target.triple || *target_64bit == &*self.target.triple
+                })
+                .next()
+                .map(|(target_32bit, target_64bit)| {
+                    let target_32bit = TargetSelection::from_user(target_32bit);
+                    let target_64bit = TargetSelection::from_user(target_64bit);
+
+                    // Running compiletest requires a C compiler to be available, but it might not
+                    // have been detected by bootstrap if the target we're testing wasn't in the
+                    // --target flags.
+                    if !builder.cc.borrow().contains_key(&target_32bit) {
+                        crate::cc_detect::find_target(builder, target_32bit);
+                    }
+                    if !builder.cc.borrow().contains_key(&target_64bit) {
+                        crate::cc_detect::find_target(builder, target_64bit);
+                    }
+
+                    vec![target_32bit, target_64bit]
+                })
+                .unwrap_or_else(|| {
+                    eprintln!(
+                        "\
+Note that not all variants of mir-opt tests are going to be blessed, as no mapping between
+a 32bit and a 64bit target was found for {target}.
+You can add that mapping by changing MIR_OPT_BLESS_TARGET_MAPPING in src/bootstrap/test.rs",
+                        target = self.target,
+                    );
+                    vec![self.target]
+                });
+
+            for target in targets {
+                run(target);
+
+                let panic_abort_target = builder.ensure(MirOptPanicAbortSyntheticTarget {
+                    compiler: self.compiler,
+                    base: target,
+                });
+                run(panic_abort_target);
+            }
+        } else {
+            run(self.target);
+        }
+    }
+}
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 struct Compiletest {
     compiler: Compiler,
@@ -1667,7 +1767,7 @@ note: if you're sure you want to do this, please open an issue as to why. In the
         // Note that if we encounter `PATH` we make sure to append to our own `PATH`
         // rather than stomp over it.
         if target.contains("msvc") {
-            for &(ref k, ref v) in builder.cc[&target].env() {
+            for &(ref k, ref v) in builder.cc.borrow()[&target].env() {
                 if k != "PATH" {
                     cmd.env(k, v);
                 }
@@ -1692,7 +1792,7 @@ note: if you're sure you want to do this, please open an issue as to why. In the
 
         cmd.arg("--adb-path").arg("adb");
         cmd.arg("--adb-test-dir").arg(ADB_TEST_DIR);
-        if target.contains("android") {
+        if target.contains("android") && !builder.config.dry_run() {
             // Assume that cc for this target comes from the android sysroot
             cmd.arg("--android-cross-path")
                 .arg(builder.cc(target).parent().unwrap().parent().unwrap());