From 8a62664190ee7e1adb0dd89997f8cb6a84ab0897 Mon Sep 17 00:00:00 2001 From: Jefffrey <22608443+Jefffrey@users.noreply.github.com> Date: Sun, 3 Dec 2023 23:05:33 +1100 Subject: [PATCH 01/20] Fix miri script target dir and update doc link --- CONTRIBUTING.md | 2 +- miri | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 40a4332cdb..7a49ff3372 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -109,7 +109,7 @@ to run the other checks while ignoring the ui output, use `MIRI_SKIP_UI_CHECKS=1 For more info on how to configure ui tests see [the documentation on the ui test crate][ui_test] -[ui_test]: ui_test/README.md +[ui_test]: https://github.com/oli-obk/ui_test/blob/main/README.md ### Testing `cargo miri` diff --git a/miri b/miri index e21738c361..169f4521f2 100755 --- a/miri +++ b/miri @@ -2,5 +2,6 @@ set -e # Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through # rustup (that sets it's own environmental variables), which is undesirable. -cargo build $CARGO_EXTRA_FLAGS -q --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml -"$(dirname "$0")"/miri-script/target/debug/miri-script "$@" +MIRI_SCRIPT_TARGET_DIR="$(dirname "$0")"/miri-script/target +cargo build $CARGO_EXTRA_FLAGS -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml +"$MIRI_SCRIPT_TARGET_DIR"/debug/miri-script "$@" From e49866a5c778b6ebe17324369604c3b448c88a4b Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 5 Dec 2023 12:05:38 +0100 Subject: [PATCH 02/20] Don't explicitly warn against `semicolon_in_expressions_from_macros` This warns-by-default since 2 years and already has been added to the future-incompat group since Rust 1.68. See https://github.com/rust-lang/rust/issues/79813 for the tracking issue. --- miri-script/src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miri-script/src/util.rs b/miri-script/src/util.rs index 64e780b61a..d41eaecf20 100644 --- a/miri-script/src/util.rs +++ b/miri-script/src/util.rs @@ -73,7 +73,7 @@ impl MiriEnv { flags.push("-C link-args=-Wl,-rpath,"); flags.push(libdir); // Enable rustc-specific lints (ignored without `-Zunstable-options`). - flags.push(" -Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes -Wsemicolon_in_expressions_from_macros"); + flags.push(" -Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes"); // Add user-defined flags. if let Some(value) = std::env::var_os("RUSTFLAGS") { flags.push(" "); From dc32d58b46269d37785a6aac9805b6005a13fefc Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 5 Dec 2023 12:18:25 +0100 Subject: [PATCH 03/20] Fix formatting --- miri-script/src/util.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/miri-script/src/util.rs b/miri-script/src/util.rs index d41eaecf20..c9bc55848d 100644 --- a/miri-script/src/util.rs +++ b/miri-script/src/util.rs @@ -73,7 +73,9 @@ impl MiriEnv { flags.push("-C link-args=-Wl,-rpath,"); flags.push(libdir); // Enable rustc-specific lints (ignored without `-Zunstable-options`). - flags.push(" -Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes"); + flags.push( + " -Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes", + ); // Add user-defined flags. if let Some(value) = std::env::var_os("RUSTFLAGS") { flags.push(" "); From ca5109b142b7b01362301fff191aa6979f3bf2db Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 5 Dec 2023 22:24:40 -0500 Subject: [PATCH 04/20] remove unnecesary `-Zunstable-options` AFAIK `-Zunstable-options` is for `cargo --config` CLI, which was stabilized in 1.63 https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#config-cli --- cargo-miri/src/util.rs | 3 --- test-cargo-miri/run-test.py | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/cargo-miri/src/util.rs b/cargo-miri/src/util.rs index 6381a4db86..3c59126845 100644 --- a/cargo-miri/src/util.rs +++ b/cargo-miri/src/util.rs @@ -200,9 +200,6 @@ pub fn ask_to_run(mut cmd: Command, ask: bool, text: &str) { // cargo invocation. fn cargo_extra_flags() -> Vec { let mut flags = Vec::new(); - // `-Zunstable-options` is required by `--config`. - flags.push("-Zunstable-options".to_string()); - // Forward `--config` flags. let config_flag = "--config"; for arg in get_arg_flag_values(config_flag) { diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index db4341169e..21479bc4d5 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -181,7 +181,7 @@ def test_cargo_miri_test(): ) del os.environ["CARGO_TARGET_DIR"] # this overrides `build.target-dir` passed by `--config`, so unset it test("`cargo miri test` (config-cli)", - cargo_miri("test") + ["--config=build.target-dir=\"config-cli\"", "-Zunstable-options"], + cargo_miri("test") + ["--config=build.target-dir=\"config-cli\""], default_ref, "test.stderr-empty.ref", env={'MIRIFLAGS': "-Zmiri-permissive-provenance"}, ) From 38d5aed0bb3da75b550e7ff75d5f5ca70140c569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Thu, 7 Dec 2023 12:35:39 +0100 Subject: [PATCH 05/20] Move unary_op_* functions from `shims::x86::sse` module to `shims::x86` --- src/shims/x86/mod.rs | 124 +++++++++++++++++++++++++++++++++++++++++ src/shims/x86/sse.rs | 130 ++----------------------------------------- 2 files changed, 129 insertions(+), 125 deletions(-) diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 2ae269db41..3a95f452a3 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -1,3 +1,6 @@ +use rand::Rng as _; + +use rustc_apfloat::{ieee::Single, Float as _}; use rustc_middle::{mir, ty}; use rustc_span::Symbol; use rustc_target::abi::Size; @@ -331,6 +334,127 @@ fn bin_op_simd_float_all<'tcx, F: rustc_apfloat::Float>( Ok(()) } +#[derive(Copy, Clone)] +enum FloatUnaryOp { + /// sqrt(x) + /// + /// + /// + Sqrt, + /// Approximation of 1/x + /// + /// + /// + Rcp, + /// Approximation of 1/sqrt(x) + /// + /// + /// + Rsqrt, +} + +/// Performs `which` scalar operation on `op` and returns the result. +#[allow(clippy::arithmetic_side_effects)] // floating point operations without side effects +fn unary_op_f32<'tcx>( + this: &mut crate::MiriInterpCx<'_, 'tcx>, + which: FloatUnaryOp, + op: &ImmTy<'tcx, Provenance>, +) -> InterpResult<'tcx, Scalar> { + match which { + FloatUnaryOp::Sqrt => { + let op = op.to_scalar(); + // FIXME using host floats + Ok(Scalar::from_u32(f32::from_bits(op.to_u32()?).sqrt().to_bits())) + } + FloatUnaryOp::Rcp => { + let op = op.to_scalar().to_f32()?; + let div = (Single::from_u128(1).value / op).value; + // Apply a relative error with a magnitude on the order of 2^-12 to simulate the + // inaccuracy of RCP. + let res = apply_random_float_error(this, div, -12); + Ok(Scalar::from_f32(res)) + } + FloatUnaryOp::Rsqrt => { + let op = op.to_scalar().to_u32()?; + // FIXME using host floats + let sqrt = Single::from_bits(f32::from_bits(op).sqrt().to_bits().into()); + let rsqrt = (Single::from_u128(1).value / sqrt).value; + // Apply a relative error with a magnitude on the order of 2^-12 to simulate the + // inaccuracy of RSQRT. + let res = apply_random_float_error(this, rsqrt, -12); + Ok(Scalar::from_f32(res)) + } + } +} + +/// Disturbes a floating-point result by a relative error on the order of (-2^scale, 2^scale). +#[allow(clippy::arithmetic_side_effects)] // floating point arithmetic cannot panic +fn apply_random_float_error( + this: &mut crate::MiriInterpCx<'_, '_>, + val: F, + err_scale: i32, +) -> F { + let rng = this.machine.rng.get_mut(); + // generates rand(0, 2^64) * 2^(scale - 64) = rand(0, 1) * 2^scale + let err = + F::from_u128(rng.gen::().into()).value.scalbn(err_scale.checked_sub(64).unwrap()); + // give it a random sign + let err = if rng.gen::() { -err } else { err }; + // multiple the value with (1+err) + (val * (F::from_u128(1).value + err).value).value +} + +/// Performs `which` operation on the first component of `op` and copies +/// the other components. The result is stored in `dest`. +fn unary_op_ss<'tcx>( + this: &mut crate::MiriInterpCx<'_, 'tcx>, + which: FloatUnaryOp, + op: &OpTy<'tcx, Provenance>, + dest: &PlaceTy<'tcx, Provenance>, +) -> InterpResult<'tcx, ()> { + let (op, op_len) = this.operand_to_simd(op)?; + let (dest, dest_len) = this.place_to_simd(dest)?; + + assert_eq!(dest_len, op_len); + + let res0 = unary_op_f32(this, which, &this.read_immediate(&this.project_index(&op, 0)?)?)?; + this.write_scalar(res0, &this.project_index(&dest, 0)?)?; + + for i in 1..dest_len { + this.copy_op( + &this.project_index(&op, i)?, + &this.project_index(&dest, i)?, + /*allow_transmute*/ false, + )?; + } + + Ok(()) +} + +/// Performs `which` operation on each component of `op`, storing the +/// result is stored in `dest`. +fn unary_op_ps<'tcx>( + this: &mut crate::MiriInterpCx<'_, 'tcx>, + which: FloatUnaryOp, + op: &OpTy<'tcx, Provenance>, + dest: &PlaceTy<'tcx, Provenance>, +) -> InterpResult<'tcx, ()> { + let (op, op_len) = this.operand_to_simd(op)?; + let (dest, dest_len) = this.place_to_simd(dest)?; + + assert_eq!(dest_len, op_len); + + for i in 0..dest_len { + let op = this.read_immediate(&this.project_index(&op, i)?)?; + let dest = this.project_index(&dest, i)?; + + let res = unary_op_f32(this, which, &op)?; + this.write_scalar(res, &dest)?; + } + + Ok(()) +} + /// Converts each element of `op` from floating point to signed integer. /// /// When the input value is NaN or out of range, fall back to minimum value. diff --git a/src/shims/x86/sse.rs b/src/shims/x86/sse.rs index 6e06f62b34..1e9afc1e9e 100644 --- a/src/shims/x86/sse.rs +++ b/src/shims/x86/sse.rs @@ -1,11 +1,12 @@ -use rustc_apfloat::{ieee::Single, Float as _}; +use rustc_apfloat::ieee::Single; use rustc_middle::mir; use rustc_span::Symbol; use rustc_target::spec::abi::Abi; -use rand::Rng as _; - -use super::{bin_op_simd_float_all, bin_op_simd_float_first, FloatBinOp}; +use super::{ + bin_op_simd_float_all, bin_op_simd_float_first, unary_op_ps, unary_op_ss, FloatBinOp, + FloatUnaryOp, +}; use crate::*; use shims::foreign_items::EmulateForeignItemResult; @@ -219,124 +220,3 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: Ok(EmulateForeignItemResult::NeedsJumping) } } - -#[derive(Copy, Clone)] -enum FloatUnaryOp { - /// sqrt(x) - /// - /// - /// - Sqrt, - /// Approximation of 1/x - /// - /// - /// - Rcp, - /// Approximation of 1/sqrt(x) - /// - /// - /// - Rsqrt, -} - -/// Performs `which` scalar operation on `op` and returns the result. -#[allow(clippy::arithmetic_side_effects)] // floating point operations without side effects -fn unary_op_f32<'tcx>( - this: &mut crate::MiriInterpCx<'_, 'tcx>, - which: FloatUnaryOp, - op: &ImmTy<'tcx, Provenance>, -) -> InterpResult<'tcx, Scalar> { - match which { - FloatUnaryOp::Sqrt => { - let op = op.to_scalar(); - // FIXME using host floats - Ok(Scalar::from_u32(f32::from_bits(op.to_u32()?).sqrt().to_bits())) - } - FloatUnaryOp::Rcp => { - let op = op.to_scalar().to_f32()?; - let div = (Single::from_u128(1).value / op).value; - // Apply a relative error with a magnitude on the order of 2^-12 to simulate the - // inaccuracy of RCP. - let res = apply_random_float_error(this, div, -12); - Ok(Scalar::from_f32(res)) - } - FloatUnaryOp::Rsqrt => { - let op = op.to_scalar().to_u32()?; - // FIXME using host floats - let sqrt = Single::from_bits(f32::from_bits(op).sqrt().to_bits().into()); - let rsqrt = (Single::from_u128(1).value / sqrt).value; - // Apply a relative error with a magnitude on the order of 2^-12 to simulate the - // inaccuracy of RSQRT. - let res = apply_random_float_error(this, rsqrt, -12); - Ok(Scalar::from_f32(res)) - } - } -} - -/// Disturbes a floating-point result by a relative error on the order of (-2^scale, 2^scale). -#[allow(clippy::arithmetic_side_effects)] // floating point arithmetic cannot panic -fn apply_random_float_error( - this: &mut crate::MiriInterpCx<'_, '_>, - val: F, - err_scale: i32, -) -> F { - let rng = this.machine.rng.get_mut(); - // generates rand(0, 2^64) * 2^(scale - 64) = rand(0, 1) * 2^scale - let err = - F::from_u128(rng.gen::().into()).value.scalbn(err_scale.checked_sub(64).unwrap()); - // give it a random sign - let err = if rng.gen::() { -err } else { err }; - // multiple the value with (1+err) - (val * (F::from_u128(1).value + err).value).value -} - -/// Performs `which` operation on the first component of `op` and copies -/// the other components. The result is stored in `dest`. -fn unary_op_ss<'tcx>( - this: &mut crate::MiriInterpCx<'_, 'tcx>, - which: FloatUnaryOp, - op: &OpTy<'tcx, Provenance>, - dest: &PlaceTy<'tcx, Provenance>, -) -> InterpResult<'tcx, ()> { - let (op, op_len) = this.operand_to_simd(op)?; - let (dest, dest_len) = this.place_to_simd(dest)?; - - assert_eq!(dest_len, op_len); - - let res0 = unary_op_f32(this, which, &this.read_immediate(&this.project_index(&op, 0)?)?)?; - this.write_scalar(res0, &this.project_index(&dest, 0)?)?; - - for i in 1..dest_len { - this.copy_op( - &this.project_index(&op, i)?, - &this.project_index(&dest, i)?, - /*allow_transmute*/ false, - )?; - } - - Ok(()) -} - -/// Performs `which` operation on each component of `op`, storing the -/// result is stored in `dest`. -fn unary_op_ps<'tcx>( - this: &mut crate::MiriInterpCx<'_, 'tcx>, - which: FloatUnaryOp, - op: &OpTy<'tcx, Provenance>, - dest: &PlaceTy<'tcx, Provenance>, -) -> InterpResult<'tcx, ()> { - let (op, op_len) = this.operand_to_simd(op)?; - let (dest, dest_len) = this.place_to_simd(dest)?; - - assert_eq!(dest_len, op_len); - - for i in 0..dest_len { - let op = this.read_immediate(&this.project_index(&op, i)?)?; - let dest = this.project_index(&dest, i)?; - - let res = unary_op_f32(this, which, &op)?; - this.write_scalar(res, &dest)?; - } - - Ok(()) -} From 18f9bbd506c9111e3ca22b7ebbb337bc65952dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Thu, 7 Dec 2023 12:37:48 +0100 Subject: [PATCH 06/20] Move round_* functions from `shims::x86::sse41` module to `shims::x86` --- src/shims/x86/mod.rs | 83 +++++++++++++++++++++++++++++++++++++++++ src/shims/x86/sse41.rs | 85 +----------------------------------------- 2 files changed, 84 insertions(+), 84 deletions(-) diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 3a95f452a3..c4c361c3d6 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -455,6 +455,89 @@ fn unary_op_ps<'tcx>( Ok(()) } +// Rounds the first element of `right` according to `rounding` +// and copies the remaining elements from `left`. +fn round_first<'tcx, F: rustc_apfloat::Float>( + this: &mut crate::MiriInterpCx<'_, 'tcx>, + left: &OpTy<'tcx, Provenance>, + right: &OpTy<'tcx, Provenance>, + rounding: &OpTy<'tcx, Provenance>, + dest: &PlaceTy<'tcx, Provenance>, +) -> InterpResult<'tcx, ()> { + let (left, left_len) = this.operand_to_simd(left)?; + let (right, right_len) = this.operand_to_simd(right)?; + let (dest, dest_len) = this.place_to_simd(dest)?; + + assert_eq!(dest_len, left_len); + assert_eq!(dest_len, right_len); + + let rounding = rounding_from_imm(this.read_scalar(rounding)?.to_i32()?)?; + + let op0: F = this.read_scalar(&this.project_index(&right, 0)?)?.to_float()?; + let res = op0.round_to_integral(rounding).value; + this.write_scalar( + Scalar::from_uint(res.to_bits(), Size::from_bits(F::BITS)), + &this.project_index(&dest, 0)?, + )?; + + for i in 1..dest_len { + this.copy_op( + &this.project_index(&left, i)?, + &this.project_index(&dest, i)?, + /*allow_transmute*/ false, + )?; + } + + Ok(()) +} + +// Rounds all elements of `op` according to `rounding`. +fn round_all<'tcx, F: rustc_apfloat::Float>( + this: &mut crate::MiriInterpCx<'_, 'tcx>, + op: &OpTy<'tcx, Provenance>, + rounding: &OpTy<'tcx, Provenance>, + dest: &PlaceTy<'tcx, Provenance>, +) -> InterpResult<'tcx, ()> { + let (op, op_len) = this.operand_to_simd(op)?; + let (dest, dest_len) = this.place_to_simd(dest)?; + + assert_eq!(dest_len, op_len); + + let rounding = rounding_from_imm(this.read_scalar(rounding)?.to_i32()?)?; + + for i in 0..dest_len { + let op: F = this.read_scalar(&this.project_index(&op, i)?)?.to_float()?; + let res = op.round_to_integral(rounding).value; + this.write_scalar( + Scalar::from_uint(res.to_bits(), Size::from_bits(F::BITS)), + &this.project_index(&dest, i)?, + )?; + } + + Ok(()) +} + +/// Gets equivalent `rustc_apfloat::Round` from rounding mode immediate of +/// `round.{ss,sd,ps,pd}` intrinsics. +fn rounding_from_imm<'tcx>(rounding: i32) -> InterpResult<'tcx, rustc_apfloat::Round> { + // The fourth bit of `rounding` only affects the SSE status + // register, which cannot be accessed from Miri (or from Rust, + // for that matter), so we can ignore it. + match rounding & !0b1000 { + // When the third bit is 0, the rounding mode is determined by the + // first two bits. + 0b000 => Ok(rustc_apfloat::Round::NearestTiesToEven), + 0b001 => Ok(rustc_apfloat::Round::TowardNegative), + 0b010 => Ok(rustc_apfloat::Round::TowardPositive), + 0b011 => Ok(rustc_apfloat::Round::TowardZero), + // When the third bit is 1, the rounding mode is determined by the + // SSE status register. Since we do not support modifying it from + // Miri (or Rust), we assume it to be at its default mode (round-to-nearest). + 0b100..=0b111 => Ok(rustc_apfloat::Round::NearestTiesToEven), + rounding => throw_unsup_format!("unsupported rounding mode 0x{rounding:02x}"), + } +} + /// Converts each element of `op` from floating point to signed integer. /// /// When the input value is NaN or out of range, fall back to minimum value. diff --git a/src/shims/x86/sse41.rs b/src/shims/x86/sse41.rs index b3d1056ab0..1043791908 100644 --- a/src/shims/x86/sse41.rs +++ b/src/shims/x86/sse41.rs @@ -1,8 +1,8 @@ use rustc_middle::mir; use rustc_span::Symbol; -use rustc_target::abi::Size; use rustc_target::spec::abi::Abi; +use super::{round_all, round_first}; use crate::*; use shims::foreign_items::EmulateForeignItemResult; @@ -283,86 +283,3 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: Ok(EmulateForeignItemResult::NeedsJumping) } } - -// Rounds the first element of `right` according to `rounding` -// and copies the remaining elements from `left`. -fn round_first<'tcx, F: rustc_apfloat::Float>( - this: &mut crate::MiriInterpCx<'_, 'tcx>, - left: &OpTy<'tcx, Provenance>, - right: &OpTy<'tcx, Provenance>, - rounding: &OpTy<'tcx, Provenance>, - dest: &PlaceTy<'tcx, Provenance>, -) -> InterpResult<'tcx, ()> { - let (left, left_len) = this.operand_to_simd(left)?; - let (right, right_len) = this.operand_to_simd(right)?; - let (dest, dest_len) = this.place_to_simd(dest)?; - - assert_eq!(dest_len, left_len); - assert_eq!(dest_len, right_len); - - let rounding = rounding_from_imm(this.read_scalar(rounding)?.to_i32()?)?; - - let op0: F = this.read_scalar(&this.project_index(&right, 0)?)?.to_float()?; - let res = op0.round_to_integral(rounding).value; - this.write_scalar( - Scalar::from_uint(res.to_bits(), Size::from_bits(F::BITS)), - &this.project_index(&dest, 0)?, - )?; - - for i in 1..dest_len { - this.copy_op( - &this.project_index(&left, i)?, - &this.project_index(&dest, i)?, - /*allow_transmute*/ false, - )?; - } - - Ok(()) -} - -// Rounds all elements of `op` according to `rounding`. -fn round_all<'tcx, F: rustc_apfloat::Float>( - this: &mut crate::MiriInterpCx<'_, 'tcx>, - op: &OpTy<'tcx, Provenance>, - rounding: &OpTy<'tcx, Provenance>, - dest: &PlaceTy<'tcx, Provenance>, -) -> InterpResult<'tcx, ()> { - let (op, op_len) = this.operand_to_simd(op)?; - let (dest, dest_len) = this.place_to_simd(dest)?; - - assert_eq!(dest_len, op_len); - - let rounding = rounding_from_imm(this.read_scalar(rounding)?.to_i32()?)?; - - for i in 0..dest_len { - let op: F = this.read_scalar(&this.project_index(&op, i)?)?.to_float()?; - let res = op.round_to_integral(rounding).value; - this.write_scalar( - Scalar::from_uint(res.to_bits(), Size::from_bits(F::BITS)), - &this.project_index(&dest, i)?, - )?; - } - - Ok(()) -} - -/// Gets equivalent `rustc_apfloat::Round` from rounding mode immediate of -/// `round.{ss,sd,ps,pd}` intrinsics. -fn rounding_from_imm<'tcx>(rounding: i32) -> InterpResult<'tcx, rustc_apfloat::Round> { - // The fourth bit of `rounding` only affects the SSE status - // register, which cannot be accessed from Miri (or from Rust, - // for that matter), so we can ignore it. - match rounding & !0b1000 { - // When the third bit is 0, the rounding mode is determined by the - // first two bits. - 0b000 => Ok(rustc_apfloat::Round::NearestTiesToEven), - 0b001 => Ok(rustc_apfloat::Round::TowardNegative), - 0b010 => Ok(rustc_apfloat::Round::TowardPositive), - 0b011 => Ok(rustc_apfloat::Round::TowardZero), - // When the third bit is 1, the rounding mode is determined by the - // SSE status register. Since we do not support modifying it from - // Miri (or Rust), we assume it to be at its default mode (round-to-nearest). - 0b100..=0b111 => Ok(rustc_apfloat::Round::NearestTiesToEven), - rounding => throw_unsup_format!("unsupported rounding mode 0x{rounding:02x}"), - } -} From 85a4ff9d620fa9d5c713843a25e5e908eae88761 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Fri, 8 Dec 2023 04:55:40 +0000 Subject: [PATCH 07/20] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index c60249f35e..8841b0c95c 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -317d14a56cb8c748bf0e2f2afff89c2249ab4423 +d6fa38a9b2426487e010a6c16862132f89755e41 From b1fcba41df4d9a3c10d755c3d6547b4965e41a79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Thu, 7 Dec 2023 12:43:53 +0100 Subject: [PATCH 08/20] Move implementation of SSE4.1 `ptest*` into a helper function --- src/shims/x86/mod.rs | 28 ++++++++++++++++++++++++++++ src/shims/x86/sse41.rs | 34 +++++++++++++--------------------- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index c4c361c3d6..6d361f5d2a 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -615,3 +615,31 @@ fn horizontal_bin_op<'tcx>( Ok(()) } + +/// Folds SIMD vectors `lhs` and `rhs` into a value of type `T` using `f`. +fn bin_op_folded<'tcx, T>( + this: &crate::MiriInterpCx<'_, 'tcx>, + lhs: &OpTy<'tcx, Provenance>, + rhs: &OpTy<'tcx, Provenance>, + init: T, + mut f: impl FnMut(T, ImmTy<'tcx, Provenance>, ImmTy<'tcx, Provenance>) -> InterpResult<'tcx, T>, +) -> InterpResult<'tcx, T> { + assert_eq!(lhs.layout, rhs.layout); + + let (lhs, lhs_len) = this.operand_to_simd(lhs)?; + let (rhs, rhs_len) = this.operand_to_simd(rhs)?; + + assert_eq!(lhs_len, rhs_len); + + let mut acc = init; + for i in 0..lhs_len { + let lhs = this.project_index(&lhs, i)?; + let rhs = this.project_index(&rhs, i)?; + + let lhs = this.read_immediate(&lhs)?; + let rhs = this.read_immediate(&rhs)?; + acc = f(acc, lhs, rhs)?; + } + + Ok(acc) +} diff --git a/src/shims/x86/sse41.rs b/src/shims/x86/sse41.rs index 1043791908..2683105b00 100644 --- a/src/shims/x86/sse41.rs +++ b/src/shims/x86/sse41.rs @@ -2,7 +2,7 @@ use rustc_middle::mir; use rustc_span::Symbol; use rustc_target::spec::abi::Abi; -use super::{round_all, round_first}; +use super::{bin_op_folded, round_all, round_first}; use crate::*; use shims::foreign_items::EmulateForeignItemResult; @@ -257,26 +257,18 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: "ptestz" | "ptestc" | "ptestnzc" => { let [op, mask] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let (op, op_len) = this.operand_to_simd(op)?; - let (mask, mask_len) = this.operand_to_simd(mask)?; - - assert_eq!(op_len, mask_len); - - let f = match unprefixed_name { - "ptestz" => |op, mask| op & mask == 0, - "ptestc" => |op, mask| op & mask == mask, - "ptestnzc" => |op, mask| op & mask != 0 && op & mask != mask, - _ => unreachable!(), - }; - - let mut all_zero = true; - for i in 0..op_len { - let op = this.read_scalar(&this.project_index(&op, i)?)?.to_u64()?; - let mask = this.read_scalar(&this.project_index(&mask, i)?)?.to_u64()?; - all_zero &= f(op, mask); - } - - this.write_scalar(Scalar::from_i32(all_zero.into()), dest)?; + let res = bin_op_folded(this, op, mask, true, |acc, op, mask| { + let op = op.to_scalar().to_uint(op.layout.size)?; + let mask = mask.to_scalar().to_uint(mask.layout.size)?; + Ok(match unprefixed_name { + "ptestz" => acc && (op & mask) == 0, + "ptestc" => acc && (op & mask) == mask, + "ptestnzc" => acc && (op & mask) != 0 && (op & mask) != mask, + _ => unreachable!(), + }) + })?; + + this.write_scalar(Scalar::from_i32(res.into()), dest)?; } _ => return Ok(EmulateForeignItemResult::NotSupported), } From 44bf5fc51747f2f5f92ce3b5cda398edac5d5475 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Thu, 7 Dec 2023 12:48:45 +0100 Subject: [PATCH 09/20] Move implementation of SSE4.1 dpps/dppd to helper function --- src/shims/x86/mod.rs | 50 ++++++++++++++++++++++++++++++++++++++++++ src/shims/x86/sse41.rs | 39 ++------------------------------ 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 6d361f5d2a..d8c3b4826a 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -616,6 +616,56 @@ fn horizontal_bin_op<'tcx>( Ok(()) } +/// Conditionally multiplies the packed floating-point elements in +/// `left` and `right` using the high 4 bits in `imm`, sums the calculated +/// products (up to 4), and conditionally stores the sum in `dest` using +/// the low 4 bits of `imm`. +fn conditional_dot_product<'tcx>( + this: &mut crate::MiriInterpCx<'_, 'tcx>, + left: &OpTy<'tcx, Provenance>, + right: &OpTy<'tcx, Provenance>, + imm: &OpTy<'tcx, Provenance>, + dest: &PlaceTy<'tcx, Provenance>, +) -> InterpResult<'tcx, ()> { + let (left, left_len) = this.operand_to_simd(left)?; + let (right, right_len) = this.operand_to_simd(right)?; + let (dest, dest_len) = this.place_to_simd(dest)?; + + assert_eq!(left_len, right_len); + assert!(dest_len <= 4); + + let imm = this.read_scalar(imm)?.to_u8()?; + + let element_layout = left.layout.field(this, 0); + + // Calculate dot product + // Elements are floating point numbers, but we can use `from_int` + // because the representation of 0.0 is all zero bits. + let mut sum = ImmTy::from_int(0u8, element_layout); + for i in 0..left_len { + if imm & (1 << i.checked_add(4).unwrap()) != 0 { + let left = this.read_immediate(&this.project_index(&left, i)?)?; + let right = this.read_immediate(&this.project_index(&right, i)?)?; + + let mul = this.wrapping_binary_op(mir::BinOp::Mul, &left, &right)?; + sum = this.wrapping_binary_op(mir::BinOp::Add, &sum, &mul)?; + } + } + + // Write to destination (conditioned to imm) + for i in 0..dest_len { + let dest = this.project_index(&dest, i)?; + + if imm & (1 << i) != 0 { + this.write_immediate(*sum, &dest)?; + } else { + this.write_scalar(Scalar::from_int(0u8, element_layout.size), &dest)?; + } + } + + Ok(()) +} + /// Folds SIMD vectors `lhs` and `rhs` into a value of type `T` using `f`. fn bin_op_folded<'tcx, T>( this: &crate::MiriInterpCx<'_, 'tcx>, diff --git a/src/shims/x86/sse41.rs b/src/shims/x86/sse41.rs index 2683105b00..08e3404a22 100644 --- a/src/shims/x86/sse41.rs +++ b/src/shims/x86/sse41.rs @@ -1,8 +1,7 @@ -use rustc_middle::mir; use rustc_span::Symbol; use rustc_target::spec::abi::Abi; -use super::{bin_op_folded, round_all, round_first}; +use super::{bin_op_folded, conditional_dot_product, round_all, round_first}; use crate::*; use shims::foreign_items::EmulateForeignItemResult; @@ -104,41 +103,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: let [left, right, imm] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let (left, left_len) = this.operand_to_simd(left)?; - let (right, right_len) = this.operand_to_simd(right)?; - let (dest, dest_len) = this.place_to_simd(dest)?; - - assert_eq!(left_len, right_len); - assert!(dest_len <= 4); - - let imm = this.read_scalar(imm)?.to_u8()?; - - let element_layout = left.layout.field(this, 0); - - // Calculate dot product - // Elements are floating point numbers, but we can use `from_int` - // because the representation of 0.0 is all zero bits. - let mut sum = ImmTy::from_int(0u8, element_layout); - for i in 0..left_len { - if imm & (1 << i.checked_add(4).unwrap()) != 0 { - let left = this.read_immediate(&this.project_index(&left, i)?)?; - let right = this.read_immediate(&this.project_index(&right, i)?)?; - - let mul = this.wrapping_binary_op(mir::BinOp::Mul, &left, &right)?; - sum = this.wrapping_binary_op(mir::BinOp::Add, &sum, &mul)?; - } - } - - // Write to destination (conditioned to imm) - for i in 0..dest_len { - let dest = this.project_index(&dest, i)?; - - if imm & (1 << i) != 0 { - this.write_immediate(*sum, &dest)?; - } else { - this.write_scalar(Scalar::from_int(0u8, element_layout.size), &dest)?; - } - } + conditional_dot_product(this, left, right, imm, dest)?; } // Used to implement the _mm_floor_ss, _mm_ceil_ss and _mm_round_ss // functions. Rounds the first element of `right` according to `rounding` From 092eb11f2cb5021c3e3f1607f8410c8ba766a957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Fri, 8 Dec 2023 22:39:11 +0100 Subject: [PATCH 10/20] Fix x86 SSE4.1 ptestnzc `(op & mask) == 0` and `(op & mask) == mask` need each to be calculated for the whole vector. For example, given * `op = [0b100, 0b010]` * `mask = [0b100, 0b110]` The correct result would be: * `op & mask = [0b100, 0b010]` Comparisons are done on the vector as a whole: * `all_zero = (op & mask) == [0, 0] = false` * `masked_set = (op & mask) == mask = false` * `!all_zero && !masked_set = true` The previous method: `op & mask = [0b100, 0b010]` Comparisons are done element-wise: * `all_zero = (op & mask) == [0, 0] = [true, true]` * `masked_set = (op & mask) == mask = [true, false]` * `!all_zero && !masked_set = [true, false]` After folding with AND, the final result would be `false`, which is incorrect. --- src/shims/x86/mod.rs | 49 ++++++++++++++++-------------- src/shims/x86/sse41.rs | 23 ++++++-------- tests/pass/intrinsics-x86-sse41.rs | 5 +++ 3 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index d8c3b4826a..1aaf820f46 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -666,30 +666,33 @@ fn conditional_dot_product<'tcx>( Ok(()) } -/// Folds SIMD vectors `lhs` and `rhs` into a value of type `T` using `f`. -fn bin_op_folded<'tcx, T>( +/// Calculates two booleans. +/// +/// The first is true when all the bits of `op & mask` are zero. +/// The second is true when `(op & mask) == mask` +fn test_bits_masked<'tcx>( this: &crate::MiriInterpCx<'_, 'tcx>, - lhs: &OpTy<'tcx, Provenance>, - rhs: &OpTy<'tcx, Provenance>, - init: T, - mut f: impl FnMut(T, ImmTy<'tcx, Provenance>, ImmTy<'tcx, Provenance>) -> InterpResult<'tcx, T>, -) -> InterpResult<'tcx, T> { - assert_eq!(lhs.layout, rhs.layout); - - let (lhs, lhs_len) = this.operand_to_simd(lhs)?; - let (rhs, rhs_len) = this.operand_to_simd(rhs)?; - - assert_eq!(lhs_len, rhs_len); - - let mut acc = init; - for i in 0..lhs_len { - let lhs = this.project_index(&lhs, i)?; - let rhs = this.project_index(&rhs, i)?; - - let lhs = this.read_immediate(&lhs)?; - let rhs = this.read_immediate(&rhs)?; - acc = f(acc, lhs, rhs)?; + op: &OpTy<'tcx, Provenance>, + mask: &OpTy<'tcx, Provenance>, +) -> InterpResult<'tcx, (bool, bool)> { + assert_eq!(op.layout, mask.layout); + + let (op, op_len) = this.operand_to_simd(op)?; + let (mask, mask_len) = this.operand_to_simd(mask)?; + + assert_eq!(op_len, mask_len); + + let mut all_zero = true; + let mut masked_set = true; + for i in 0..op_len { + let op = this.project_index(&op, i)?; + let mask = this.project_index(&mask, i)?; + + let op = this.read_scalar(&op)?.to_uint(op.layout.size)?; + let mask = this.read_scalar(&mask)?.to_uint(mask.layout.size)?; + all_zero &= (op & mask) == 0; + masked_set &= (op & mask) == mask; } - Ok(acc) + Ok((all_zero, masked_set)) } diff --git a/src/shims/x86/sse41.rs b/src/shims/x86/sse41.rs index 08e3404a22..67bb63f0a3 100644 --- a/src/shims/x86/sse41.rs +++ b/src/shims/x86/sse41.rs @@ -1,7 +1,7 @@ use rustc_span::Symbol; use rustc_target::spec::abi::Abi; -use super::{bin_op_folded, conditional_dot_product, round_all, round_first}; +use super::{conditional_dot_product, round_all, round_first, test_bits_masked}; use crate::*; use shims::foreign_items::EmulateForeignItemResult; @@ -217,21 +217,18 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: } // Used to implement the _mm_testz_si128, _mm_testc_si128 // and _mm_testnzc_si128 functions. - // Tests `op & mask == 0`, `op & mask == mask` or - // `op & mask != 0 && op & mask != mask` + // Tests `(op & mask) == 0`, `(op & mask) == mask` or + // `(op & mask) != 0 && (op & mask) != mask` "ptestz" | "ptestc" | "ptestnzc" => { let [op, mask] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let res = bin_op_folded(this, op, mask, true, |acc, op, mask| { - let op = op.to_scalar().to_uint(op.layout.size)?; - let mask = mask.to_scalar().to_uint(mask.layout.size)?; - Ok(match unprefixed_name { - "ptestz" => acc && (op & mask) == 0, - "ptestc" => acc && (op & mask) == mask, - "ptestnzc" => acc && (op & mask) != 0 && (op & mask) != mask, - _ => unreachable!(), - }) - })?; + let (all_zero, masked_set) = test_bits_masked(this, op, mask)?; + let res = match unprefixed_name { + "ptestz" => all_zero, + "ptestc" => masked_set, + "ptestnzc" => !all_zero && !masked_set, + _ => unreachable!(), + }; this.write_scalar(Scalar::from_i32(res.into()), dest)?; } diff --git a/tests/pass/intrinsics-x86-sse41.rs b/tests/pass/intrinsics-x86-sse41.rs index 13856d29d3..06607f3fd5 100644 --- a/tests/pass/intrinsics-x86-sse41.rs +++ b/tests/pass/intrinsics-x86-sse41.rs @@ -515,6 +515,11 @@ unsafe fn test_sse41() { let mask = _mm_set1_epi8(0b101); let r = _mm_testnzc_si128(a, mask); assert_eq!(r, 0); + + let a = _mm_setr_epi32(0b100, 0, 0, 0b010); + let mask = _mm_setr_epi32(0b100, 0, 0, 0b110); + let r = _mm_testnzc_si128(a, mask); + assert_eq!(r, 1); } test_mm_testnzc_si128(); } From ea5c1c3f6e9ed05e131d196fad1e2c2f9b2ee40f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 10 Dec 2023 09:03:44 +0100 Subject: [PATCH 11/20] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 8841b0c95c..73e201aa30 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -d6fa38a9b2426487e010a6c16862132f89755e41 +61afc9c92896a43fce92bd5e3bba6274c5e3e960 From 22fd2a6db94361827c46f9671cd3d29cf6ed36f5 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 10 Dec 2023 12:23:40 -0500 Subject: [PATCH 12/20] Return MAP_FAILED when mmap fails --- src/shims/unix/linux/mem.rs | 2 +- src/shims/unix/mem.rs | 6 +-- tests/pass-dep/shims/mmap.rs | 79 ++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/shims/unix/linux/mem.rs b/src/shims/unix/linux/mem.rs index 026fd6ae5e..d1b4416cb1 100644 --- a/src/shims/unix/linux/mem.rs +++ b/src/shims/unix/linux/mem.rs @@ -38,7 +38,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if flags & this.eval_libc_i32("MREMAP_MAYMOVE") == 0 { // We only support MREMAP_MAYMOVE, so not passing the flag is just a failure this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; - return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + return Ok(this.eval_libc("MAP_FAILED")); } let old_address = Machine::ptr_from_addr_cast(this, old_address)?; diff --git a/src/shims/unix/mem.rs b/src/shims/unix/mem.rs index cf36d4b191..5b84f047f4 100644 --- a/src/shims/unix/mem.rs +++ b/src/shims/unix/mem.rs @@ -48,11 +48,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // First, we do some basic argument validation as required by mmap if (flags & (map_private | map_shared)).count_ones() != 1 { this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; - return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + return Ok(this.eval_libc("MAP_FAILED")); } if length == 0 { this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; - return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + return Ok(this.eval_libc("MAP_FAILED")); } // If a user tries to map a file, we want to loudly inform them that this is not going @@ -72,7 +72,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Miri doesn't support MAP_FIXED or any any protections other than PROT_READ|PROT_WRITE. if flags & map_fixed != 0 || prot != prot_read | prot_write { this.set_last_error(Scalar::from_i32(this.eval_libc_i32("ENOTSUP")))?; - return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + return Ok(this.eval_libc("MAP_FAILED")); } // Miri does not support shared mappings, or any of the other extensions that for example diff --git a/tests/pass-dep/shims/mmap.rs b/tests/pass-dep/shims/mmap.rs index 03ffc3b389..fefbed80fd 100644 --- a/tests/pass-dep/shims/mmap.rs +++ b/tests/pass-dep/shims/mmap.rs @@ -2,6 +2,7 @@ //@compile-flags: -Zmiri-disable-isolation -Zmiri-permissive-provenance #![feature(strict_provenance)] +use std::io::Error; use std::{ptr, slice}; fn test_mmap() { @@ -32,6 +33,67 @@ fn test_mmap() { let just_an_address = ptr::invalid_mut(ptr.addr()); let res = unsafe { libc::munmap(just_an_address, page_size) }; assert_eq!(res, 0i32); + + // Test all of our error conditions + let ptr = unsafe { + libc::mmap( + ptr::null_mut(), + page_size, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_SHARED, // Can't be both private and shared + -1, + 0, + ) + }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); + + let ptr = unsafe { + libc::mmap( + ptr::null_mut(), + 0, // Can't map no memory + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ) + }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); + + let ptr = unsafe { + libc::mmap( + ptr::invalid_mut(page_size * 64), + page_size, + libc::PROT_READ | libc::PROT_WRITE, + // We don't support MAP_FIXED + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS | libc::MAP_FIXED, + -1, + 0, + ) + }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::ENOTSUP); + + // We don't support protections other than read+write + for prot in [libc::PROT_NONE, libc::PROT_EXEC, libc::PROT_READ, libc::PROT_WRITE] { + let ptr = unsafe { + libc::mmap( + ptr::null_mut(), + page_size, + prot, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ) + }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::ENOTSUP); + } + + let res = unsafe { libc::munmap(ptr::invalid_mut(1), page_size) }; + assert_eq!(res, -1); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); } #[cfg(target_os = "linux")] @@ -61,6 +123,23 @@ fn test_mremap() { let res = unsafe { libc::munmap(ptr, page_size * 2) }; assert_eq!(res, 0i32); + + // Test all of our error conditions + // Not aligned + let ptr = + unsafe { libc::mremap(ptr::invalid_mut(1), page_size, page_size, libc::MREMAP_MAYMOVE) }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); + + // Zero size + let ptr = unsafe { libc::mremap(ptr::null_mut(), page_size, 0, libc::MREMAP_MAYMOVE) }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); + + // Not setting MREMAP_MAYMOVE + let ptr = unsafe { libc::mremap(ptr::null_mut(), page_size, page_size, 0) }; + assert_eq!(ptr, libc::MAP_FAILED); + assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL); } fn main() { From 9875edc91ee9f2e5e15c6aa62ac51f002f3add71 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 11 Dec 2023 15:37:50 +0100 Subject: [PATCH 13/20] use Waker::noop instead of defining our own Waker --- tests/pass/async-fn.rs | 13 +++---------- tests/pass/dyn-star.rs | 18 ++---------------- tests/pass/future-self-referential.rs | 18 +++--------------- tests/pass/issues/issue-miri-2068.rs | 18 ++++++------------ tests/pass/move-data-across-await-point.rs | 13 +++---------- 5 files changed, 17 insertions(+), 63 deletions(-) diff --git a/tests/pass/async-fn.rs b/tests/pass/async-fn.rs index 1d5d9a27cc..69f35afe86 100644 --- a/tests/pass/async-fn.rs +++ b/tests/pass/async-fn.rs @@ -1,4 +1,5 @@ #![feature(never_type)] +#![feature(noop_waker)] use std::future::Future; @@ -58,17 +59,9 @@ async fn hello_world() { } fn run_fut(fut: impl Future) -> T { - use std::sync::Arc; - use std::task::{Context, Poll, Wake, Waker}; + use std::task::{Context, Poll, Waker}; - struct MyWaker; - impl Wake for MyWaker { - fn wake(self: Arc) { - unimplemented!() - } - } - - let waker = Waker::from(Arc::new(MyWaker)); + let waker = Waker::noop(); let mut context = Context::from_waker(&waker); let mut pinned = Box::pin(fut); diff --git a/tests/pass/dyn-star.rs b/tests/pass/dyn-star.rs index 1fac16352a..8e26c4850f 100644 --- a/tests/pass/dyn-star.rs +++ b/tests/pass/dyn-star.rs @@ -1,6 +1,7 @@ #![feature(dyn_star)] #![allow(incomplete_features)] #![feature(custom_inner_attributes)] +#![feature(noop_waker)] // rustfmt destroys `dyn* Trait` syntax #![rustfmt::skip] @@ -89,25 +90,10 @@ fn dispatch_on_pin_mut() { use std::pin::Pin; use std::task::*; - pub fn noop_waker() -> Waker { - let raw = RawWaker::new(std::ptr::null(), &NOOP_WAKER_VTABLE); - - // SAFETY: the contracts for RawWaker and RawWakerVTable are upheld - unsafe { Waker::from_raw(raw) } - } - - const NOOP_WAKER_VTABLE: RawWakerVTable = RawWakerVTable::new(noop_clone, noop, noop, noop); - - unsafe fn noop_clone(_p: *const ()) -> RawWaker { - RawWaker::new(std::ptr::null(), &NOOP_WAKER_VTABLE) - } - - unsafe fn noop(_p: *const ()) {} - let mut fut = async_main(); // Poll loop, just to test the future... - let waker = noop_waker(); + let waker = Waker::noop(); let ctx = &mut Context::from_waker(&waker); loop { diff --git a/tests/pass/future-self-referential.rs b/tests/pass/future-self-referential.rs index 763eceeb6f..38cb700fd5 100644 --- a/tests/pass/future-self-referential.rs +++ b/tests/pass/future-self-referential.rs @@ -1,5 +1,6 @@ //@revisions: stack tree //@[tree]compile-flags: -Zmiri-tree-borrows +#![feature(noop_waker)] use std::future::*; use std::marker::PhantomPinned; @@ -29,19 +30,6 @@ impl Future for Delay { } } -fn mk_waker() -> Waker { - use std::sync::Arc; - - struct MyWaker; - impl Wake for MyWaker { - fn wake(self: Arc) { - unimplemented!() - } - } - - Waker::from(Arc::new(MyWaker)) -} - async fn do_stuff() { (&mut Delay::new(1)).await; } @@ -89,7 +77,7 @@ impl Future for DoStuff { } fn run_fut(fut: impl Future) -> T { - let waker = mk_waker(); + let waker = Waker::noop(); let mut context = Context::from_waker(&waker); let mut pinned = pin!(fut); @@ -102,7 +90,7 @@ fn run_fut(fut: impl Future) -> T { } fn self_referential_box() { - let waker = mk_waker(); + let waker = Waker::noop(); let cx = &mut Context::from_waker(&waker); async fn my_fut() -> i32 { diff --git a/tests/pass/issues/issue-miri-2068.rs b/tests/pass/issues/issue-miri-2068.rs index fe4078f771..f18c4a3a06 100644 --- a/tests/pass/issues/issue-miri-2068.rs +++ b/tests/pass/issues/issue-miri-2068.rs @@ -1,19 +1,13 @@ -use core::future::Future; -use core::pin::Pin; -use core::task::{Context, Poll}; +#![feature(noop_waker)] -use std::sync::Arc; - -struct NopWaker; - -impl std::task::Wake for NopWaker { - fn wake(self: Arc) {} -} +use std::future::Future; +use std::pin::Pin; +use std::task::{Context, Poll, Waker}; pub fn fuzzing_block_on>(fut: F) -> O { let mut fut = std::pin::pin!(fut); - let waker = std::task::Waker::from(Arc::new(NopWaker)); - let mut context = std::task::Context::from_waker(&waker); + let waker = Waker::noop(); + let mut context = Context::from_waker(&waker); loop { match fut.as_mut().poll(&mut context) { Poll::Ready(v) => return v, diff --git a/tests/pass/move-data-across-await-point.rs b/tests/pass/move-data-across-await-point.rs index 5990d66fbd..9bea6ea574 100644 --- a/tests/pass/move-data-across-await-point.rs +++ b/tests/pass/move-data-across-await-point.rs @@ -1,3 +1,4 @@ +#![feature(noop_waker)] use std::future::Future; use std::ptr; @@ -53,17 +54,9 @@ fn data_moved() { } fn run_fut(fut: impl Future) -> T { - use std::sync::Arc; - use std::task::{Context, Poll, Wake, Waker}; + use std::task::{Context, Poll, Waker}; - struct MyWaker; - impl Wake for MyWaker { - fn wake(self: Arc) { - unimplemented!() - } - } - - let waker = Waker::from(Arc::new(MyWaker)); + let waker = Waker::noop(); let mut context = Context::from_waker(&waker); let mut pinned = Box::pin(fut); From 6d59c5a0b6e457df6f5a219bee2f9427ce2ec7ac Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Tue, 12 Dec 2023 05:04:22 +0000 Subject: [PATCH 14/20] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 73e201aa30..7987fadbfc 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -61afc9c92896a43fce92bd5e3bba6274c5e3e960 +e2a3c9b3f0895c866c104bd2fff2a8bf16eaf964 From 42b590b4ba504db217b74d572463c3d00b50e13c Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Tue, 12 Dec 2023 05:12:04 +0000 Subject: [PATCH 15/20] fmt --- src/borrow_tracker/stacked_borrows/diagnostics.rs | 4 +--- src/borrow_tracker/stacked_borrows/mod.rs | 3 ++- src/borrow_tracker/tree_borrows/mod.rs | 4 +--- tests/pass/enum-nullable-const-null-with-fields.rs | 1 - 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/borrow_tracker/stacked_borrows/diagnostics.rs index b31f51e5a5..b964b1f9ec 100644 --- a/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -5,9 +5,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_span::{Span, SpanData}; use rustc_target::abi::Size; -use crate::borrow_tracker::{ - AccessKind, GlobalStateInner, ProtectorKind, -}; +use crate::borrow_tracker::{AccessKind, GlobalStateInner, ProtectorKind}; use crate::*; /// Error reporting diff --git a/src/borrow_tracker/stacked_borrows/mod.rs b/src/borrow_tracker/stacked_borrows/mod.rs index 345c71f75b..eac315d043 100644 --- a/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/borrow_tracker/stacked_borrows/mod.rs @@ -18,7 +18,8 @@ use rustc_target::abi::{Abi, Size}; use crate::borrow_tracker::{ stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder}, - AccessKind, GlobalStateInner, ProtectorKind,}; + AccessKind, GlobalStateInner, ProtectorKind, +}; use crate::*; use diagnostics::{RetagCause, RetagInfo}; diff --git a/src/borrow_tracker/tree_borrows/mod.rs b/src/borrow_tracker/tree_borrows/mod.rs index 84e4e0f222..573d13bf4a 100644 --- a/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/borrow_tracker/tree_borrows/mod.rs @@ -2,9 +2,7 @@ use log::trace; use rustc_target::abi::{Abi, Size}; -use crate::borrow_tracker::{ - AccessKind, GlobalState, GlobalStateInner, ProtectorKind, -}; +use crate::borrow_tracker::{AccessKind, GlobalState, GlobalStateInner, ProtectorKind}; use rustc_middle::{ mir::{Mutability, RetagKind}, ty::{ diff --git a/tests/pass/enum-nullable-const-null-with-fields.rs b/tests/pass/enum-nullable-const-null-with-fields.rs index c42cd87f9a..86f30f42b6 100644 --- a/tests/pass/enum-nullable-const-null-with-fields.rs +++ b/tests/pass/enum-nullable-const-null-with-fields.rs @@ -1,4 +1,3 @@ - static C: Result<(), Box> = Ok(()); // This is because of yet another bad assertion (ICE) about the null side of a nullable enum. From 462f0d17403ba99cdf33b09d93327ebcb27ea3b2 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Thu, 14 Dec 2023 04:56:27 +0000 Subject: [PATCH 16/20] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 7987fadbfc..0e75e593a9 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -e2a3c9b3f0895c866c104bd2fff2a8bf16eaf964 +e6d1b0ec9859e6f5c29aaa3b6525fb625bf354ad From eddae8b02e3c63924220e5e79b6d94fcc501a1bb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 14 Dec 2023 07:57:36 +0100 Subject: [PATCH 17/20] add test for uninhabited saved locals in a coroutine --- tests/pass/coroutine.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/pass/coroutine.rs b/tests/pass/coroutine.rs index 49bfa92a05..7e1f64df04 100644 --- a/tests/pass/coroutine.rs +++ b/tests/pass/coroutine.rs @@ -220,7 +220,38 @@ fn smoke_resume_arg() { }); } +fn uninit_fields() { + // Test that uninhabited saved local doesn't make the entire variant uninhabited. + // (https://github.com/rust-lang/rust/issues/115145, https://github.com/rust-lang/rust/pull/118871) + fn conjure() -> T { + loop {} + } + + fn run(x: bool, y: bool) { + let mut c = || { + if x { + let _a: T; + if y { + _a = conjure::(); + } + yield (); + } else { + let _a: T; + if y { + _a = conjure::(); + } + yield (); + } + }; + assert!(matches!(Pin::new(&mut c).resume(()), CoroutineState::Yielded(()))); + assert!(matches!(Pin::new(&mut c).resume(()), CoroutineState::Complete(()))); + } + + run::(false, false); +} + fn main() { basic(); smoke_resume_arg(); + uninit_fields(); } From bad75494cf8691e815aaab9196db44012330f9bf Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Fri, 15 Dec 2023 04:54:30 +0000 Subject: [PATCH 18/20] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 0e75e593a9..e82b7f8415 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -e6d1b0ec9859e6f5c29aaa3b6525fb625bf354ad +604f185fae9a4b0edf7e28f616a0f53880f8f074 From 226ef8265194d4fd61c31bd49da7df47a72dcc75 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Thu, 14 Dec 2023 18:22:23 -0500 Subject: [PATCH 19/20] Add the test minimized from deadpool --- tests/pass/async-fn.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/pass/async-fn.rs b/tests/pass/async-fn.rs index 69f35afe86..6c92735df0 100644 --- a/tests/pass/async-fn.rs +++ b/tests/pass/async-fn.rs @@ -58,6 +58,21 @@ async fn hello_world() { read_exact(&mut reader, &mut marker).await.unwrap(); } +// This example comes from https://github.com/rust-lang/rust/issues/115145 +async fn uninhabited_variant() { + async fn unreachable(_: Never) {} + + let c = async {}; + match None:: { + None => { + c.await; + } + Some(r) => { + unreachable(r).await; + } + } +} + fn run_fut(fut: impl Future) -> T { use std::task::{Context, Poll, Waker}; @@ -80,4 +95,5 @@ fn main() { assert_eq!(run_fut(includes_never(false, 4)), 16); assert_eq!(run_fut(partial_init(4)), 8); run_fut(hello_world()); + run_fut(uninhabited_variant()); } From f82a1c255c06d43605657713a768acec0ecfc6f6 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 15 Dec 2023 23:49:28 -0500 Subject: [PATCH 20/20] Visit the AllocIds and BorTags in borrow state FrameExtra --- src/borrow_tracker/mod.rs | 16 ++++++++-------- tests/pass/protector-gc.rs | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 tests/pass/protector-gc.rs diff --git a/src/borrow_tracker/mod.rs b/src/borrow_tracker/mod.rs index 8fae526922..a6961208ff 100644 --- a/src/borrow_tracker/mod.rs +++ b/src/borrow_tracker/mod.rs @@ -65,7 +65,7 @@ pub struct FrameState { /// incremental updates of the global list of protected tags stored in the /// `stacked_borrows::GlobalState` upon function return, and if we attempt to pop a protected /// tag, to identify which call is responsible for protecting the tag. - /// See `Stack::item_popped` for more explanation. + /// See `Stack::item_invalidated` for more explanation. /// Tree Borrows also needs to know which allocation these tags /// belong to so that it can perform a read through them immediately before /// the frame gets popped. @@ -76,8 +76,10 @@ pub struct FrameState { } impl VisitProvenance for FrameState { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // `protected_tags` are already recorded by `GlobalStateInner`. + fn visit_provenance(&self, visit: &mut VisitWith<'_>) { + for (id, tag) in &self.protected_tags { + visit(Some(*id), Some(*tag)); + } } } @@ -98,7 +100,7 @@ pub struct GlobalStateInner { /// An item is protected if its tag is in this set, *and* it has the "protected" bit set. /// We add tags to this when they are created with a protector in `reborrow`, and /// we remove tags from this when the call which is protecting them returns, in - /// `GlobalStateInner::end_call`. See `Stack::item_popped` for more details. + /// `GlobalStateInner::end_call`. See `Stack::item_invalidated` for more details. protected_tags: FxHashMap, /// The pointer ids to trace tracked_pointer_tags: FxHashSet, @@ -111,10 +113,8 @@ pub struct GlobalStateInner { } impl VisitProvenance for GlobalStateInner { - fn visit_provenance(&self, visit: &mut VisitWith<'_>) { - for &tag in self.protected_tags.keys() { - visit(None, Some(tag)); - } + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { + // All the provenance in protected_tags is also stored in FrameState, and visited there. // The only other candidate is base_ptr_tags, and that does not need visiting since we don't ever // GC the bottommost/root tag. } diff --git a/tests/pass/protector-gc.rs b/tests/pass/protector-gc.rs new file mode 100644 index 0000000000..4e422c323e --- /dev/null +++ b/tests/pass/protector-gc.rs @@ -0,0 +1,17 @@ +// When we pop a stack frame with weak protectors, we need to check if the protected pointer's +// allocation is still live. If the provenance GC only knows about the BorTag that is protected, +// we can ICE. This test checks that we don't. +// See https://github.com/rust-lang/miri/issues/3228 + +#[path = "../utils/mod.rs"] +mod utils; + +#[allow(unused)] +fn oof(mut b: Box) { + b = Box::new(0u8); + utils::run_provenance_gc(); +} + +fn main() { + oof(Box::new(0u8)); +}