Skip to content

Commit 871df0d

Browse files
borsbors
committedApr 1, 2024
Auto merge of #123192 - RalfJung:bootstrap-test-miri, r=onur-ozkan
Refactor the way bootstrap invokes `cargo miri` Instead of basically doing `cargo run --manifest-path=<cargo-miri's manifest> -- miri`, let's invoke the `cargo-miri` binary directly. That means less indirections, and also makes it easier to e.g. run the libcore test suite in Miri. (But there are still other issues with that.) Also also adjusted Miri's stage numbering so that it is consistent with rustc/rustdoc. This also makes `./x.py test miri` honor `--no-doc`. And this fixes #123177 by moving where we handle parallel_compiler.
2 parents 7f84ede + 85d460e commit 871df0d

File tree

10 files changed

+174
-135
lines changed

10 files changed

+174
-135
lines changed
 

‎src/bootstrap/src/bin/rustc.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ fn main() {
150150
{
151151
cmd.arg("-Ztls-model=initial-exec");
152152
}
153-
} else if std::env::var("MIRI").is_err() {
153+
} else {
154154
// Find any host flags that were passed by bootstrap.
155155
// The flags are stored in a RUSTC_HOST_FLAGS variable, separated by spaces.
156156
if let Ok(flags) = std::env::var("RUSTC_HOST_FLAGS") {

‎src/bootstrap/src/core/build_steps/compile.rs

-6
Original file line numberDiff line numberDiff line change
@@ -1130,12 +1130,6 @@ pub fn rustc_cargo_env(
11301130
cargo.env("CFG_DEFAULT_LINKER", s);
11311131
}
11321132

1133-
if builder.config.rustc_parallel {
1134-
// keep in sync with `bootstrap/lib.rs:Build::rustc_features`
1135-
// `cfg` option for rustc, `features` option for cargo, for conditional compilation
1136-
cargo.rustflag("--cfg=parallel_compiler");
1137-
cargo.rustdocflag("--cfg=parallel_compiler");
1138-
}
11391133
if builder.config.rust_verify_llvm_ir {
11401134
cargo.env("RUSTC_VERIFY_LLVM_IR", "1");
11411135
}

‎src/bootstrap/src/core/build_steps/run.rs

+19-15
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,6 @@ impl Step for ReplaceVersionPlaceholder {
121121

122122
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
123123
pub struct Miri {
124-
stage: u32,
125-
host: TargetSelection,
126124
target: TargetSelection,
127125
}
128126

@@ -135,29 +133,35 @@ impl Step for Miri {
135133
}
136134

137135
fn make_run(run: RunConfig<'_>) {
138-
run.builder.ensure(Miri {
139-
stage: run.builder.top_stage,
140-
host: run.build_triple(),
141-
target: run.target,
142-
});
136+
run.builder.ensure(Miri { target: run.target });
143137
}
144138

145139
fn run(self, builder: &Builder<'_>) {
146-
let stage = self.stage;
147-
let host = self.host;
140+
let host = builder.build.build;
148141
let target = self.target;
149-
let compiler = builder.compiler(stage, host);
150-
151-
let miri =
152-
builder.ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() });
153-
let miri_sysroot = test::Miri::build_miri_sysroot(builder, compiler, &miri, target);
142+
let stage = builder.top_stage;
143+
if stage == 0 {
144+
eprintln!("miri cannot be run at stage 0");
145+
std::process::exit(1);
146+
}
147+
148+
// This compiler runs on the host, we'll just use it for the target.
149+
let target_compiler = builder.compiler(stage, host);
150+
// Similar to `compile::Assemble`, build with the previous stage's compiler. Otherwise
151+
// we'd have stageN/bin/rustc and stageN/bin/rustdoc be effectively different stage
152+
// compilers, which isn't what we want. Rustdoc should be linked in the same way as the
153+
// rustc compiler it's paired with, so it must be built with the previous stage compiler.
154+
let host_compiler = builder.compiler(stage - 1, host);
155+
156+
// Get a target sysroot for Miri.
157+
let miri_sysroot = test::Miri::build_miri_sysroot(builder, target_compiler, target);
154158

155159
// # Run miri.
156160
// Running it via `cargo run` as that figures out the right dylib path.
157161
// add_rustc_lib_path does not add the path that contains librustc_driver-<...>.so.
158162
let mut miri = tool::prepare_tool_cargo(
159163
builder,
160-
compiler,
164+
host_compiler,
161165
Mode::ToolRustc,
162166
host,
163167
"run",

‎src/bootstrap/src/core/build_steps/test.rs

+75-81
Original file line numberDiff line numberDiff line change
@@ -493,8 +493,6 @@ impl Step for RustDemangler {
493493

494494
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
495495
pub struct Miri {
496-
stage: u32,
497-
host: TargetSelection,
498496
target: TargetSelection,
499497
}
500498

@@ -503,41 +501,26 @@ impl Miri {
503501
pub fn build_miri_sysroot(
504502
builder: &Builder<'_>,
505503
compiler: Compiler,
506-
miri: &Path,
507504
target: TargetSelection,
508505
) -> String {
509506
let miri_sysroot = builder.out.join(compiler.host.triple).join("miri-sysroot");
510-
let mut cargo = tool::prepare_tool_cargo(
507+
let mut cargo = builder::Cargo::new(
511508
builder,
512509
compiler,
513-
Mode::ToolRustc,
514-
compiler.host,
515-
"run",
516-
"src/tools/miri/cargo-miri",
517-
SourceType::InTree,
518-
&[],
510+
Mode::Std,
511+
SourceType::Submodule,
512+
target,
513+
"miri-setup",
519514
);
520-
cargo.add_rustc_lib_path(builder);
521-
cargo.arg("--").arg("miri").arg("setup");
522-
cargo.arg("--target").arg(target.rustc_target_arg());
523515

524516
// Tell `cargo miri setup` where to find the sources.
525517
cargo.env("MIRI_LIB_SRC", builder.src.join("library"));
526-
// Tell it where to find Miri.
527-
cargo.env("MIRI", miri);
528518
// Tell it where to put the sysroot.
529519
cargo.env("MIRI_SYSROOT", &miri_sysroot);
530-
// Debug things.
531-
cargo.env("RUST_BACKTRACE", "1");
532520

533521
let mut cargo = Command::from(cargo);
534-
let _guard = builder.msg(
535-
Kind::Build,
536-
compiler.stage + 1,
537-
"miri sysroot",
538-
compiler.host,
539-
compiler.host,
540-
);
522+
let _guard =
523+
builder.msg(Kind::Build, compiler.stage, "miri sysroot", compiler.host, target);
541524
builder.run(&mut cargo);
542525

543526
// # Determine where Miri put its sysroot.
@@ -574,68 +557,75 @@ impl Step for Miri {
574557
}
575558

576559
fn make_run(run: RunConfig<'_>) {
577-
run.builder.ensure(Miri {
578-
stage: run.builder.top_stage,
579-
host: run.build_triple(),
580-
target: run.target,
581-
});
560+
run.builder.ensure(Miri { target: run.target });
582561
}
583562

584563
/// Runs `cargo test` for miri.
585564
fn run(self, builder: &Builder<'_>) {
586-
let stage = self.stage;
587-
let host = self.host;
565+
let host = builder.build.build;
588566
let target = self.target;
589-
let compiler = builder.compiler(stage, host);
590-
// We need the stdlib for the *next* stage, as it was built with this compiler that also built Miri.
591-
// Except if we are at stage 2, the bootstrap loop is complete and we can stick with our current stage.
592-
let compiler_std = builder.compiler(if stage < 2 { stage + 1 } else { stage }, host);
593-
594-
let miri =
595-
builder.ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() });
596-
let _cargo_miri = builder.ensure(tool::CargoMiri {
597-
compiler,
598-
target: self.host,
567+
let stage = builder.top_stage;
568+
if stage == 0 {
569+
eprintln!("miri cannot be tested at stage 0");
570+
std::process::exit(1);
571+
}
572+
573+
// This compiler runs on the host, we'll just use it for the target.
574+
let target_compiler = builder.compiler(stage, host);
575+
// Similar to `compile::Assemble`, build with the previous stage's compiler. Otherwise
576+
// we'd have stageN/bin/rustc and stageN/bin/rustdoc be effectively different stage
577+
// compilers, which isn't what we want. Rustdoc should be linked in the same way as the
578+
// rustc compiler it's paired with, so it must be built with the previous stage compiler.
579+
let host_compiler = builder.compiler(stage - 1, host);
580+
581+
// Build our tools.
582+
let miri = builder.ensure(tool::Miri {
583+
compiler: host_compiler,
584+
target: host,
585+
extra_features: Vec::new(),
586+
});
587+
// the ui tests also assume cargo-miri has been built
588+
builder.ensure(tool::CargoMiri {
589+
compiler: host_compiler,
590+
target: host,
599591
extra_features: Vec::new(),
600592
});
601-
// The stdlib we need might be at a different stage. And just asking for the
602-
// sysroot does not seem to populate it, so we do that first.
603-
builder.ensure(compile::Std::new(compiler_std, host));
604-
let sysroot = builder.sysroot(compiler_std);
605-
// We also need a Miri sysroot.
606-
let miri_sysroot = Miri::build_miri_sysroot(builder, compiler, &miri, target);
593+
594+
// We also need sysroots, for Miri and for the host (the latter for build scripts).
595+
// This is for the tests so everything is done with the target compiler.
596+
let miri_sysroot = Miri::build_miri_sysroot(builder, target_compiler, target);
597+
builder.ensure(compile::Std::new(target_compiler, host));
598+
let sysroot = builder.sysroot(target_compiler);
607599

608600
// # Run `cargo test`.
601+
// This is with the Miri crate, so it uses the host compiler.
609602
let mut cargo = tool::prepare_tool_cargo(
610603
builder,
611-
compiler,
604+
host_compiler,
612605
Mode::ToolRustc,
613606
host,
614607
"test",
615608
"src/tools/miri",
616609
SourceType::InTree,
617610
&[],
618611
);
619-
let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "miri", host, target);
620612

621613
cargo.add_rustc_lib_path(builder);
622614

615+
// We can NOT use `run_cargo_test` since Miri's integration tests do not use the usual test
616+
// harness and therefore do not understand the flags added by `add_flags_and_try_run_test`.
617+
let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", host_compiler, host, builder);
618+
623619
// miri tests need to know about the stage sysroot
624620
cargo.env("MIRI_SYSROOT", &miri_sysroot);
625621
cargo.env("MIRI_HOST_SYSROOT", &sysroot);
626622
cargo.env("MIRI", &miri);
627-
if builder.config.locked_deps {
628-
// enforce lockfiles
629-
cargo.env("CARGO_EXTRA_FLAGS", "--locked");
630-
}
631623

632624
// Set the target.
633625
cargo.env("MIRI_TEST_TARGET", target.rustc_target_arg());
634626

635-
// This can NOT be `run_cargo_test` since the Miri test runner
636-
// does not understand the flags added by `add_flags_and_try_run_test`.
637-
let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", compiler, target, builder);
638627
{
628+
let _guard = builder.msg_sysroot_tool(Kind::Test, stage, "miri", host, target);
639629
let _time = helpers::timeit(builder);
640630
builder.run(&mut cargo);
641631
}
@@ -650,8 +640,14 @@ impl Step for Miri {
650640
// Optimizations can change error locations and remove UB so don't run `fail` tests.
651641
cargo.args(["tests/pass", "tests/panic"]);
652642

653-
let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", compiler, target, builder);
654643
{
644+
let _guard = builder.msg_sysroot_tool(
645+
Kind::Test,
646+
stage,
647+
"miri (mir-opt-level 4)",
648+
host,
649+
target,
650+
);
655651
let _time = helpers::timeit(builder);
656652
builder.run(&mut cargo);
657653
}
@@ -660,42 +656,40 @@ impl Step for Miri {
660656
// # Run `cargo miri test`.
661657
// This is just a smoke test (Miri's own CI invokes this in a bunch of different ways and ensures
662658
// that we get the desired output), but that is sufficient to make sure that the libtest harness
663-
// itself executes properly under Miri.
659+
// itself executes properly under Miri, and that all the logic in `cargo-miri` does not explode.
660+
// This is running the build `cargo-miri` for the given target, so we need the target compiler.
664661
let mut cargo = tool::prepare_tool_cargo(
665662
builder,
666-
compiler,
667-
Mode::ToolRustc,
668-
host,
669-
"run",
670-
"src/tools/miri/cargo-miri",
663+
target_compiler,
664+
Mode::ToolStd, // it's unclear what to use here, we're not building anything just doing a smoke test!
665+
target,
666+
"miri-test",
667+
"src/tools/miri/test-cargo-miri",
671668
SourceType::Submodule,
672669
&[],
673670
);
674-
cargo.add_rustc_lib_path(builder);
675-
cargo.arg("--").arg("miri").arg("test");
676-
if builder.config.locked_deps {
677-
cargo.arg("--locked");
678-
}
679-
cargo
680-
.arg("--manifest-path")
681-
.arg(builder.src.join("src/tools/miri/test-cargo-miri/Cargo.toml"));
682-
cargo.arg("--target").arg(target.rustc_target_arg());
683-
cargo.arg("--").args(builder.config.test_args());
684671

685-
// `prepare_tool_cargo` sets RUSTDOC to the bootstrap wrapper and RUSTDOC_REAL to a dummy path as this is a "run", not a "test".
686-
// Also, we want the rustdoc from the "next" stage for the same reason that we build a std from the next stage.
687-
// So let's just set that here, and bypass bootstrap's RUSTDOC (just like cargo-miri already ignores bootstrap's RUSTC_WRAPPER).
688-
cargo.env("RUSTDOC", builder.rustdoc(compiler_std));
672+
// We're not using `prepare_cargo_test` so we have to do this ourselves.
673+
// (We're not using that as the test-cargo-miri crate is not known to bootstrap.)
674+
match builder.doc_tests {
675+
DocTests::Yes => {}
676+
DocTests::No => {
677+
cargo.args(["--lib", "--bins", "--examples", "--tests", "--benches"]);
678+
}
679+
DocTests::Only => {
680+
cargo.arg("--doc");
681+
}
682+
}
689683

690-
// Tell `cargo miri` where to find things.
684+
// Tell `cargo miri` where to find the sysroots.
691685
cargo.env("MIRI_SYSROOT", &miri_sysroot);
692686
cargo.env("MIRI_HOST_SYSROOT", sysroot);
693-
cargo.env("MIRI", &miri);
694-
// Debug things.
695-
cargo.env("RUST_BACKTRACE", "1");
696687

688+
// Finally, pass test-args and run everything.
689+
cargo.arg("--").args(builder.config.test_args());
697690
let mut cargo = Command::from(cargo);
698691
{
692+
let _guard = builder.msg_sysroot_tool(Kind::Test, stage, "cargo-miri", host, target);
699693
let _time = helpers::timeit(builder);
700694
builder.run(&mut cargo);
701695
}

‎src/bootstrap/src/core/build_steps/tool.rs

+2-10
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ impl Step for Rustdoc {
469469
features.push("jemalloc".to_string());
470470
}
471471

472-
let mut cargo = prepare_tool_cargo(
472+
let cargo = prepare_tool_cargo(
473473
builder,
474474
build_compiler,
475475
Mode::ToolRustc,
@@ -480,10 +480,6 @@ impl Step for Rustdoc {
480480
features.as_slice(),
481481
);
482482

483-
if builder.config.rustc_parallel {
484-
cargo.rustflag("--cfg=parallel_compiler");
485-
}
486-
487483
let _guard = builder.msg_tool(
488484
Mode::ToolRustc,
489485
"rustdoc",
@@ -732,7 +728,7 @@ impl Step for LlvmBitcodeLinker {
732728
builder.ensure(compile::Std::new(self.compiler, self.compiler.host));
733729
builder.ensure(compile::Rustc::new(self.compiler, self.target));
734730

735-
let mut cargo = prepare_tool_cargo(
731+
let cargo = prepare_tool_cargo(
736732
builder,
737733
self.compiler,
738734
Mode::ToolRustc,
@@ -743,10 +739,6 @@ impl Step for LlvmBitcodeLinker {
743739
&self.extra_features,
744740
);
745741

746-
if builder.config.rustc_parallel {
747-
cargo.rustflag("--cfg=parallel_compiler");
748-
}
749-
750742
builder.run(&mut cargo.into());
751743

752744
let tool_out = builder

0 commit comments

Comments
 (0)