From 9ad81cbb53f01eb7ec9550d009319b49a22141bd Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Tue, 4 Mar 2025 11:45:22 -0500 Subject: [PATCH 1/5] Trace exact Cargo command arguments --- compiler/base/orchestrator/src/coordinator.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/base/orchestrator/src/coordinator.rs b/compiler/base/orchestrator/src/coordinator.rs index 62f24e54..06384489 100644 --- a/compiler/base/orchestrator/src/coordinator.rs +++ b/compiler/base/orchestrator/src/coordinator.rs @@ -1802,6 +1802,8 @@ impl Container { .await .context(AcquirePermitSnafu)?; + trace!(?execute_cargo, "starting cargo task"); + let (stdin_tx, mut stdin_rx) = mpsc::channel(8); let (stdout_tx, stdout_rx) = mpsc::channel(8); let (stderr_tx, stderr_rx) = mpsc::channel(8); From af1e430160acfa3cf39ba522c17ff2b423a60aba Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Tue, 4 Mar 2025 16:21:17 -0500 Subject: [PATCH 2/5] Add a macro for hashmap key-values --- compiler/base/orchestrator/src/coordinator.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/compiler/base/orchestrator/src/coordinator.rs b/compiler/base/orchestrator/src/coordinator.rs index 06384489..784b4afc 100644 --- a/compiler/base/orchestrator/src/coordinator.rs +++ b/compiler/base/orchestrator/src/coordinator.rs @@ -33,6 +33,14 @@ use crate::{ DropErrorDetailsExt, TaskAbortExt as _, }; +macro_rules! kvs { + ($($k:expr => $v:expr),+$(,)?) => { + [ + $((Into::into($k), Into::into($v)),)+ + ].into_iter() + }; +} + pub mod limits; #[derive(Debug, Clone, PartialEq, Eq)] @@ -393,7 +401,7 @@ impl LowerRequest for ExecuteRequest { let mut envs = HashMap::new(); if self.backtrace { - envs.insert("RUST_BACKTRACE".to_owned(), "1".to_owned()); + envs.extend(kvs!("RUST_BACKTRACE" => "1")); } ExecuteCommandRequest { @@ -522,7 +530,7 @@ impl LowerRequest for CompileRequest { } let mut envs = HashMap::new(); if self.backtrace { - envs.insert("RUST_BACKTRACE".to_owned(), "1".to_owned()); + envs.extend(kvs!("RUST_BACKTRACE" => "1")); } ExecuteCommandRequest { @@ -685,7 +693,7 @@ impl LowerRequest for MiriRequest { ExecuteCommandRequest { cmd: "cargo".to_owned(), args: vec!["miri-playground".to_owned()], - envs: HashMap::from_iter([("MIRIFLAGS".to_owned(), miriflags)]), + envs: kvs!("MIRIFLAGS" => miriflags).collect(), cwd: None, } } From ac81765bdce7480ce84dd87c82dc19559ea5f456 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Tue, 4 Mar 2025 16:22:38 -0500 Subject: [PATCH 3/5] Skip our miri-playground wrapper script We don't really need it anymore now that we can control environment variables from outside the container. This also opens us up to calling `cargo miri test` in addition to the currently hardcoded `cargo miri run`. --- compiler/base/Dockerfile | 1 + compiler/base/orchestrator/src/coordinator.rs | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/compiler/base/Dockerfile b/compiler/base/Dockerfile index 09c27917..cfbdaa05 100644 --- a/compiler/base/Dockerfile +++ b/compiler/base/Dockerfile @@ -111,6 +111,7 @@ RUN rm src/*.rs COPY --from=build-orchestrator /playground/.cargo/bin/worker /playground/.cargo/bin/worker COPY --from=wasm-tools /playground/.cargo/bin/wasm-tools /playground/.cargo/bin COPY --chown=playground cargo-wasm /playground/.cargo/bin +# `cargo-miri-playground` is vestigial and can be removed after a while COPY --chown=playground cargo-miri-playground /playground/.cargo/bin ENTRYPOINT ["/playground/tools/entrypoint.sh"] diff --git a/compiler/base/orchestrator/src/coordinator.rs b/compiler/base/orchestrator/src/coordinator.rs index 784b4afc..f83a0f70 100644 --- a/compiler/base/orchestrator/src/coordinator.rs +++ b/compiler/base/orchestrator/src/coordinator.rs @@ -688,12 +688,23 @@ impl LowerRequest for MiriRequest { miriflags.push("-Zmiri-tree-borrows"); } + miriflags.push("-Zmiri-disable-isolation"); + let miriflags = miriflags.join(" "); ExecuteCommandRequest { cmd: "cargo".to_owned(), - args: vec!["miri-playground".to_owned()], - envs: kvs!("MIRIFLAGS" => miriflags).collect(), + args: ["miri", "run"].map(Into::into).into(), + envs: kvs! { + "MIRIFLAGS" => miriflags, + // Be sure that `cargo miri` will not build a new + // sysroot. Creating a sysroot takes a while and Miri + // will build one by default if it's missing. If + // `MIRI_SYSROOT` is set and the sysroot is missing, + // it will error instead. + "MIRI_SYSROOT" => "/playground/.cache/miri", + } + .collect(), cwd: None, } } @@ -3971,8 +3982,7 @@ mod tests { #[tokio::test] #[snafu::report] async fn miri() -> Result<()> { - // cargo-miri-playground only exists inside the container - let coordinator = new_coordinator_docker(); + let coordinator = new_coordinator(); let req = MiriRequest { code: r#" From 5a6571212e63ac2dd224ce2031980b3c645bf14d Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Tue, 4 Mar 2025 11:44:05 -0500 Subject: [PATCH 4/5] Support running `miri test` in the orchestrator --- compiler/base/orchestrator/src/coordinator.rs | 36 ++++++++++++++++++- ui/src/metrics.rs | 3 +- ui/src/server_axum.rs | 1 + 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/compiler/base/orchestrator/src/coordinator.rs b/compiler/base/orchestrator/src/coordinator.rs index f83a0f70..c6477581 100644 --- a/compiler/base/orchestrator/src/coordinator.rs +++ b/compiler/base/orchestrator/src/coordinator.rs @@ -668,6 +668,7 @@ pub struct MiriRequest { pub channel: Channel, pub crate_type: CrateType, pub edition: Edition, + pub tests: bool, pub aliasing_model: AliasingModel, pub code: String, } @@ -692,9 +693,11 @@ impl LowerRequest for MiriRequest { let miriflags = miriflags.join(" "); + let subcommand = if self.tests { "test" } else { "run" }; + ExecuteCommandRequest { cmd: "cargo".to_owned(), - args: ["miri", "run"].map(Into::into).into(), + args: ["miri", subcommand].map(Into::into).into(), envs: kvs! { "MIRIFLAGS" => miriflags, // Be sure that `cargo miri` will not build a new @@ -3975,6 +3978,7 @@ mod tests { channel: Channel::Nightly, crate_type: CrateType::Binary, edition: Edition::Rust2021, + tests: false, aliasing_model: AliasingModel::Stacked, code: String::new(), }; @@ -4007,6 +4011,36 @@ mod tests { Ok(()) } + #[tokio::test] + #[snafu::report] + async fn miri_tests() -> Result<()> { + let coordinator = new_coordinator(); + + let req = MiriRequest { + tests: true, + code: r#" + #[test] + fn oops() { + unsafe { core::mem::MaybeUninit::::uninit().assume_init() }; + } + "# + .into(), + ..ARBITRARY_MIRI_REQUEST + }; + + let response = coordinator.miri(req).with_timeout().await.unwrap(); + + assert!(!response.success, "stderr: {}", response.stderr); + + assert_contains!(response.stderr, "Undefined Behavior"); + assert_contains!(response.stderr, "using uninitialized data"); + assert_contains!(response.stderr, "operation requires initialized memory"); + + coordinator.shutdown().await?; + + Ok(()) + } + const ARBITRARY_MACRO_EXPANSION_REQUEST: MacroExpansionRequest = MacroExpansionRequest { channel: Channel::Nightly, crate_type: CrateType::Library(LibraryType::Cdylib), diff --git a/ui/src/metrics.rs b/ui/src/metrics.rs index 973e5d4e..fac11b0d 100644 --- a/ui/src/metrics.rs +++ b/ui/src/metrics.rs @@ -361,6 +361,7 @@ impl HasLabelsCore for coordinator::MiriRequest { channel, crate_type, edition, + tests, aliasing_model: _, code: _, } = *self; @@ -371,7 +372,7 @@ impl HasLabelsCore for coordinator::MiriRequest { mode: None, edition: Some(Some(edition)), crate_type: Some(crate_type), - tests: None, + tests: Some(tests), backtrace: None, } } diff --git a/ui/src/server_axum.rs b/ui/src/server_axum.rs index f4d38393..58b5e40e 100644 --- a/ui/src/server_axum.rs +++ b/ui/src/server_axum.rs @@ -1300,6 +1300,7 @@ pub(crate) mod api_orchestrator_integration_impls { channel: Channel::Nightly, // TODO: use what user has submitted crate_type: CrateType::Binary, // TODO: use what user has submitted edition: parse_edition(&edition)?, + tests: false, aliasing_model, code, }) From a54907db6b39407b83172b5f7fbcf604a046a1a4 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Tue, 4 Mar 2025 12:56:26 -0500 Subject: [PATCH 5/5] Use new `miri test` functionality from the frontend --- ui/frontend/ToolsMenu.tsx | 5 ++++- ui/frontend/reducers/output/miri.ts | 1 + ui/frontend/selectors/index.ts | 5 +++-- ui/src/public_http_api.rs | 2 ++ ui/src/server_axum.rs | 3 ++- 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ui/frontend/ToolsMenu.tsx b/ui/frontend/ToolsMenu.tsx index f8450aa8..cbd732f5 100644 --- a/ui/frontend/ToolsMenu.tsx +++ b/ui/frontend/ToolsMenu.tsx @@ -26,6 +26,9 @@ const ToolsMenu: React.FC = props => { const nightlyVersion = useAppSelector(selectors.nightlyVersionText); const nightlyVersionDetails = useAppSelector(selectors.nightlyVersionDetailsText); + const miriRunningTests = useAppSelector(selectors.runAsTest); + const miriText = miriRunningTests ? "these tests" : "this program"; + const dispatch = useAppDispatch(); const clippy = useCallback(() => { dispatch(performClippy()); @@ -62,7 +65,7 @@ const ToolsMenu: React.FC = props => { name="Miri" onClick={miri}>
- Execute this program in the Miri interpreter to detect certain + Execute {miriText} in the Miri interpreter to detect certain cases of undefined behavior (like out-of-bounds memory access).
{miriVersion} ({miriVersionDetails}) diff --git a/ui/frontend/reducers/output/miri.ts b/ui/frontend/reducers/output/miri.ts index 0ba94e84..491a33d7 100644 --- a/ui/frontend/reducers/output/miri.ts +++ b/ui/frontend/reducers/output/miri.ts @@ -22,6 +22,7 @@ interface State { interface MiriRequestBody { code: string; edition: string; + tests: boolean; aliasingModel: AliasingModel; } diff --git a/ui/frontend/selectors/index.ts b/ui/frontend/selectors/index.ts index 69b93912..1357494d 100644 --- a/ui/frontend/selectors/index.ts +++ b/ui/frontend/selectors/index.ts @@ -397,9 +397,10 @@ export const formatRequestSelector = createSelector( export const miriRequestSelector = createSelector( editionSelector, - codeSelector, + runAsTest, aliasingModelSelector, - (edition, code, aliasingModel) => ({ edition, code, aliasingModel }), + codeSelector, + (edition, tests, aliasingModel, code, ) => ({ edition, tests, aliasingModel, code }), ); export const macroExpansionRequestSelector = createSelector( diff --git a/ui/src/public_http_api.rs b/ui/src/public_http_api.rs index 6ca71b9f..4ab17b12 100644 --- a/ui/src/public_http_api.rs +++ b/ui/src/public_http_api.rs @@ -103,6 +103,8 @@ pub(crate) struct MiriRequest { pub(crate) code: String, #[serde(default)] pub(crate) edition: String, + #[serde(default)] + pub(crate) tests: bool, #[serde(default, rename = "aliasingModel")] pub(crate) aliasing_model: Option, } diff --git a/ui/src/server_axum.rs b/ui/src/server_axum.rs index 58b5e40e..fb1c6126 100644 --- a/ui/src/server_axum.rs +++ b/ui/src/server_axum.rs @@ -1288,6 +1288,7 @@ pub(crate) mod api_orchestrator_integration_impls { let api::MiriRequest { code, edition, + tests, aliasing_model, } = other; @@ -1300,7 +1301,7 @@ pub(crate) mod api_orchestrator_integration_impls { channel: Channel::Nightly, // TODO: use what user has submitted crate_type: CrateType::Binary, // TODO: use what user has submitted edition: parse_edition(&edition)?, - tests: false, + tests, aliasing_model, code, })