Skip to content

check-aux: test core, alloc, std in Miri #123506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions library/alloc/benches/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Disabling on android for the time being
// See https://github.com/rust-lang/rust/issues/73535#event-3477699747
#![cfg(not(target_os = "android"))]
// Disabling in Miri as these would take too long.
#![cfg(not(miri))]
#![feature(btree_extract_if)]
#![feature(iter_next_chunk)]
#![feature(repr_simd)]
Expand Down
4 changes: 3 additions & 1 deletion library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,9 @@ impl<T, A: Allocator> Arc<T, A> {
///
/// // Create a long list and clone it
/// let mut x = LinkedList::new();
/// for i in 0..100000 {
/// let size = 100000;
/// # let size = if cfg!(miri) { 100 } else { size };
/// for i in 0..size {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that an okay approach to make the test shorter in Miri? The alternative would be more convoluted but would let the code look as before on the web view:

# let size = if cfg!(miri) { 100 } else { 100000 };
# for i in 0..size {
# /*
for i in 0..100000 {
# */

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me, especially given it's relatively isolated. Realistically I'm not convinced this example needs a huge length (100 elements or 100,000 is about equally bad if you're pushing a new stack frame per element, IMO, given that you might already be close to the end of the allowed stack when starting to execute).

/// x.push(i); // Adds i to the front of x
/// }
/// let y = x.clone();
Expand Down
2 changes: 2 additions & 0 deletions library/core/benches/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// wasm32 does not support benches (no time).
#![cfg(not(target_arch = "wasm32"))]
// Disabling in Miri as these would take too long.
#![cfg(not(miri))]
#![feature(flt2dec)]
#![feature(test)]
#![feature(trusted_random_access)]
Expand Down
2 changes: 2 additions & 0 deletions library/std/benches/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// Disabling in Miri as these would take too long.
#![cfg(not(miri))]
#![feature(test)]

extern crate test;
Expand Down
1 change: 1 addition & 0 deletions library/std/tests/process_spawning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::str;
mod common;

#[test]
#[cfg_attr(miri, ignore)] // Process spawning not supported by Miri
fn issue_15149() {
// If we're the parent, copy our own binary to a new directory.
let my_path = env::current_exe().unwrap();
Expand Down
1 change: 1 addition & 0 deletions library/std/tests/switch-stdout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ fn switch_stdout_to(file: OwnedHandle) -> OwnedHandle {
}

#[test]
#[cfg_attr(miri, ignore)] // dup/SetStdHandle not supported by Miri
fn switch_stdout() {
let temp = common::tmpdir();
let path = temp.join("switch-stdout-output");
Expand Down
3 changes: 2 additions & 1 deletion library/std/tests/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use std::time::Duration;

#[test]
#[cfg_attr(target_os = "emscripten", ignore)]
fn sleep() {
#[cfg_attr(miri, ignore)] // Miri does not like the thread leak
fn sleep_very_long() {
let finished = Arc::new(Mutex::new(false));
let t_finished = finished.clone();
thread::spawn(move || {
Expand Down
30 changes: 30 additions & 0 deletions src/bootstrap/mk/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,36 @@ check-aux:
src/tools/cargo \
src/tools/cargotest \
$(BOOTSTRAP_ARGS)
# Run standard library tests in Miri.
# We use a 64bit little-endian and a 32bit big-endian target for max coverage.
$(Q)BOOTSTRAP_SKIP_TARGET_SANITY=1 \
$(BOOTSTRAP) miri --stage 2 \
--target x86_64-unknown-linux-gnu,mips-unknown-linux-gnu \
library/core \
library/alloc \
--no-doc
# Some doctests have intentional memory leaks.
# Also, they work only on the host.
$(Q)MIRIFLAGS="-Zmiri-ignore-leaks -Zmiri-disable-isolation" \
$(BOOTSTRAP) miri --stage 2 \
library/core \
library/alloc \
--doc
# In `std` we cannot test everything.
$(Q)MIRIFLAGS="-Zmiri-disable-isolation" BOOTSTRAP_SKIP_TARGET_SANITY=1 \
$(BOOTSTRAP) miri --stage 2 library/std \
--no-doc -- \
Copy link
Member Author

@RalfJung RalfJung Apr 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't add mips-unknown-linux-gnu here as I think std has much less code that depends on bitwidth or endianess, so the extra 10min CI time does not seem worth it. Instead we have the macOS and Windows checks below, as std does contain a lot of OS-specific code. (And it's a 32bit Windows so at least that kind of architecture is covered.)

--skip fs:: --skip net:: --skip process:: --skip sys::pal::
$(Q)MIRIFLAGS="-Zmiri-ignore-leaks -Zmiri-disable-isolation" BOOTSTRAP_SKIP_TARGET_SANITY=1 \
$(BOOTSTRAP) miri --stage 2 library/std \
--doc -- \
--skip fs:: --skip net:: --skip process:: --skip sys::pal::
# Also test some very target-specific modules on other targets.
$(Q)MIRIFLAGS="-Zmiri-disable-isolation" BOOTSTRAP_SKIP_TARGET_SANITY=1 \
$(BOOTSTRAP) miri --stage 2 library/std \
--target aarch64-apple-darwin,i686-pc-windows-gnu \
--no-doc -- \
time:: sync:: thread:: env::
dist:
$(Q)$(BOOTSTRAP) dist $(BOOTSTRAP_ARGS)
distcheck:
Expand Down
31 changes: 21 additions & 10 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2689,16 +2689,27 @@ impl Step for Crate {

match mode {
Mode::Std => {
compile::std_cargo(builder, target, compiler.stage, &mut cargo);
// `std_cargo` actually does the wrong thing: it passes `--sysroot build/host/stage2`,
// but we want to use the force-recompile std we just built in `build/host/stage2-test-sysroot`.
// Override it.
if builder.download_rustc() && compiler.stage > 0 {
let sysroot = builder
.out
.join(compiler.host.triple)
.join(format!("stage{}-test-sysroot", compiler.stage));
cargo.env("RUSTC_SYSROOT", sysroot);
if builder.kind == Kind::Miri {
// We can't use `std_cargo` as that uses `optimized-compiler-builtins` which
// needs host tools for the given target. This is similar to what `compile::Std`
// does when `is_for_mir_opt_tests` is true. There's probably a chance for
// de-duplication here... `std_cargo` should support a mode that avoids needing
// host tools.
cargo
.arg("--manifest-path")
.arg(builder.src.join("library/sysroot/Cargo.toml"));
} else {
compile::std_cargo(builder, target, compiler.stage, &mut cargo);
// `std_cargo` actually does the wrong thing: it passes `--sysroot build/host/stage2`,
// but we want to use the force-recompile std we just built in `build/host/stage2-test-sysroot`.
// Override it.
if builder.download_rustc() && compiler.stage > 0 {
let sysroot = builder
.out
.join(compiler.host.triple)
.join(format!("stage{}-test-sysroot", compiler.stage));
cargo.env("RUSTC_SYSROOT", sysroot);
}
}
}
Mode::Rustc => {
Expand Down
4 changes: 4 additions & 0 deletions src/ci/docker/host-x86_64/x86_64-gnu-aux/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,9 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

# Miri is just too slow with full assertions
ENV NO_DEBUG_ASSERTIONS=1
ENV NO_OVERFLOW_CHECKS=1

ENV RUST_CONFIGURE_ARGS --build=x86_64-unknown-linux-gnu
ENV RUST_CHECK_TARGET check-aux