From e72b06ee296350a951715e72782fe76ecd031653 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 25 Mar 2024 11:01:28 +0100 Subject: [PATCH 1/9] set --sysroot outside the driver rather than messing with the arguments passed to the driver --- README.md | 5 ++--- cargo-miri/src/phases.rs | 23 ++++++++++++++++------- miri-script/src/commands.rs | 18 +++++++++++------- src/bin/miri.rs | 19 ------------------- tests/ui.rs | 7 +++++++ 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 60a1c1fa1d..28f5f112e9 100644 --- a/README.md +++ b/README.md @@ -464,10 +464,9 @@ Moreover, Miri recognizes some environment variables: standard library that it will build and use for interpretation. This directory must point to the `library` subdirectory of a `rust-lang/rust` repository checkout. -* `MIRI_SYSROOT` (recognized by `cargo miri` and the Miri driver) indicates the sysroot to use. When +* `MIRI_SYSROOT` (recognized by `cargo miri` and the test suite) indicates the sysroot to use. When using `cargo miri`, this skips the automatic setup -- only set this if you do not want to use the - automatically created sysroot. For directly invoking the Miri driver, this variable (or a - `--sysroot` flag) is mandatory. When invoking `cargo miri setup`, this indicates where the sysroot + automatically created sysroot. When invoking `cargo miri setup`, this indicates where the sysroot will be put. * `MIRI_TEST_TARGET` (recognized by the test suite and the `./miri` script) indicates which target architecture to test against. `miri` and `cargo miri` accept the `--target` flag for the same diff --git a/cargo-miri/src/phases.rs b/cargo-miri/src/phases.rs index 3f6c484a05..482b3dcbdf 100644 --- a/cargo-miri/src/phases.rs +++ b/cargo-miri/src/phases.rs @@ -172,8 +172,6 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { // Forward all further arguments (not consumed by `ArgSplitFlagValue`) to cargo. cmd.args(args); - // Let it know where the Miri sysroot lives. - cmd.env("MIRI_SYSROOT", miri_sysroot); // Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation, // i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish // the two codepaths. (That extra argument is why we prefer this over setting `RUSTC`.) @@ -200,10 +198,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { // always applied. However, buggy build scripts (https://github.com/eyre-rs/eyre/issues/84) and // also cargo (https://github.com/rust-lang/cargo/issues/10885) will invoke `rustc` even when // `RUSTC_WRAPPER` is set, bypassing the wrapper. To make sure everything is coherent, we want - // that to be the Miri driver, but acting as rustc, on the target level. (Target, rather than - // host, is needed for cross-interpretation situations.) This is not a perfect emulation of real - // rustc (it might be unable to produce binaries since the sysroot is check-only), but it's as - // close as we can get, and it's good enough for autocfg. + // that to be the Miri driver, but acting as rustc, in host mode. // // In `main`, we need the value of `RUSTC` to distinguish RUSTC_WRAPPER invocations from rustdoc // or TARGET_RUNNER invocations, so we canonicalize it here to make it exceedingly unlikely that @@ -212,7 +207,10 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { // bootstrap `rustc` thing in our way! Instead, we have MIRI_HOST_SYSROOT to use for host // builds. cmd.env("RUSTC", fs::canonicalize(find_miri()).unwrap()); - cmd.env("MIRI_BE_RUSTC", "target"); // we better remember to *unset* this in the other phases! + // In case we get invoked as RUSTC without the wrapper, let's be a host rustc. This makes no + // sense for cross-interpretation situations, but without the wrapper, this will use the host + // sysroot, so asking it to behave like a target build makes even less sense. + cmd.env("MIRI_BE_RUSTC", "host"); // we better remember to *unset* this in the other phases! // Set rustdoc to us as well, so we can run doctests. if let Some(orig_rustdoc) = env::var_os("RUSTDOC") { @@ -220,6 +218,8 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { } cmd.env("RUSTDOC", &cargo_miri_path); + // Forward some crucial information to our own re-invocations. + cmd.env("MIRI_SYSROOT", miri_sysroot); cmd.env("MIRI_LOCAL_CRATES", local_crates(&metadata)); if verbose > 0 { cmd.env("MIRI_VERBOSE", verbose.to_string()); // This makes the other phases verbose. @@ -412,6 +412,9 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { // Arguments are treated very differently depending on whether this crate is // for interpretation by Miri, or for use by a build script / proc macro. if target_crate { + // Set the sysroot. + cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); + // Forward arguments, but patched. let emit_flag = "--emit"; // This hack helps bootstrap run standard library tests in Miri. The issue is as follows: @@ -579,6 +582,12 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner cmd.env(name, val); } + if phase != RunnerPhase::Rustdoc { + // Set the sysroot. Not necessary in rustdoc, where we already set the sysroot when invoking + // rustdoc itself, which will forward that flag when invoking rustc (i.e., us), so the flag + // is present in `info.args`. + cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); + } // Forward rustc arguments. // We need to patch "--extern" filenames because we forced a check-only // build without cargo knowing about that: replace `.rlib` suffix by diff --git a/miri-script/src/commands.rs b/miri-script/src/commands.rs index 55b3b62819..66707dee5e 100644 --- a/miri-script/src/commands.rs +++ b/miri-script/src/commands.rs @@ -2,6 +2,7 @@ use std::env; use std::ffi::OsString; use std::io::Write; use std::ops::Not; +use std::path::PathBuf; use std::process; use std::thread; use std::time; @@ -20,10 +21,11 @@ const JOSH_FILTER: &str = const JOSH_PORT: &str = "42042"; impl MiriEnv { - fn build_miri_sysroot(&mut self, quiet: bool) -> Result<()> { - if self.sh.var("MIRI_SYSROOT").is_ok() { + /// Returns the location of the sysroot. + fn build_miri_sysroot(&mut self, quiet: bool) -> Result { + if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") { // Sysroot already set, use that. - return Ok(()); + return Ok(miri_sysroot.into()); } let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); let Self { toolchain, cargo_extra_flags, .. } = &self; @@ -57,8 +59,8 @@ impl MiriEnv { .with_context(|| "`cargo miri setup` failed")?; panic!("`cargo miri setup` didn't fail again the 2nd time?"); }; - self.sh.set_var("MIRI_SYSROOT", output); - Ok(()) + self.sh.set_var("MIRI_SYSROOT", &output); + Ok(output.into()) } } @@ -505,8 +507,10 @@ impl Command { flags.push("--edition=2021".into()); // keep in sync with `tests/ui.rs`.` } - // Prepare a sysroot. - e.build_miri_sysroot(/* quiet */ true)?; + // Prepare a sysroot, and add it to the flags. + let miri_sysroot = e.build_miri_sysroot(/* quiet */ true)?; + flags.push("--sysroot".into()); + flags.push(miri_sysroot.into()); // Then run the actual command. Also add MIRIFLAGS. let miri_manifest = path!(e.miri_dir / "Cargo.toml"); diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 8fffb91542..67a5bf3d14 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -271,25 +271,6 @@ fn run_compiler( callbacks: &mut (dyn rustc_driver::Callbacks + Send), using_internal_features: std::sync::Arc, ) -> ! { - if target_crate { - // Miri needs a custom sysroot for target crates. - // If no `--sysroot` is given, the `MIRI_SYSROOT` env var is consulted to find where - // that sysroot lives, and that is passed to rustc. - let sysroot_flag = "--sysroot"; - if !args.iter().any(|e| e.starts_with(sysroot_flag)) { - // Using the built-in default here would be plain wrong, so we *require* - // the env var to make sure things make sense. - let miri_sysroot = env::var("MIRI_SYSROOT").unwrap_or_else(|_| { - show_error!( - "Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set" - ) - }); - - args.push(sysroot_flag.to_owned()); - args.push(miri_sysroot); - } - } - // Don't insert `MIRI_DEFAULT_ARGS`, in particular, `--cfg=miri`, if we are building // a "host" crate. That may cause procedural macros (and probably build scripts) to // depend on Miri-only symbols, such as `miri_resolve_frame`: diff --git a/tests/ui.rs b/tests/ui.rs index 7e8d140118..8d985a0520 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -112,6 +112,13 @@ fn run_tests( config.program.envs.push(("RUST_BACKTRACE".into(), Some("1".into()))); // Add some flags we always want. + config.program.args.push( + format!( + "--sysroot={}", + env::var("MIRI_SYSROOT").expect("MIRI_SYSROOT must be set to run the ui test suite") + ) + .into(), + ); config.program.args.push("-Dwarnings".into()); config.program.args.push("-Dunused".into()); config.program.args.push("-Ainternal_features".into()); From 8727602106f3b7df3460af8b678cadf77cf16a1c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 26 Mar 2024 17:17:05 +0100 Subject: [PATCH 2/9] readme updates --- README.md | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 28f5f112e9..948f1ee6c6 100644 --- a/README.md +++ b/README.md @@ -451,35 +451,32 @@ Some native rustc `-Z` flags are also very relevant for Miri: * `-Zmir-emit-retag` controls whether `Retag` statements are emitted. Miri enables this per default because it is needed for [Stacked Borrows] and [Tree Borrows]. -Moreover, Miri recognizes some environment variables: +Moreover, Miri recognizes some environment variables (unless noted otherwise, these are supported +by all intended entry points, i.e. `cargo miri` and `./miri {test,run}`): * `MIRI_AUTO_OPS` indicates whether the automatic execution of rustfmt, clippy and toolchain setup should be skipped. If it is set to `no`, they are skipped. This is used to allow automated IDE actions to avoid the auto ops. * `MIRI_LOG`, `MIRI_BACKTRACE` control logging and backtrace printing during Miri executions, also [see "Testing the Miri driver" in `CONTRIBUTING.md`][testing-miri]. -* `MIRIFLAGS` (recognized by `cargo miri` and the test suite) defines extra - flags to be passed to Miri. -* `MIRI_LIB_SRC` defines the directory where Miri expects the sources of the - standard library that it will build and use for interpretation. This directory - must point to the `library` subdirectory of a `rust-lang/rust` repository - checkout. -* `MIRI_SYSROOT` (recognized by `cargo miri` and the test suite) indicates the sysroot to use. When - using `cargo miri`, this skips the automatic setup -- only set this if you do not want to use the - automatically created sysroot. When invoking `cargo miri setup`, this indicates where the sysroot - will be put. -* `MIRI_TEST_TARGET` (recognized by the test suite and the `./miri` script) indicates which target +* `MIRIFLAGS` defines extra flags to be passed to Miri. +* `MIRI_LIB_SRC` defines the directory where Miri expects the sources of the standard library that + it will build and use for interpretation. This directory must point to the `library` subdirectory + of a `rust-lang/rust` repository checkout. +* `MIRI_SYSROOT` indicates the sysroot to use. When using `cargo miri`, this skips the automatic + setup -- only set this if you do not want to use the automatically created sysroot. When invoking + `cargo miri setup`, this indicates where the sysroot will be put. +* `MIRI_TEST_TARGET` (recognized by `./miri {test,run}`) indicates which target architecture to test against. `miri` and `cargo miri` accept the `--target` flag for the same purpose. -* `MIRI_TEST_THREADS` (recognized by the test suite): set the number of threads to use for running tests. - By default the number of cores is used. -* `MIRI_NO_STD` (recognized by `cargo miri`) makes sure that the target's sysroot is built without - libstd. This allows testing and running no_std programs. - (Miri has a heuristic to detect no-std targets based on the target name; this environment variable - is only needed when that heuristic fails.) -* `RUSTC_BLESS` (recognized by the test suite and `cargo-miri-test/run-test.py`): overwrite all +* `MIRI_TEST_THREADS` (recognized by `./miri test`): set the number of threads to use for running tests. + By default, the number of cores is used. +* `MIRI_NO_STD` makes sure that the target's sysroot is built without libstd. This allows testing + and running no_std programs. (Miri has a heuristic to detect no-std targets based on the target + name; this environment variable is only needed when that heuristic fails.) +* `RUSTC_BLESS` (recognized by `./miri test` and `cargo-miri-test/run-test.py`): overwrite all `stderr` and `stdout` files instead of checking whether the output matches. -* `MIRI_SKIP_UI_CHECKS` (recognized by the test suite): don't check whether the +* `MIRI_SKIP_UI_CHECKS` (recognized by `./miri test`): don't check whether the `stderr` or `stdout` files match the actual output. The following environment variables are *internal* and must not be used by From e428789998feac0b402ede2fb76549ef85e0beac Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 15 Apr 2024 09:35:06 +0200 Subject: [PATCH 3/9] Windows: add basic support for FormatMessageW --- src/helpers.rs | 41 ++++++++++++++++++------------ src/shims/os_str.rs | 1 + src/shims/windows/foreign_items.rs | 38 +++++++++++++++++++++++++++ tests/pass/shims/fs.rs | 2 +- tests/pass/shims/io.rs | 16 ++++++++++-- 5 files changed, 79 insertions(+), 19 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 6e320b60ee..84eb5f832d 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -78,6 +78,17 @@ const UNIX_IO_ERROR_TABLE: &[(&str, std::io::ErrorKind)] = { ("EAGAIN", WouldBlock), ] }; +// This mapping should match `decode_error_kind` in +// . +const WINDOWS_IO_ERROR_TABLE: &[(&str, std::io::ErrorKind)] = { + use std::io::ErrorKind::*; + // FIXME: this is still incomplete. + &[ + ("ERROR_ACCESS_DENIED", PermissionDenied), + ("ERROR_FILE_NOT_FOUND", NotFound), + ("ERROR_INVALID_PARAMETER", InvalidInput), + ] +}; /// Gets an instance for a path. /// @@ -712,20 +723,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } throw_unsup_format!("io error {:?} cannot be translated into a raw os error", err_kind) } else if target.families.iter().any(|f| f == "windows") { - // FIXME: we have to finish implementing the Windows equivalent of this. - use std::io::ErrorKind::*; - Ok(this.eval_windows( - "c", - match err_kind { - NotFound => "ERROR_FILE_NOT_FOUND", - PermissionDenied => "ERROR_ACCESS_DENIED", - _ => - throw_unsup_format!( - "io error {:?} cannot be translated into a raw os error", - err_kind - ), - }, - )) + for &(name, kind) in WINDOWS_IO_ERROR_TABLE { + if err_kind == kind { + return Ok(this.eval_windows("c", name)); + } + } + throw_unsup_format!("io error {:?} cannot be translated into a raw os error", err_kind); } else { throw_unsup_format!( "converting io::Error into errnum is unsupported for OS {}", @@ -749,8 +752,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { return Ok(Some(kind)); } } - // Our table is as complete as the mapping in std, so we are okay with saying "that's a - // strange one" here. + return Ok(None); + } else if target.families.iter().any(|f| f == "windows") { + let errnum = errnum.to_u32()?; + for &(name, kind) in WINDOWS_IO_ERROR_TABLE { + if errnum == this.eval_windows("c", name).to_u32()? { + return Ok(Some(kind)); + } + } return Ok(None); } else { throw_unsup_format!( diff --git a/src/shims/os_str.rs b/src/shims/os_str.rs index 62ce2ee58a..74e8d1d9ed 100644 --- a/src/shims/os_str.rs +++ b/src/shims/os_str.rs @@ -98,6 +98,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// /// If `truncate == true`, then in case `size` is not large enough it *will* write the first /// `size.saturating_sub(1)` many items, followed by a null terminator (if `size > 0`). + /// The return value is still `(false, length)` in that case. fn write_os_str_to_wide_str( &mut self, os_str: &OsStr, diff --git a/src/shims/windows/foreign_items.rs b/src/shims/windows/foreign_items.rs index 9c6994cec7..2e514b11cb 100644 --- a/src/shims/windows/foreign_items.rs +++ b/src/shims/windows/foreign_items.rs @@ -1,3 +1,4 @@ +use std::ffi::OsStr; use std::iter; use std::str; @@ -533,6 +534,43 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.set_last_error(insufficient_buffer)?; } } + "FormatMessageW" => { + let [flags, module, message_id, language_id, buffer, size, arguments] = + this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; + + let flags = this.read_scalar(flags)?.to_u32()?; + let _module = this.read_pointer(module)?; // seems to contain a module name + let message_id = this.read_scalar(message_id)?; + let _language_id = this.read_scalar(language_id)?.to_u32()?; + let buffer = this.read_pointer(buffer)?; + let size = this.read_scalar(size)?.to_u32()?; + let _arguments = this.read_pointer(arguments)?; + + // We only support `FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS` + // This also means `arguments` can be ignored. + if flags != 4096u32 | 512u32 { + throw_unsup_format!("FormatMessageW: unsupported flags {flags:#x}"); + } + + let error = this.try_errnum_to_io_error(message_id)?; + let formatted = match error { + Some(err) => format!("{err}"), + None => format!(""), + }; + let (complete, length) = this.write_os_str_to_wide_str( + OsStr::new(&formatted), + buffer, + size.into(), + /*trunacte*/ false, + )?; + if !complete { + // The API docs don't say what happens when the buffer is not big enough... + // Let's just bail. + throw_unsup_format!("FormatMessageW: buffer not big enough"); + } + // The return value is the number of characters stored *excluding* the null terminator. + this.write_int(length.checked_sub(1).unwrap(), dest)?; + } // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. diff --git a/tests/pass/shims/fs.rs b/tests/pass/shims/fs.rs index d10faebac7..8a500b857b 100644 --- a/tests/pass/shims/fs.rs +++ b/tests/pass/shims/fs.rs @@ -260,7 +260,7 @@ fn test_errors() { // Opening a non-existing file should fail with a "not found" error. assert_eq!(ErrorKind::NotFound, File::open(&path).unwrap_err().kind()); // Make sure we can also format this. - format!("{0:?}: {0}", File::open(&path).unwrap_err()); + format!("{0}: {0:?}", File::open(&path).unwrap_err()); // Removing a non-existing file should fail with a "not found" error. assert_eq!(ErrorKind::NotFound, remove_file(&path).unwrap_err().kind()); // Reading the metadata of a non-existing file should fail with a "not found" error. diff --git a/tests/pass/shims/io.rs b/tests/pass/shims/io.rs index 295723957a..d20fc75b79 100644 --- a/tests/pass/shims/io.rs +++ b/tests/pass/shims/io.rs @@ -1,7 +1,19 @@ -use std::io::IsTerminal; +use std::io::{self, IsTerminal}; fn main() { // We can't really assume that this is truly a terminal, and anyway on Windows Miri will always // return `false` here, but we can check that the call succeeds. - std::io::stdout().is_terminal(); + io::stdout().is_terminal(); + + // Ensure we can format `io::Error` created from OS errors + // (calls OS-specific error formatting functions). + let raw_os_error = if cfg!(unix) { + 22 // EINVAL (on most Unixes, anyway) + } else if cfg!(windows) { + 87 // ERROR_INVALID_PARAMETER + } else { + panic!("unsupported OS") + }; + let err = io::Error::from_raw_os_error(raw_os_error); + format!("{err}: {err:?}"); } From 204326dd9fa1410b055c75f27280e1f56413dd53 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 15 Apr 2024 09:53:32 +0200 Subject: [PATCH 4/9] fix error display for './miri run --dep' --- tests/ui.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ui.rs b/tests/ui.rs index 7e8d140118..68446f40e0 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -296,12 +296,13 @@ fn main() -> Result<()> { fn run_dep_mode(target: String, mut args: impl Iterator) -> Result<()> { let path = args.next().expect("./miri run-dep must be followed by a file name"); - let config = miri_config( + let mut config = miri_config( &target, "", Mode::Yolo { rustfix: RustfixMode::Disabled }, /* with dependencies */ true, ); + config.program.args.clear(); // remove the `--error-format` that ui_test adds by default let dep_args = config.build_dependencies()?; let mut cmd = config.program.build(&config.out_dir); From f592764197a5e9858b8c10a0a82bad07d3c0e8ef Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 15 Apr 2024 14:32:22 +0200 Subject: [PATCH 5/9] add some basic support for GetFullPathNameW --- src/helpers.rs | 15 +++++ src/lib.rs | 1 + src/shims/env.rs | 25 +++----- src/shims/os_str.rs | 24 +++++++- src/shims/windows/foreign_items.rs | 93 +++++++++++++++++++++++++++++- tests/pass/shims/path.rs | 38 ++++++++++++ 6 files changed, 174 insertions(+), 22 deletions(-) create mode 100644 tests/pass/shims/path.rs diff --git a/src/helpers.rs b/src/helpers.rs index 84eb5f832d..998de80a7e 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -1285,3 +1285,18 @@ pub(crate) fn simd_element_to_bool(elem: ImmTy<'_, Provenance>) -> InterpResult< _ => throw_ub_format!("each element of a SIMD mask must be all-0-bits or all-1-bits"), }) } + +/// Check whether an operation that writes to a target buffer was successful. +/// Accordingly select return value. +/// Local helper function to be used in Windows shims. +pub(crate) fn windows_check_buffer_size((success, len): (bool, u64)) -> u32 { + if success { + // If the function succeeds, the return value is the number of characters stored in the target buffer, + // not including the terminating null character. + u32::try_from(len.checked_sub(1).unwrap()).unwrap() + } else { + // If the target buffer was not large enough to hold the data, the return value is the buffer size, in characters, + // required to hold the string and its terminating null character. + u32::try_from(len).unwrap() + } +} diff --git a/src/lib.rs b/src/lib.rs index 7821aa9efd..484908086a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,7 @@ #![feature(let_chains)] #![feature(lint_reasons)] #![feature(trait_upcasting)] +#![feature(absolute_path)] // Configure clippy and other lints #![allow( clippy::collapsible_else_if, diff --git a/src/shims/env.rs b/src/shims/env.rs index 9e8239cdba..1779189c9c 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -9,21 +9,7 @@ use rustc_middle::ty::Ty; use rustc_target::abi::Size; use crate::*; - -/// Check whether an operation that writes to a target buffer was successful. -/// Accordingly select return value. -/// Local helper function to be used in Windows shims. -fn windows_check_buffer_size((success, len): (bool, u64)) -> u32 { - if success { - // If the function succeeds, the return value is the number of characters stored in the target buffer, - // not including the terminating null character. - u32::try_from(len.checked_sub(1).unwrap()).unwrap() - } else { - // If the target buffer was not large enough to hold the data, the return value is the buffer size, in characters, - // required to hold the string and its terminating null character. - u32::try_from(len).unwrap() - } -} +use helpers::windows_check_buffer_size; #[derive(Default)] pub struct EnvVars<'tcx> { @@ -164,7 +150,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let name_ptr = this.read_pointer(name_op)?; let name = this.read_os_str_from_wide_str(name_ptr)?; Ok(match this.machine.env_vars.map.get(&name) { - Some(var_ptr) => { + Some(&var_ptr) => { + this.set_last_error(Scalar::from_u32(0))?; // make sure this is unambiguously not an error // The offset is used to strip the "{name}=" part of the string. #[rustfmt::skip] let name_offset_bytes = u64::try_from(name.len()).unwrap() @@ -375,10 +362,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // If we cannot get the current directory, we return 0 match env::current_dir() { - Ok(cwd) => + Ok(cwd) => { + this.set_last_error(Scalar::from_u32(0))?; // make sure this is unambiguously not an error return Ok(Scalar::from_u32(windows_check_buffer_size( this.write_path_to_wide_str(&cwd, buf, size, /*truncate*/ false)?, - ))), + ))); + } Err(e) => this.set_last_error_from_io_error(e.kind())?, } Ok(Scalar::from_u32(0)) diff --git a/src/shims/os_str.rs b/src/shims/os_str.rs index 74e8d1d9ed..0157c4845c 100644 --- a/src/shims/os_str.rs +++ b/src/shims/os_str.rs @@ -316,9 +316,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // We also have to ensure that absolute paths remain absolute. match direction { PathConversion::HostToTarget => { - // If this start withs a `\`, we add `\\?` so it starts with `\\?\` which is - // some magic path on Windows that *is* considered absolute. - if converted.get(0).copied() == Some(b'\\') { + // If the path is `/C:/`, the leading backslash was probably added by the below + // driver letter handling and we should get rid of it again. + if converted.get(0).copied() == Some(b'\\') + && converted.get(2).copied() == Some(b':') + && converted.get(3).copied() == Some(b'\\') + { + converted.remove(0); + } + // If this start withs a `\` but not a `\\`, then for Windows this is a relative + // path. But the host path is absolute as it started with `/`. We add `\\?` so + // it starts with `\\?\` which is some magic path on Windows that *is* + // considered absolute. + else if converted.get(0).copied() == Some(b'\\') + && converted.get(1).copied() != Some(b'\\') + { converted.splice(0..0, b"\\\\?".iter().copied()); } } @@ -333,6 +345,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Remove first 3 characters converted.splice(0..3, std::iter::empty()); } + // If it starts with a drive letter, convert it to an absolute Unix path. + else if converted.get(1).copied() == Some(b':') + && converted.get(2).copied() == Some(b'/') + { + converted.insert(0, b'/'); + } } } Cow::Owned(OsString::from_vec(converted)) diff --git a/src/shims/windows/foreign_items.rs b/src/shims/windows/foreign_items.rs index 2e514b11cb..de80df3c80 100644 --- a/src/shims/windows/foreign_items.rs +++ b/src/shims/windows/foreign_items.rs @@ -1,5 +1,7 @@ use std::ffi::OsStr; +use std::io; use std::iter; +use std::path::{self, Path, PathBuf}; use std::str; use rustc_span::Symbol; @@ -21,6 +23,61 @@ fn is_dyn_sym(name: &str) -> bool { ) } +#[cfg(windows)] +fn win_absolute<'tcx>(path: &Path) -> InterpResult<'tcx, io::Result> { + // We are on Windows so we can simply lte the host do this. + return Ok(path::absolute(path)); +} + +#[cfg(unix)] +#[allow(clippy::get_first, clippy::arithmetic_side_effects)] +fn win_absolute<'tcx>(path: &Path) -> InterpResult<'tcx, io::Result> { + // We are on Unix, so we need to implement parts of the logic ourselves. + let bytes = path.as_os_str().as_encoded_bytes(); + // If it starts with `//` (these were backslashes but are already converted) + // then this is a magic special path, we just leave it unchanged. + if bytes.get(0).copied() == Some(b'/') && bytes.get(1).copied() == Some(b'/') { + return Ok(Ok(path.into())); + }; + // Special treatment for Windows' magic filenames: they are treated as being relative to `\\.\`. + let magic_filenames = &[ + "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", + "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", + ]; + if magic_filenames.iter().any(|m| m.as_bytes() == bytes) { + let mut result: Vec = br"//./".into(); + result.extend(bytes); + return Ok(Ok(bytes_to_os_str(&result)?.into())); + } + // Otherwise we try to do something kind of close to what Windows does, but this is probably not + // right in all cases. We iterate over the components between `/`, and remove trailing `.`, + // except that trailing `..` remain unchanged. + let mut result = vec![]; + let mut bytes = bytes; // the remaining bytes to process + loop { + let len = bytes.iter().position(|&b| b == b'/').unwrap_or(bytes.len()); + let mut component = &bytes[..len]; + if len >= 2 && component[len - 1] == b'.' && component[len - 2] != b'.' { + // Strip trailing `.` + component = &component[..len - 1]; + } + // Add this component to output. + result.extend(component); + // Prepare next iteration. + if len < bytes.len() { + // There's a component after this; add `/` and process remaining bytes. + result.push(b'/'); + bytes = &bytes[len + 1..]; + continue; + } else { + // This was the last component and it did not have a trailing `/`. + break; + } + } + // Let the host `absolute` function do working-dir handling + Ok(path::absolute(bytes_to_os_str(&result)?)) +} + impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn emulate_foreign_item_inner( @@ -112,7 +169,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let written = if handle == -11 || handle == -12 { // stdout/stderr - use std::io::{self, Write}; + use io::Write; let buf_cont = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(u64::from(n)))?; @@ -146,6 +203,40 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { dest, )?; } + "GetFullPathNameW" => { + let [filename, size, buffer, filepart] = + this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; + this.check_no_isolation("`GetFullPathNameW`")?; + + let filename = this.read_pointer(filename)?; + let size = this.read_scalar(size)?.to_u32()?; + let buffer = this.read_pointer(buffer)?; + let filepart = this.read_pointer(filepart)?; + + if !this.ptr_is_null(filepart)? { + throw_unsup_format!("GetFullPathNameW: non-null `lpFilePart` is not supported"); + } + + let filename = this.read_path_from_wide_str(filename)?; + let result = match win_absolute(&filename)? { + Err(err) => { + this.set_last_error_from_io_error(err.kind())?; + Scalar::from_u32(0) // return zero upon failure + } + Ok(abs_filename) => { + this.set_last_error(Scalar::from_u32(0))?; // make sure this is unambiguously not an error + Scalar::from_u32(helpers::windows_check_buffer_size( + this.write_path_to_wide_str( + &abs_filename, + buffer, + size.into(), + /*truncate*/ false, + )?, + )) + } + }; + this.write_scalar(result, dest)?; + } // Allocation "HeapAlloc" => { diff --git a/tests/pass/shims/path.rs b/tests/pass/shims/path.rs new file mode 100644 index 0000000000..9fc6e7faef --- /dev/null +++ b/tests/pass/shims/path.rs @@ -0,0 +1,38 @@ +//@compile-flags: -Zmiri-disable-isolation +#![feature(absolute_path)] +use std::path::{absolute, Path}; + +#[track_caller] +fn test_absolute(in_: &str, out: &str) { + assert_eq!(absolute(in_).unwrap().as_os_str(), Path::new(out).as_os_str()); +} + +fn main() { + if cfg!(unix) { + test_absolute("/a/b/c", "/a/b/c"); + test_absolute("/a/b/c", "/a/b/c"); + test_absolute("/a//b/c", "/a/b/c"); + test_absolute("//a/b/c", "//a/b/c"); + test_absolute("///a/b/c", "/a/b/c"); + test_absolute("/a/b/c/", "/a/b/c/"); + test_absolute("/a/./b/../c/.././..", "/a/b/../c/../.."); + } else if cfg!(windows) { + // Test that all these are unchanged + test_absolute(r"C:\path\to\file", r"C:\path\to\file"); + test_absolute(r"C:\path\to\file\", r"C:\path\to\file\"); + test_absolute(r"\\server\share\to\file", r"\\server\share\to\file"); + test_absolute(r"\\server.\share.\to\file", r"\\server.\share.\to\file"); + test_absolute(r"\\.\PIPE\name", r"\\.\PIPE\name"); + test_absolute(r"\\.\C:\path\to\COM1", r"\\.\C:\path\to\COM1"); + test_absolute(r"\\?\C:\path\to\file", r"\\?\C:\path\to\file"); + test_absolute(r"\\?\UNC\server\share\to\file", r"\\?\UNC\server\share\to\file"); + test_absolute(r"\\?\PIPE\name", r"\\?\PIPE\name"); + // Verbatim paths are always unchanged, no matter what. + test_absolute(r"\\?\path.\to/file..", r"\\?\path.\to/file.."); + + test_absolute(r"C:\path..\to.\file.", r"C:\path..\to\file"); + test_absolute(r"COM1", r"\\.\COM1"); + } else { + panic!("unsupported OS"); + } +} From 2d90a21dbf4b4ab69110d7f9ae4a8b8eb6d9648e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Sun, 14 Apr 2024 19:30:20 +0200 Subject: [PATCH 6/9] Make `split_simd_to_128bit_chunks` take only one operand It will allow more flexible uses in the future. This makes `split_simd_to_128bit_chunks` simpler, moving some of the complexity to its callers. --- src/shims/x86/mod.rs | 77 +++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 44 deletions(-) diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 2a663d300a..615821b2e3 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -664,54 +664,35 @@ fn convert_float_to_int<'tcx>( Ok(()) } -/// Splits `left`, `right` and `dest` (which must be SIMD vectors) -/// into 128-bit chuncks. -/// -/// `left`, `right` and `dest` cannot have different types. +/// Splits `op` (which must be a SIMD vector) into 128-bit chuncks. /// /// Returns a tuple where: /// * The first element is the number of 128-bit chunks (let's call it `N`). /// * The second element is the number of elements per chunk (let's call it `M`). -/// * The third element is the `left` vector split into chunks, i.e, it's -/// type is `[[T; M]; N]`. -/// * The fourth element is the `right` vector split into chunks. -/// * The fifth element is the `dest` vector split into chunks. -fn split_simd_to_128bit_chunks<'tcx>( +/// * The third element is the `op` vector split into chunks, i.e, it's +/// type is `[[T; M]; N]` where `T` is the element type of `op`. +fn split_simd_to_128bit_chunks<'tcx, P: Projectable<'tcx, Provenance>>( this: &mut crate::MiriInterpCx<'_, 'tcx>, - left: &OpTy<'tcx, Provenance>, - right: &OpTy<'tcx, Provenance>, - dest: &MPlaceTy<'tcx, Provenance>, -) -> InterpResult< - 'tcx, - (u64, u64, MPlaceTy<'tcx, Provenance>, MPlaceTy<'tcx, Provenance>, MPlaceTy<'tcx, Provenance>), -> { - assert_eq!(dest.layout, left.layout); - assert_eq!(dest.layout, right.layout); + op: &P, +) -> InterpResult<'tcx, (u64, u64, P)> { + let simd_layout = op.layout(); + let (simd_len, element_ty) = simd_layout.ty.simd_size_and_type(this.tcx.tcx); - let (left, left_len) = this.operand_to_simd(left)?; - let (right, right_len) = this.operand_to_simd(right)?; - let (dest, dest_len) = this.mplace_to_simd(dest)?; - - assert_eq!(dest_len, left_len); - assert_eq!(dest_len, right_len); - - assert_eq!(dest.layout.size.bits() % 128, 0); - let num_chunks = dest.layout.size.bits() / 128; - assert_eq!(dest_len.checked_rem(num_chunks), Some(0)); - let items_per_chunk = dest_len.checked_div(num_chunks).unwrap(); + assert_eq!(simd_layout.size.bits() % 128, 0); + let num_chunks = simd_layout.size.bits() / 128; + let items_per_chunk = simd_len.checked_div(num_chunks).unwrap(); // Transmute to `[[T; items_per_chunk]; num_chunks]` - let element_layout = left.layout.field(this, 0); - let chunked_layout = this.layout_of(Ty::new_array( - this.tcx.tcx, - Ty::new_array(this.tcx.tcx, element_layout.ty, items_per_chunk), - num_chunks, - ))?; - let left = left.transmute(chunked_layout, this)?; - let right = right.transmute(chunked_layout, this)?; - let dest = dest.transmute(chunked_layout, this)?; - - Ok((num_chunks, items_per_chunk, left, right, dest)) + let chunked_layout = this + .layout_of(Ty::new_array( + this.tcx.tcx, + Ty::new_array(this.tcx.tcx, element_ty, items_per_chunk), + num_chunks, + )) + .unwrap(); + let chunked_op = op.transmute(chunked_layout, this)?; + + Ok((num_chunks, items_per_chunk, chunked_op)) } /// Horizontaly performs `which` operation on adjacent values of @@ -731,8 +712,12 @@ fn horizontal_bin_op<'tcx>( right: &OpTy<'tcx, Provenance>, dest: &MPlaceTy<'tcx, Provenance>, ) -> InterpResult<'tcx, ()> { - let (num_chunks, items_per_chunk, left, right, dest) = - split_simd_to_128bit_chunks(this, left, right, dest)?; + assert_eq!(left.layout, dest.layout); + assert_eq!(right.layout, dest.layout); + + let (num_chunks, items_per_chunk, left) = split_simd_to_128bit_chunks(this, left)?; + let (_, _, right) = split_simd_to_128bit_chunks(this, right)?; + let (_, _, dest) = split_simd_to_128bit_chunks(this, dest)?; let middle = items_per_chunk / 2; for i in 0..num_chunks { @@ -779,8 +764,12 @@ fn conditional_dot_product<'tcx>( imm: &OpTy<'tcx, Provenance>, dest: &MPlaceTy<'tcx, Provenance>, ) -> InterpResult<'tcx, ()> { - let (num_chunks, items_per_chunk, left, right, dest) = - split_simd_to_128bit_chunks(this, left, right, dest)?; + assert_eq!(left.layout, dest.layout); + assert_eq!(right.layout, dest.layout); + + let (num_chunks, items_per_chunk, left) = split_simd_to_128bit_chunks(this, left)?; + let (_, _, right) = split_simd_to_128bit_chunks(this, right)?; + let (_, _, dest) = split_simd_to_128bit_chunks(this, dest)?; let element_layout = left.layout.field(this, 0).field(this, 0); assert!(items_per_chunk <= 4); From 69a6c8c2d4bc428633239151cdd03702a256851e Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Mon, 15 Apr 2024 11:51:56 -0400 Subject: [PATCH 7/9] Bump rustc-build-sysroot to 0.4.6 --- cargo-miri/Cargo.lock | 4 ++-- cargo-miri/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cargo-miri/Cargo.lock b/cargo-miri/Cargo.lock index 6841a345ce..a1dbc85605 100644 --- a/cargo-miri/Cargo.lock +++ b/cargo-miri/Cargo.lock @@ -194,9 +194,9 @@ dependencies = [ [[package]] name = "rustc-build-sysroot" -version = "0.4.5" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a26170e1d79ea32f7ccec3188dd13cfc1f18c82764a9cbc1071667c0f865a4ea" +checksum = "a9bf37423495cd3a6a9ef8c75fc4566de0d26de0ab75f36f67a426e58df5768c" dependencies = [ "anyhow", "rustc_version", diff --git a/cargo-miri/Cargo.toml b/cargo-miri/Cargo.toml index 55f6b5ac7e..b16068b6d1 100644 --- a/cargo-miri/Cargo.toml +++ b/cargo-miri/Cargo.toml @@ -18,7 +18,7 @@ directories = "5" rustc_version = "0.4" serde_json = "1.0.40" cargo_metadata = "0.18.0" -rustc-build-sysroot = "0.4.1" +rustc-build-sysroot = "0.4.6" # Enable some feature flags that dev-dependencies need but dependencies # do not. This makes `./miri install` after `./miri build` faster. From d66d33b90a27c0fc53fa0c0a9a4f0f52eed5bf57 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 16 Apr 2024 08:52:06 +0200 Subject: [PATCH 8/9] avoid passing --sysroot twice in bootstrap --- cargo-miri/src/phases.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cargo-miri/src/phases.rs b/cargo-miri/src/phases.rs index ca8b35a17b..b774ca8fa7 100644 --- a/cargo-miri/src/phases.rs +++ b/cargo-miri/src/phases.rs @@ -412,8 +412,11 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { // Arguments are treated very differently depending on whether this crate is // for interpretation by Miri, or for use by a build script / proc macro. if target_crate { - // Set the sysroot. - cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); + if phase != RustcPhase::Setup { + // Set the sysroot -- except during setup, where we don't have an existing sysroot yet + // and where the bootstrap wrapper adds its own `--sysroot` flag so we can't set ours. + cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); + } // Forward arguments, but patched. let emit_flag = "--emit"; @@ -578,9 +581,9 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner } if phase != RunnerPhase::Rustdoc { - // Set the sysroot. Not necessary in rustdoc, where we already set the sysroot when invoking - // rustdoc itself, which will forward that flag when invoking rustc (i.e., us), so the flag - // is present in `info.args`. + // Set the sysroot. Not necessary in rustdoc, where we already set the sysroot in + // `phase_rustdoc`. rustdoc will forward that flag when invoking rustc (i.e., us), so the + // flag is present in `info.args`. cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); } // Forward rustc arguments. From 2962277b2077200483de0047d087c3855d8330f5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 16 Apr 2024 16:37:34 +0200 Subject: [PATCH 9/9] interpret: pass MemoryKind to before_memory_deallocation --- src/borrow_tracker/mod.rs | 2 +- src/borrow_tracker/stacked_borrows/mod.rs | 2 +- src/borrow_tracker/tree_borrows/mod.rs | 2 +- src/concurrency/data_race.rs | 2 +- src/diagnostics.rs | 4 ++-- src/lib.rs | 2 +- src/machine.rs | 15 ++++++++------- src/shims/os_str.rs | 8 ++++---- 8 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/borrow_tracker/mod.rs b/src/borrow_tracker/mod.rs index 8d76a48826..f21315790a 100644 --- a/src/borrow_tracker/mod.rs +++ b/src/borrow_tracker/mod.rs @@ -260,7 +260,7 @@ impl GlobalStateInner { &mut self, id: AllocId, alloc_size: Size, - kind: MemoryKind, + kind: MemoryKind, machine: &MiriMachine<'_, '_>, ) -> AllocState { match self.borrow_tracker_method { diff --git a/src/borrow_tracker/stacked_borrows/mod.rs b/src/borrow_tracker/stacked_borrows/mod.rs index 96ff298402..b4005515d9 100644 --- a/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/borrow_tracker/stacked_borrows/mod.rs @@ -509,7 +509,7 @@ impl Stacks { id: AllocId, size: Size, state: &mut GlobalStateInner, - kind: MemoryKind, + kind: MemoryKind, machine: &MiriMachine<'_, '_>, ) -> Self { let (base_tag, perm) = match kind { diff --git a/src/borrow_tracker/tree_borrows/mod.rs b/src/borrow_tracker/tree_borrows/mod.rs index a3d49756e4..492e324de4 100644 --- a/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/borrow_tracker/tree_borrows/mod.rs @@ -34,7 +34,7 @@ impl<'tcx> Tree { id: AllocId, size: Size, state: &mut GlobalStateInner, - _kind: MemoryKind, + _kind: MemoryKind, machine: &MiriMachine<'_, 'tcx>, ) -> Self { let tag = state.base_ptr_tag(id, machine); // Fresh tag for the root diff --git a/src/concurrency/data_race.rs b/src/concurrency/data_race.rs index d51160b283..95049b91cb 100644 --- a/src/concurrency/data_race.rs +++ b/src/concurrency/data_race.rs @@ -844,7 +844,7 @@ impl VClockAlloc { global: &GlobalState, thread_mgr: &ThreadManager<'_, '_>, len: Size, - kind: MemoryKind, + kind: MemoryKind, current_span: Span, ) -> VClockAlloc { let (alloc_timestamp, alloc_index) = match kind { diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 30349c003a..a2b817ea0d 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -115,7 +115,7 @@ pub enum NonHaltingDiagnostic { /// This `Item` was popped from the borrow stack. The string explains the reason. PoppedPointerTag(Item, String), CreatedCallId(CallId), - CreatedAlloc(AllocId, Size, Align, MemoryKind), + CreatedAlloc(AllocId, Size, Align, MemoryKind), FreedAlloc(AllocId), AccessedAlloc(AllocId, AccessKind), RejectedIsolatedOp(String), @@ -414,7 +414,7 @@ pub fn report_error<'tcx, 'mir>( pub fn report_leaks<'mir, 'tcx>( ecx: &InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>, - leaks: Vec<(AllocId, MemoryKind, Allocation>)>, + leaks: Vec<(AllocId, MemoryKind, Allocation>)>, ) { let mut any_pruned = false; for (id, kind, mut alloc) in leaks { diff --git a/src/lib.rs b/src/lib.rs index 7821aa9efd..42e66057b9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -125,7 +125,7 @@ pub use crate::eval::{ }; pub use crate::helpers::{AccessKind, EvalContextExt as _}; pub use crate::machine::{ - AllocExtra, FrameExtra, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind, + AllocExtra, FrameExtra, MemoryKind, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind, PrimitiveLayouts, Provenance, ProvenanceExtra, }; pub use crate::mono_hash_map::MonoHashMap; diff --git a/src/machine.rs b/src/machine.rs index ff081328a7..1d06d5c69d 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -135,9 +135,9 @@ pub enum MiriMemoryKind { Mmap, } -impl From for MemoryKind { +impl From for MemoryKind { #[inline(always)] - fn from(kind: MiriMemoryKind) -> MemoryKind { + fn from(kind: MiriMemoryKind) -> MemoryKind { MemoryKind::Machine(kind) } } @@ -185,6 +185,8 @@ impl fmt::Display for MiriMemoryKind { } } +pub type MemoryKind = interpret::MemoryKind; + /// Pointer provenance. #[derive(Clone, Copy)] pub enum Provenance { @@ -863,10 +865,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { type ProvenanceExtra = ProvenanceExtra; type Bytes = Box<[u8]>; - type MemoryMap = MonoHashMap< - AllocId, - (MemoryKind, Allocation), - >; + type MemoryMap = + MonoHashMap)>; const GLOBAL_KIND: Option = Some(MiriMemoryKind::Global); @@ -1088,7 +1088,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { ecx: &MiriInterpCx<'mir, 'tcx>, id: AllocId, alloc: Cow<'b, Allocation>, - kind: Option>, + kind: Option, ) -> InterpResult<'tcx, Cow<'b, Allocation>> { let kind = kind.expect("we set our STATIC_KIND so this cannot be None"); if ecx.machine.tracked_alloc_ids.contains(&id) { @@ -1280,6 +1280,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { (alloc_id, prove_extra): (AllocId, Self::ProvenanceExtra), size: Size, align: Align, + _kind: MemoryKind, ) -> InterpResult<'tcx> { if machine.tracked_alloc_ids.contains(&alloc_id) { machine.emit_diagnostic(NonHaltingDiagnostic::FreedAlloc(alloc_id)); diff --git a/src/shims/os_str.rs b/src/shims/os_str.rs index 62ce2ee58a..a27c9b746d 100644 --- a/src/shims/os_str.rs +++ b/src/shims/os_str.rs @@ -136,7 +136,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn alloc_os_str_as_c_str( &mut self, os_str: &OsStr, - memkind: MemoryKind, + memkind: MemoryKind, ) -> InterpResult<'tcx, Pointer>> { let size = u64::try_from(os_str.len()).unwrap().checked_add(1).unwrap(); // Make space for `0` terminator. let this = self.eval_context_mut(); @@ -152,7 +152,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn alloc_os_str_as_wide_str( &mut self, os_str: &OsStr, - memkind: MemoryKind, + memkind: MemoryKind, ) -> InterpResult<'tcx, Pointer>> { let size = u64::try_from(os_str.len()).unwrap().checked_add(1).unwrap(); // Make space for `0x0000` terminator. let this = self.eval_context_mut(); @@ -229,7 +229,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn alloc_path_as_c_str( &mut self, path: &Path, - memkind: MemoryKind, + memkind: MemoryKind, ) -> InterpResult<'tcx, Pointer>> { let this = self.eval_context_mut(); let os_str = @@ -242,7 +242,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn alloc_path_as_wide_str( &mut self, path: &Path, - memkind: MemoryKind, + memkind: MemoryKind, ) -> InterpResult<'tcx, Pointer>> { let this = self.eval_context_mut(); let os_str =