From 73f7c7cfe5874e6aa8991a84da8a4b6b2e84207b Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 20 Feb 2022 23:52:54 -0500 Subject: [PATCH 1/4] Add a magic comment syntax to set MIRIFLAGS --- compiler/miri/cargo-miri-playground | 1 - ui/src/sandbox.rs | 152 +++++++++++++++++++++++++++- 2 files changed, 147 insertions(+), 6 deletions(-) diff --git a/compiler/miri/cargo-miri-playground b/compiler/miri/cargo-miri-playground index 98781de1e..d3302941b 100755 --- a/compiler/miri/cargo-miri-playground +++ b/compiler/miri/cargo-miri-playground @@ -3,5 +3,4 @@ set -eu export MIRI_SYSROOT=~/.cache/miri/HOST -export MIRIFLAGS="-Zmiri-disable-isolation" exec cargo miri run diff --git a/ui/src/sandbox.rs b/ui/src/sandbox.rs index c77daebba..843d4ef31 100644 --- a/ui/src/sandbox.rs +++ b/ui/src/sandbox.rs @@ -1,3 +1,4 @@ +use regex::Regex; use serde_derive::Deserialize; use snafu::{ResultExt, Snafu}; use std::{ffi::OsStr, fmt, io, os::unix::fs::PermissionsExt, string, time::Duration}; @@ -280,6 +281,27 @@ fn set_execution_environment( cmd.apply_backtrace(&req); } +fn parse_magic_comments(code: &str) -> Vec<(&'static str, String)> { + lazy_static::lazy_static! { + pub static ref MIRIFLAGS: Regex = Regex::new(r#"(///|//!|//)\s*MIRIFLAGS\s*=\s*(.*)"#).unwrap(); + } + + fn match_of(line: &str, pat: &Regex) -> Option { + pat.captures(line.trim()) + .and_then(|caps| caps.get(2)) + .map(|mat| mat.as_str().trim_matches(&['\'', '"'][..]).to_string()) + } + + let mut settings = Vec::new(); + if let Some(first_line) = code.trim().lines().next() { + if let Some(miri_flags) = match_of(first_line, &*MIRIFLAGS) { + settings.push(("MIRIFLAGS", miri_flags)); + } + } + + settings +} + pub mod fut { use snafu::prelude::*; use std::{ @@ -292,9 +314,9 @@ pub mod fut { use tokio::{fs, process::Command, time}; use super::{ - basic_secure_docker_command, build_execution_command, set_execution_environment, - vec_to_str, wide_open_permissions, BacktraceRequest, Channel, ClippyRequest, - ClippyResponse, CompileRequest, CompileResponse, CompileTarget, + basic_secure_docker_command, build_execution_command, parse_magic_comments, + set_execution_environment, vec_to_str, wide_open_permissions, BacktraceRequest, Channel, + ClippyRequest, ClippyResponse, CompileRequest, CompileResponse, CompileTarget, CompilerExecutionTimedOutSnafu, CrateInformation, CrateInformationInner, CrateType, CrateTypeRequest, DemangleAssembly, DockerCommandExt, EditionRequest, ExecuteRequest, ExecuteResponse, FormatRequest, FormatResponse, MacroExpansionRequest, @@ -454,7 +476,9 @@ pub mod fut { pub async fn miri(&self, req: &MiriRequest) -> Result { self.write_source_code(&req.code).await?; - let command = self.miri_command(req); + + let settings = parse_magic_comments(&req.code); + let command = self.miri_command(settings, req); let output = run_command_with_timeout(command).await?; @@ -651,10 +675,23 @@ pub mod fut { cmd } - fn miri_command(&self, req: impl EditionRequest) -> Command { + fn miri_command( + &self, + env_settings: Vec<(&'static str, String)>, + req: impl EditionRequest, + ) -> Command { let mut cmd = self.docker_command(None); cmd.apply_edition(req); + let miri_env = + if let Some((_, flags)) = env_settings.iter().find(|(k, _)| k == &"MIRIFLAGS") { + format!("MIRIFLAGS={} -Zmiri-disable-isolation", flags) + } else { + "MIRIFLAGS=-Zmiri-disable-isolation".to_string() + }; + + cmd.args(&["--env", &miri_env]); + cmd.arg("miri").args(&["cargo", "miri-playground"]); log::debug!("Miri command is {:?}", cmd); @@ -1653,6 +1690,111 @@ mod test { Ok(()) } + #[test] + fn magic_comments() { + let _singleton = one_test_at_a_time(); + + let expected = vec![("MIRIFLAGS", "-Zmiri-tag-raw-pointers".to_string())]; + + // Every comment syntax + let code = r#"//MIRIFLAGS=-Zmiri-tag-raw-pointers"#; + assert_eq!(parse_magic_comments(code), expected); + + let code = r#"///MIRIFLAGS=-Zmiri-tag-raw-pointers"#; + assert_eq!(parse_magic_comments(code), expected); + + let code = r#"//!MIRIFLAGS=-Zmiri-tag-raw-pointers"#; + assert_eq!(parse_magic_comments(code), expected); + + // Whitespace is ignored + let code = r#"// MIRIFLAGS = -Zmiri-tag-raw-pointers"#; + assert_eq!(parse_magic_comments(code), expected); + + // Both quotes are stripped + let code = r#"//MIRIFLAGS='-Zmiri-tag-raw-pointers'"#; + assert_eq!(parse_magic_comments(code), expected); + + let code = r#"//MIRIFLAGS="-Zmiri-tag-raw-pointers""#; + assert_eq!(parse_magic_comments(code), expected); + + // Leading spaces are ignored + let code = r#" + //MIRIFLAGS=-Zmiri-tag-raw-pointers"#; + assert_eq!(parse_magic_comments(code), expected); + + let expected = vec![( + "MIRIFLAGS", + "-Zmiri-tag-raw-pointers -Zmiri-symbolic-alignment-check".to_string(), + )]; + + // Doesn't break up flags even if they contain whitespace + let code = r#"//MIRIFLAGS="-Zmiri-tag-raw-pointers -Zmiri-symbolic-alignment-check""#; + assert_eq!(parse_magic_comments(code), expected); + + // Even if they aren't wrapped in quoted (possibly dubious) + let code = r#"//MIRIFLAGS=-Zmiri-tag-raw-pointers -Zmiri-symbolic-alignment-check"#; + assert_eq!(parse_magic_comments(code), expected); + } + + #[test] + fn miriflags_work() -> Result<()> { + let _singleton = one_test_at_a_time(); + + let code = r#" + fn main() { + let a = 0u8; + let ptr = &a as *const u8 as usize as *const u8; + unsafe { + println!("{}", *ptr); + } + } + "#; + + let req = MiriRequest { + code: code.to_string(), + edition: None, + }; + + let sb = Sandbox::new()?; + let resp = sb.miri(&req)?; + + assert!(resp.success); + + let code = r#" + // MIRIFLAGS=-Zmiri-tag-raw-pointers + fn main() { + let a = 0u8; + let ptr = &a as *const u8 as usize as *const u8; + unsafe { + println!("{}", *ptr); + } + } + "#; + + let req = MiriRequest { + code: code.to_string(), + edition: None, + }; + + let sb = Sandbox::new()?; + let resp = sb.miri(&req)?; + + assert!(!resp.success); + + assert!( + resp.stderr + .contains("trying to reborrow for SharedReadOnly"), + "was: {}", + resp.stderr + ); + assert!( + resp.stderr.contains("but parent tag"), + "was: {}", + resp.stderr + ); + Ok(()) + } + #[test] fn network_connections_are_disabled() { let _singleton = one_test_at_a_time(); From e1c07ef72da4067d0510b931a8e94b683137f009 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Wed, 23 Feb 2022 17:17:03 -0500 Subject: [PATCH 2/4] Don't default to diabling isolation, but demo it in docs --- ui/frontend/Help.tsx | 7 +++++-- ui/frontend/package.json | 2 +- ui/src/sandbox.rs | 11 +++-------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/ui/frontend/Help.tsx b/ui/frontend/Help.tsx index 48e7c35a6..ab860aef6 100644 --- a/ui/frontend/Help.tsx +++ b/ui/frontend/Help.tsx @@ -37,7 +37,8 @@ const CLIPPY_EXAMPLE = `fn main() { } }`; -const MIRI_EXAMPLE = `fn main() { +const MIRI_EXAMPLE = `// MIRIFLAGS=-Zmiri-disable-isolation +fn main() { let mut a: [u8; 0] = []; unsafe { *a.get_unchecked_mut(1) = 1; @@ -154,7 +155,9 @@ const Help: React.SFC = () => { Miri is an interpreter for Rust’s mid-level intermediate representation (MIR) and can be used to detect certain kinds of undefined behavior in your unsafe Rust code. Click on the Miri button in - the Tools menu to check. + the Tools menu to check. When running code with Miri, if the first + source line is a comment following a bash-like syntax for setting environment + variables, it will be used to set the MIRIFLAGS environment variable.

diff --git a/ui/frontend/package.json b/ui/frontend/package.json index 66c8e5334..7f7830265 100644 --- a/ui/frontend/package.json +++ b/ui/frontend/package.json @@ -81,7 +81,7 @@ "webpack-cli": "^4.5.0" }, "engines": { - "node": "^16.13.1" + "node": "^17.3.0" }, "scripts": { "test": "jest", diff --git a/ui/src/sandbox.rs b/ui/src/sandbox.rs index 843d4ef31..0396a3f3c 100644 --- a/ui/src/sandbox.rs +++ b/ui/src/sandbox.rs @@ -683,14 +683,9 @@ pub mod fut { let mut cmd = self.docker_command(None); cmd.apply_edition(req); - let miri_env = - if let Some((_, flags)) = env_settings.iter().find(|(k, _)| k == &"MIRIFLAGS") { - format!("MIRIFLAGS={} -Zmiri-disable-isolation", flags) - } else { - "MIRIFLAGS=-Zmiri-disable-isolation".to_string() - }; - - cmd.args(&["--env", &miri_env]); + if let Some((_, flags)) = env_settings.iter().find(|(k, _)| k == &"MIRIFLAGS") { + cmd.args(&["--env", &format!("MIRIFLAGS={}", flags)]); + } cmd.arg("miri").args(&["cargo", "miri-playground"]); From 605f1fee57677d756bf8e1d86b93f07ed34b49c2 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Mon, 28 Feb 2022 17:01:44 -0500 Subject: [PATCH 3/4] Parse magic comment on lines after the first if they are not indented --- ui/src/sandbox.rs | 77 +++++++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 42 deletions(-) diff --git a/ui/src/sandbox.rs b/ui/src/sandbox.rs index 0396a3f3c..eb799c9af 100644 --- a/ui/src/sandbox.rs +++ b/ui/src/sandbox.rs @@ -281,25 +281,25 @@ fn set_execution_environment( cmd.apply_backtrace(&req); } -fn parse_magic_comments(code: &str) -> Vec<(&'static str, String)> { +fn parse_miri_flags(code: &str) -> Option { lazy_static::lazy_static! { pub static ref MIRIFLAGS: Regex = Regex::new(r#"(///|//!|//)\s*MIRIFLAGS\s*=\s*(.*)"#).unwrap(); } fn match_of(line: &str, pat: &Regex) -> Option { - pat.captures(line.trim()) + pat.captures(line) .and_then(|caps| caps.get(2)) .map(|mat| mat.as_str().trim_matches(&['\'', '"'][..]).to_string()) } - let mut settings = Vec::new(); - if let Some(first_line) = code.trim().lines().next() { - if let Some(miri_flags) = match_of(first_line, &*MIRIFLAGS) { - settings.push(("MIRIFLAGS", miri_flags)); + for line in code.trim().lines() { + let line = line.trim_end(); + if let Some(miri_flags) = match_of(line, &*MIRIFLAGS) { + return Some(miri_flags); } } - settings + None } pub mod fut { @@ -314,7 +314,7 @@ pub mod fut { use tokio::{fs, process::Command, time}; use super::{ - basic_secure_docker_command, build_execution_command, parse_magic_comments, + basic_secure_docker_command, build_execution_command, parse_miri_flags, set_execution_environment, vec_to_str, wide_open_permissions, BacktraceRequest, Channel, ClippyRequest, ClippyResponse, CompileRequest, CompileResponse, CompileTarget, CompilerExecutionTimedOutSnafu, CrateInformation, CrateInformationInner, CrateType, @@ -477,8 +477,8 @@ pub mod fut { pub async fn miri(&self, req: &MiriRequest) -> Result { self.write_source_code(&req.code).await?; - let settings = parse_magic_comments(&req.code); - let command = self.miri_command(settings, req); + let miri_flags = parse_miri_flags(&req.code); + let command = self.miri_command(miri_flags, req); let output = run_command_with_timeout(command).await?; @@ -675,15 +675,11 @@ pub mod fut { cmd } - fn miri_command( - &self, - env_settings: Vec<(&'static str, String)>, - req: impl EditionRequest, - ) -> Command { + fn miri_command(&self, miri_flags: Option, req: impl EditionRequest) -> Command { let mut cmd = self.docker_command(None); cmd.apply_edition(req); - if let Some((_, flags)) = env_settings.iter().find(|(k, _)| k == &"MIRIFLAGS") { + if let Some(flags) = miri_flags { cmd.args(&["--env", &format!("MIRIFLAGS={}", flags)]); } @@ -1668,17 +1664,7 @@ mod test { assert!( resp.stderr - .contains("pointer must be in-bounds at offset 1"), - "was: {}", - resp.stderr - ); - assert!( - resp.stderr.contains("outside bounds of alloc"), - "was: {}", - resp.stderr - ); - assert!( - resp.stderr.contains("which has size 0"), + .contains("has size 0, so pointer to 1 byte starting at offset 0 is out-of-bounds"), "was: {}", resp.stderr ); @@ -1689,46 +1675,53 @@ mod test { fn magic_comments() { let _singleton = one_test_at_a_time(); - let expected = vec![("MIRIFLAGS", "-Zmiri-tag-raw-pointers".to_string())]; + let expected = Some("-Zmiri-tag-raw-pointers".to_string()); // Every comment syntax let code = r#"//MIRIFLAGS=-Zmiri-tag-raw-pointers"#; - assert_eq!(parse_magic_comments(code), expected); + assert_eq!(parse_miri_flags(code), expected); let code = r#"///MIRIFLAGS=-Zmiri-tag-raw-pointers"#; - assert_eq!(parse_magic_comments(code), expected); + assert_eq!(parse_miri_flags(code), expected); let code = r#"//!MIRIFLAGS=-Zmiri-tag-raw-pointers"#; - assert_eq!(parse_magic_comments(code), expected); + assert_eq!(parse_miri_flags(code), expected); // Whitespace is ignored let code = r#"// MIRIFLAGS = -Zmiri-tag-raw-pointers"#; - assert_eq!(parse_magic_comments(code), expected); + assert_eq!(parse_miri_flags(code), expected); // Both quotes are stripped let code = r#"//MIRIFLAGS='-Zmiri-tag-raw-pointers'"#; - assert_eq!(parse_magic_comments(code), expected); + assert_eq!(parse_miri_flags(code), expected); let code = r#"//MIRIFLAGS="-Zmiri-tag-raw-pointers""#; - assert_eq!(parse_magic_comments(code), expected); + assert_eq!(parse_miri_flags(code), expected); + + // Leading code is ignored + let code = r#" +fn main() {} +//MIRIFLAGS=-Zmiri-tag-raw-pointers"#; + assert_eq!(parse_miri_flags(code), expected); + + // A leading space on the first line is ignored + let code = r#" //MIRIFLAGS=-Zmiri-tag-raw-pointers"#; + assert_eq!(parse_miri_flags(code), expected); - // Leading spaces are ignored + // But on any other line, it disagles the magic let code = r#" //MIRIFLAGS=-Zmiri-tag-raw-pointers"#; - assert_eq!(parse_magic_comments(code), expected); + assert_eq!(parse_miri_flags(code), expected); - let expected = vec![( - "MIRIFLAGS", - "-Zmiri-tag-raw-pointers -Zmiri-symbolic-alignment-check".to_string(), - )]; + let expected = Some("-Zmiri-tag-raw-pointers -Zmiri-symbolic-alignment-check".to_string()); // Doesn't break up flags even if they contain whitespace let code = r#"//MIRIFLAGS="-Zmiri-tag-raw-pointers -Zmiri-symbolic-alignment-check""#; - assert_eq!(parse_magic_comments(code), expected); + assert_eq!(parse_miri_flags(code), expected); // Even if they aren't wrapped in quoted (possibly dubious) let code = r#"//MIRIFLAGS=-Zmiri-tag-raw-pointers -Zmiri-symbolic-alignment-check"#; - assert_eq!(parse_magic_comments(code), expected); + assert_eq!(parse_miri_flags(code), expected); } #[test] From 0e47158e5521d560f5d0247137843bb2dacb6b34 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Mon, 28 Feb 2022 17:08:30 -0500 Subject: [PATCH 4/4] Accept the comment anywhere --- ui/frontend/package.json | 2 +- ui/src/sandbox.rs | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/ui/frontend/package.json b/ui/frontend/package.json index 7f7830265..66c8e5334 100644 --- a/ui/frontend/package.json +++ b/ui/frontend/package.json @@ -81,7 +81,7 @@ "webpack-cli": "^4.5.0" }, "engines": { - "node": "^17.3.0" + "node": "^16.13.1" }, "scripts": { "test": "jest", diff --git a/ui/src/sandbox.rs b/ui/src/sandbox.rs index eb799c9af..4bff06d76 100644 --- a/ui/src/sandbox.rs +++ b/ui/src/sandbox.rs @@ -293,7 +293,7 @@ fn parse_miri_flags(code: &str) -> Option { } for line in code.trim().lines() { - let line = line.trim_end(); + let line = line.trim(); if let Some(miri_flags) = match_of(line, &*MIRIFLAGS) { return Some(miri_flags); } @@ -1704,15 +1704,10 @@ fn main() {} //MIRIFLAGS=-Zmiri-tag-raw-pointers"#; assert_eq!(parse_miri_flags(code), expected); - // A leading space on the first line is ignored + // Leading whitespace is ignored, flags still parse let code = r#" //MIRIFLAGS=-Zmiri-tag-raw-pointers"#; assert_eq!(parse_miri_flags(code), expected); - // But on any other line, it disagles the magic - let code = r#" - //MIRIFLAGS=-Zmiri-tag-raw-pointers"#; - assert_eq!(parse_miri_flags(code), expected); - let expected = Some("-Zmiri-tag-raw-pointers -Zmiri-symbolic-alignment-check".to_string()); // Doesn't break up flags even if they contain whitespace