From e928185f6e4912891748da963b5d5fdfd44dfdf0 Mon Sep 17 00:00:00 2001 From: bendn Date: Sat, 11 May 2024 08:41:04 +0700 Subject: [PATCH 01/24] support `f*_algebraic` --- src/tools/miri/src/intrinsics/mod.rs | 22 +++++++++++++ .../pass/intrinsics/float_algebraic_math.rs | 32 +++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 src/tools/miri/tests/pass/intrinsics/float_algebraic_math.rs diff --git a/src/tools/miri/src/intrinsics/mod.rs b/src/tools/miri/src/intrinsics/mod.rs index effd7f6d543..e38a9e4d5e0 100644 --- a/src/tools/miri/src/intrinsics/mod.rs +++ b/src/tools/miri/src/intrinsics/mod.rs @@ -256,6 +256,28 @@ fn emulate_intrinsic_by_name( let res = this.adjust_nan(res, &[f]); this.write_scalar(res, dest)?; } + #[rustfmt::skip] + | "fadd_algebraic" + | "fsub_algebraic" + | "fmul_algebraic" + | "fdiv_algebraic" + | "frem_algebraic" + => { + let [a, b] = check_arg_count(args)?; + let a = this.read_immediate(a)?; + let b = this.read_immediate(b)?; + let op = match intrinsic_name { + "fadd_algebraic" => mir::BinOp::Add, + "fsub_algebraic" => mir::BinOp::Sub, + "fmul_algebraic" => mir::BinOp::Mul, + "fdiv_algebraic" => mir::BinOp::Div, + "frem_algebraic" => mir::BinOp::Rem, + _ => bug!(), + }; + let res = this.wrapping_binary_op(op, &a, &b)?; + // `wrapping_binary_op` already called `generate_nan` if necessary. + this.write_immediate(*res, dest)?; + } #[rustfmt::skip] | "fadd_fast" diff --git a/src/tools/miri/tests/pass/intrinsics/float_algebraic_math.rs b/src/tools/miri/tests/pass/intrinsics/float_algebraic_math.rs new file mode 100644 index 00000000000..f6f083f7b5f --- /dev/null +++ b/src/tools/miri/tests/pass/intrinsics/float_algebraic_math.rs @@ -0,0 +1,32 @@ +#![feature(core_intrinsics)] + +use std::intrinsics::{ + fadd_algebraic, fdiv_algebraic, fmul_algebraic, frem_algebraic, fsub_algebraic, +}; + +#[inline(never)] +pub fn test_operations_f64(a: f64, b: f64) { + // make sure they all map to the correct operation + assert_eq!(fadd_algebraic(a, b), a + b); + assert_eq!(fsub_algebraic(a, b), a - b); + assert_eq!(fmul_algebraic(a, b), a * b); + assert_eq!(fdiv_algebraic(a, b), a / b); + assert_eq!(frem_algebraic(a, b), a % b); +} + +#[inline(never)] +pub fn test_operations_f32(a: f32, b: f32) { + // make sure they all map to the correct operation + assert_eq!(fadd_algebraic(a, b), a + b); + assert_eq!(fsub_algebraic(a, b), a - b); + assert_eq!(fmul_algebraic(a, b), a * b); + assert_eq!(fdiv_algebraic(a, b), a / b); + assert_eq!(frem_algebraic(a, b), a % b); +} + +fn main() { + test_operations_f64(1., 2.); + test_operations_f64(10., 5.); + test_operations_f32(11., 2.); + test_operations_f32(10., 15.); +} From bf5906fbb44f8bbe9707bf98ddb4d4705e4b302a Mon Sep 17 00:00:00 2001 From: tiif Date: Sun, 12 May 2024 15:01:18 +0800 Subject: [PATCH 02/24] Add non-null pointer for posix_memalign --- .../miri/src/shims/unix/foreign_items.rs | 16 +++++------- .../posix_memalign_size_zero_double_free.rs | 14 +++++++++++ ...osix_memalign_size_zero_double_free.stderr | 25 +++++++++++++++++++ .../libc/posix_memalign_size_zero_leak.rs | 10 ++++++++ .../libc/posix_memalign_size_zero_leak.stderr | 15 +++++++++++ .../miri/tests/pass-dep/libc/libc-mem.rs | 7 +++--- 6 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_double_free.rs create mode 100644 src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_double_free.stderr create mode 100644 src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.rs create mode 100644 src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 08f64e499b5..b5165548d1b 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -259,16 +259,12 @@ fn emulate_foreign_item_inner( let einval = this.eval_libc_i32("EINVAL"); this.write_int(einval, dest)?; } else { - if size == 0 { - this.write_null(&ret)?; - } else { - let ptr = this.allocate_ptr( - Size::from_bytes(size), - Align::from_bytes(align).unwrap(), - MiriMemoryKind::C.into(), - )?; - this.write_pointer(ptr, &ret)?; - } + let ptr = this.allocate_ptr( + Size::from_bytes(size), + Align::from_bytes(align).unwrap(), + MiriMemoryKind::C.into(), + )?; + this.write_pointer(ptr, &ret)?; this.write_null(dest)?; } } diff --git a/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_double_free.rs b/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_double_free.rs new file mode 100644 index 00000000000..b6b7b007f2b --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_double_free.rs @@ -0,0 +1,14 @@ +//@ignore-target-windows: No posix_memalign on Windows + +use std::ptr; + +fn main() { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 64; + let size = 0; + unsafe { + let _ = libc::posix_memalign(&mut ptr, align, size); + libc::free(ptr); + libc::free(ptr); //~ERROR: dangling + } +} diff --git a/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_double_free.stderr b/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_double_free.stderr new file mode 100644 index 00000000000..3ed117c5a0a --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_double_free.stderr @@ -0,0 +1,25 @@ +error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling + --> $DIR/posix_memalign_size_zero_double_free.rs:LL:CC + | +LL | libc::free(ptr); + | ^^^^^^^^^^^^^^^ memory access failed: ALLOC has been freed, so this pointer is dangling + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information +help: ALLOC was allocated here: + --> $DIR/posix_memalign_size_zero_double_free.rs:LL:CC + | +LL | let _ = libc::posix_memalign(&mut ptr, align, size); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: ALLOC was deallocated here: + --> $DIR/posix_memalign_size_zero_double_free.rs:LL:CC + | +LL | libc::free(ptr); + | ^^^^^^^^^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `main` at $DIR/posix_memalign_size_zero_double_free.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.rs b/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.rs new file mode 100644 index 00000000000..1a4c9605fe0 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.rs @@ -0,0 +1,10 @@ +//@ignore-target-windows: No posix_memalign on Windows + +use std::ptr; + +fn main() { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 64; + let size = 0; + let _ = unsafe { libc::posix_memalign(&mut ptr, align, size) }; //~ERROR: memory leak +} diff --git a/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr b/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr new file mode 100644 index 00000000000..7ea0fa31469 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr @@ -0,0 +1,15 @@ +error: memory leaked: ALLOC (C heap, size: 0, align: 64), allocated here: + --> $DIR/posix_memalign_size_zero_leak.rs:LL:CC + | +LL | let _ = unsafe { libc::posix_memalign(&mut ptr, align, size) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: BACKTRACE: + = note: inside `main` at $DIR/posix_memalign_size_zero_leak.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs index 5df3ace7496..b36fb436b57 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs @@ -190,11 +190,10 @@ fn test_memalign() { let align = 64; let size = 0; assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); - // We are not required to return null if size == 0, but we currently do. - // It's fine to remove this assert if we start returning non-null pointers. - assert!(ptr.is_null()); + // Non-null pointer is returned if size == 0. + // (This is not a guarantee, it just reflects our current behavior.) + assert!(!ptr.is_null()); assert!(ptr.is_aligned_to(align)); - // Regardless of what we return, it must be `free`able. libc::free(ptr); } From 9fc569d67f5e07941ed27d16148df8ebfad0a97b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 12 May 2024 10:09:45 +0200 Subject: [PATCH 03/24] organize float intrinsic implementations a bit --- src/tools/miri/src/intrinsics/mod.rs | 214 +++++++++++++-------------- 1 file changed, 102 insertions(+), 112 deletions(-) diff --git a/src/tools/miri/src/intrinsics/mod.rs b/src/tools/miri/src/intrinsics/mod.rs index cf5d9913817..7344846d6d9 100644 --- a/src/tools/miri/src/intrinsics/mod.rs +++ b/src/tools/miri/src/intrinsics/mod.rs @@ -167,6 +167,7 @@ fn emulate_intrinsic_by_name( // This is a "bitwise" operation, so there's no NaN non-determinism. this.write_scalar(Scalar::from_f64(f.abs()), dest)?; } + "floorf32" | "ceilf32" | "truncf32" | "roundf32" | "rintf32" => { let [f] = check_arg_count(args)?; let f = this.read_scalar(f)?.to_f32()?; @@ -182,6 +183,22 @@ fn emulate_intrinsic_by_name( let res = this.adjust_nan(res, &[f]); this.write_scalar(res, dest)?; } + "floorf64" | "ceilf64" | "truncf64" | "roundf64" | "rintf64" => { + let [f] = check_arg_count(args)?; + let f = this.read_scalar(f)?.to_f64()?; + let mode = match intrinsic_name { + "floorf64" => Round::TowardNegative, + "ceilf64" => Round::TowardPositive, + "truncf64" => Round::TowardZero, + "roundf64" => Round::NearestTiesToAway, + "rintf64" => Round::NearestTiesToEven, + _ => bug!(), + }; + let res = f.round_to_integral(mode).value; + let res = this.adjust_nan(res, &[f]); + this.write_scalar(res, dest)?; + } + #[rustfmt::skip] | "sinf32" | "cosf32" @@ -211,22 +228,6 @@ fn emulate_intrinsic_by_name( let res = this.adjust_nan(res, &[f]); this.write_scalar(res, dest)?; } - - "floorf64" | "ceilf64" | "truncf64" | "roundf64" | "rintf64" => { - let [f] = check_arg_count(args)?; - let f = this.read_scalar(f)?.to_f64()?; - let mode = match intrinsic_name { - "floorf64" => Round::TowardNegative, - "ceilf64" => Round::TowardPositive, - "truncf64" => Round::TowardZero, - "roundf64" => Round::NearestTiesToAway, - "rintf64" => Round::NearestTiesToEven, - _ => bug!(), - }; - let res = f.round_to_integral(mode).value; - let res = this.adjust_nan(res, &[f]); - this.write_scalar(res, dest)?; - } #[rustfmt::skip] | "sinf64" | "cosf64" @@ -256,6 +257,91 @@ fn emulate_intrinsic_by_name( let res = this.adjust_nan(res, &[f]); this.write_scalar(res, dest)?; } + + "minnumf32" | "maxnumf32" | "copysignf32" => { + let [a, b] = check_arg_count(args)?; + let a = this.read_scalar(a)?.to_f32()?; + let b = this.read_scalar(b)?.to_f32()?; + let res = match intrinsic_name { + "minnumf32" => this.adjust_nan(a.min(b), &[a, b]), + "maxnumf32" => this.adjust_nan(a.max(b), &[a, b]), + "copysignf32" => a.copy_sign(b), // bitwise, no NaN adjustments + _ => bug!(), + }; + this.write_scalar(Scalar::from_f32(res), dest)?; + } + "minnumf64" | "maxnumf64" | "copysignf64" => { + let [a, b] = check_arg_count(args)?; + let a = this.read_scalar(a)?.to_f64()?; + let b = this.read_scalar(b)?.to_f64()?; + let res = match intrinsic_name { + "minnumf64" => this.adjust_nan(a.min(b), &[a, b]), + "maxnumf64" => this.adjust_nan(a.max(b), &[a, b]), + "copysignf64" => a.copy_sign(b), // bitwise, no NaN adjustments + _ => bug!(), + }; + this.write_scalar(Scalar::from_f64(res), dest)?; + } + + "fmaf32" => { + let [a, b, c] = check_arg_count(args)?; + let a = this.read_scalar(a)?.to_f32()?; + let b = this.read_scalar(b)?.to_f32()?; + let c = this.read_scalar(c)?.to_f32()?; + // FIXME: Using host floats, to work around https://github.com/rust-lang/rustc_apfloat/issues/11 + let res = a.to_host().mul_add(b.to_host(), c.to_host()).to_soft(); + let res = this.adjust_nan(res, &[a, b, c]); + this.write_scalar(res, dest)?; + } + "fmaf64" => { + let [a, b, c] = check_arg_count(args)?; + let a = this.read_scalar(a)?.to_f64()?; + let b = this.read_scalar(b)?.to_f64()?; + let c = this.read_scalar(c)?.to_f64()?; + // FIXME: Using host floats, to work around https://github.com/rust-lang/rustc_apfloat/issues/11 + let res = a.to_host().mul_add(b.to_host(), c.to_host()).to_soft(); + let res = this.adjust_nan(res, &[a, b, c]); + this.write_scalar(res, dest)?; + } + + "powf32" => { + let [f1, f2] = check_arg_count(args)?; + let f1 = this.read_scalar(f1)?.to_f32()?; + let f2 = this.read_scalar(f2)?.to_f32()?; + // Using host floats (but it's fine, this operation does not have guaranteed precision). + let res = f1.to_host().powf(f2.to_host()).to_soft(); + let res = this.adjust_nan(res, &[f1, f2]); + this.write_scalar(res, dest)?; + } + "powf64" => { + let [f1, f2] = check_arg_count(args)?; + let f1 = this.read_scalar(f1)?.to_f64()?; + let f2 = this.read_scalar(f2)?.to_f64()?; + // Using host floats (but it's fine, this operation does not have guaranteed precision). + let res = f1.to_host().powf(f2.to_host()).to_soft(); + let res = this.adjust_nan(res, &[f1, f2]); + this.write_scalar(res, dest)?; + } + + "powif32" => { + let [f, i] = check_arg_count(args)?; + let f = this.read_scalar(f)?.to_f32()?; + let i = this.read_scalar(i)?.to_i32()?; + // Using host floats (but it's fine, this operation does not have guaranteed precision). + let res = f.to_host().powi(i).to_soft(); + let res = this.adjust_nan(res, &[f]); + this.write_scalar(res, dest)?; + } + "powif64" => { + let [f, i] = check_arg_count(args)?; + let f = this.read_scalar(f)?.to_f64()?; + let i = this.read_scalar(i)?.to_i32()?; + // Using host floats (but it's fine, this operation does not have guaranteed precision). + let res = f.to_host().powi(i).to_soft(); + let res = this.adjust_nan(res, &[f]); + this.write_scalar(res, dest)?; + } + #[rustfmt::skip] | "fadd_algebraic" | "fsub_algebraic" @@ -329,102 +415,6 @@ fn emulate_intrinsic_by_name( this.write_immediate(*res, dest)?; } - #[rustfmt::skip] - | "minnumf32" - | "maxnumf32" - | "copysignf32" - => { - let [a, b] = check_arg_count(args)?; - let a = this.read_scalar(a)?.to_f32()?; - let b = this.read_scalar(b)?.to_f32()?; - let res = match intrinsic_name { - "minnumf32" => this.adjust_nan(a.min(b), &[a, b]), - "maxnumf32" => this.adjust_nan(a.max(b), &[a, b]), - "copysignf32" => a.copy_sign(b), // bitwise, no NaN adjustments - _ => bug!(), - }; - this.write_scalar(Scalar::from_f32(res), dest)?; - } - - #[rustfmt::skip] - | "minnumf64" - | "maxnumf64" - | "copysignf64" - => { - let [a, b] = check_arg_count(args)?; - let a = this.read_scalar(a)?.to_f64()?; - let b = this.read_scalar(b)?.to_f64()?; - let res = match intrinsic_name { - "minnumf64" => this.adjust_nan(a.min(b), &[a, b]), - "maxnumf64" => this.adjust_nan(a.max(b), &[a, b]), - "copysignf64" => a.copy_sign(b), // bitwise, no NaN adjustments - _ => bug!(), - }; - this.write_scalar(Scalar::from_f64(res), dest)?; - } - - "fmaf32" => { - let [a, b, c] = check_arg_count(args)?; - let a = this.read_scalar(a)?.to_f32()?; - let b = this.read_scalar(b)?.to_f32()?; - let c = this.read_scalar(c)?.to_f32()?; - // FIXME: Using host floats, to work around https://github.com/rust-lang/rustc_apfloat/issues/11 - let res = a.to_host().mul_add(b.to_host(), c.to_host()).to_soft(); - let res = this.adjust_nan(res, &[a, b, c]); - this.write_scalar(res, dest)?; - } - - "fmaf64" => { - let [a, b, c] = check_arg_count(args)?; - let a = this.read_scalar(a)?.to_f64()?; - let b = this.read_scalar(b)?.to_f64()?; - let c = this.read_scalar(c)?.to_f64()?; - // FIXME: Using host floats, to work around https://github.com/rust-lang/rustc_apfloat/issues/11 - let res = a.to_host().mul_add(b.to_host(), c.to_host()).to_soft(); - let res = this.adjust_nan(res, &[a, b, c]); - this.write_scalar(res, dest)?; - } - - "powf32" => { - let [f1, f2] = check_arg_count(args)?; - let f1 = this.read_scalar(f1)?.to_f32()?; - let f2 = this.read_scalar(f2)?.to_f32()?; - // Using host floats (but it's fine, this operation does not have guaranteed precision). - let res = f1.to_host().powf(f2.to_host()).to_soft(); - let res = this.adjust_nan(res, &[f1, f2]); - this.write_scalar(res, dest)?; - } - - "powf64" => { - let [f1, f2] = check_arg_count(args)?; - let f1 = this.read_scalar(f1)?.to_f64()?; - let f2 = this.read_scalar(f2)?.to_f64()?; - // Using host floats (but it's fine, this operation does not have guaranteed precision). - let res = f1.to_host().powf(f2.to_host()).to_soft(); - let res = this.adjust_nan(res, &[f1, f2]); - this.write_scalar(res, dest)?; - } - - "powif32" => { - let [f, i] = check_arg_count(args)?; - let f = this.read_scalar(f)?.to_f32()?; - let i = this.read_scalar(i)?.to_i32()?; - // Using host floats (but it's fine, this operation does not have guaranteed precision). - let res = f.to_host().powi(i).to_soft(); - let res = this.adjust_nan(res, &[f]); - this.write_scalar(res, dest)?; - } - - "powif64" => { - let [f, i] = check_arg_count(args)?; - let f = this.read_scalar(f)?.to_f64()?; - let i = this.read_scalar(i)?.to_i32()?; - // Using host floats (but it's fine, this operation does not have guaranteed precision). - let res = f.to_host().powi(i).to_soft(); - let res = this.adjust_nan(res, &[f]); - this.write_scalar(res, dest)?; - } - "float_to_int_unchecked" => { let [val] = check_arg_count(args)?; let val = this.read_immediate(val)?; From 5d76ec9cdde57099e3a07bda1b0776a398b24497 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 12 May 2024 10:21:00 +0200 Subject: [PATCH 04/24] merge float tests into one --- src/tools/miri/tests/pass/float.rs | 67 +++++++++++++++++++ src/tools/miri/tests/pass/float_fast_math.rs | 34 ---------- .../pass/intrinsics/float_algebraic_math.rs | 32 --------- 3 files changed, 67 insertions(+), 66 deletions(-) delete mode 100644 src/tools/miri/tests/pass/float_fast_math.rs delete mode 100644 src/tools/miri/tests/pass/intrinsics/float_algebraic_math.rs diff --git a/src/tools/miri/tests/pass/float.rs b/src/tools/miri/tests/pass/float.rs index 1bb44d56bf6..8aea9b3e6f9 100644 --- a/src/tools/miri/tests/pass/float.rs +++ b/src/tools/miri/tests/pass/float.rs @@ -1,5 +1,6 @@ #![feature(stmt_expr_attributes)] #![feature(float_gamma)] +#![feature(core_intrinsics)] #![allow(arithmetic_overflow)] use std::fmt::Debug; @@ -22,6 +23,8 @@ fn main() { rounding(); mul_add(); libm(); + test_fast(); + test_algebraic(); } // Helper function to avoid promotion so that this tests "run-time" casts, not CTFE. @@ -751,3 +754,67 @@ fn ldexp(a: f64, b: i32) -> f64 { assert_approx_eq!(val, (2.0 * f64::consts::PI.sqrt()).ln()); assert_eq!(sign, -1); } + +fn test_fast() { + use std::intrinsics::{fadd_fast, fdiv_fast, fmul_fast, frem_fast, fsub_fast}; + + #[inline(never)] + pub fn test_operations_f64(a: f64, b: f64) { + // make sure they all map to the correct operation + unsafe { + assert_eq!(fadd_fast(a, b), a + b); + assert_eq!(fsub_fast(a, b), a - b); + assert_eq!(fmul_fast(a, b), a * b); + assert_eq!(fdiv_fast(a, b), a / b); + assert_eq!(frem_fast(a, b), a % b); + } + } + + #[inline(never)] + pub fn test_operations_f32(a: f32, b: f32) { + // make sure they all map to the correct operation + unsafe { + assert_eq!(fadd_fast(a, b), a + b); + assert_eq!(fsub_fast(a, b), a - b); + assert_eq!(fmul_fast(a, b), a * b); + assert_eq!(fdiv_fast(a, b), a / b); + assert_eq!(frem_fast(a, b), a % b); + } + } + + test_operations_f64(1., 2.); + test_operations_f64(10., 5.); + test_operations_f32(11., 2.); + test_operations_f32(10., 15.); +} + +fn test_algebraic() { + use std::intrinsics::{ + fadd_algebraic, fdiv_algebraic, fmul_algebraic, frem_algebraic, fsub_algebraic, + }; + + #[inline(never)] + pub fn test_operations_f64(a: f64, b: f64) { + // make sure they all map to the correct operation + assert_eq!(fadd_algebraic(a, b), a + b); + assert_eq!(fsub_algebraic(a, b), a - b); + assert_eq!(fmul_algebraic(a, b), a * b); + assert_eq!(fdiv_algebraic(a, b), a / b); + assert_eq!(frem_algebraic(a, b), a % b); + } + + #[inline(never)] + pub fn test_operations_f32(a: f32, b: f32) { + // make sure they all map to the correct operation + assert_eq!(fadd_algebraic(a, b), a + b); + assert_eq!(fsub_algebraic(a, b), a - b); + assert_eq!(fmul_algebraic(a, b), a * b); + assert_eq!(fdiv_algebraic(a, b), a / b); + assert_eq!(frem_algebraic(a, b), a % b); + } + + test_operations_f64(1., 2.); + test_operations_f64(10., 5.); + test_operations_f32(11., 2.); + test_operations_f32(10., 15.); +} diff --git a/src/tools/miri/tests/pass/float_fast_math.rs b/src/tools/miri/tests/pass/float_fast_math.rs deleted file mode 100644 index 52d985667df..00000000000 --- a/src/tools/miri/tests/pass/float_fast_math.rs +++ /dev/null @@ -1,34 +0,0 @@ -#![feature(core_intrinsics)] - -use std::intrinsics::{fadd_fast, fdiv_fast, fmul_fast, frem_fast, fsub_fast}; - -#[inline(never)] -pub fn test_operations_f64(a: f64, b: f64) { - // make sure they all map to the correct operation - unsafe { - assert_eq!(fadd_fast(a, b), a + b); - assert_eq!(fsub_fast(a, b), a - b); - assert_eq!(fmul_fast(a, b), a * b); - assert_eq!(fdiv_fast(a, b), a / b); - assert_eq!(frem_fast(a, b), a % b); - } -} - -#[inline(never)] -pub fn test_operations_f32(a: f32, b: f32) { - // make sure they all map to the correct operation - unsafe { - assert_eq!(fadd_fast(a, b), a + b); - assert_eq!(fsub_fast(a, b), a - b); - assert_eq!(fmul_fast(a, b), a * b); - assert_eq!(fdiv_fast(a, b), a / b); - assert_eq!(frem_fast(a, b), a % b); - } -} - -fn main() { - test_operations_f64(1., 2.); - test_operations_f64(10., 5.); - test_operations_f32(11., 2.); - test_operations_f32(10., 15.); -} diff --git a/src/tools/miri/tests/pass/intrinsics/float_algebraic_math.rs b/src/tools/miri/tests/pass/intrinsics/float_algebraic_math.rs deleted file mode 100644 index f6f083f7b5f..00000000000 --- a/src/tools/miri/tests/pass/intrinsics/float_algebraic_math.rs +++ /dev/null @@ -1,32 +0,0 @@ -#![feature(core_intrinsics)] - -use std::intrinsics::{ - fadd_algebraic, fdiv_algebraic, fmul_algebraic, frem_algebraic, fsub_algebraic, -}; - -#[inline(never)] -pub fn test_operations_f64(a: f64, b: f64) { - // make sure they all map to the correct operation - assert_eq!(fadd_algebraic(a, b), a + b); - assert_eq!(fsub_algebraic(a, b), a - b); - assert_eq!(fmul_algebraic(a, b), a * b); - assert_eq!(fdiv_algebraic(a, b), a / b); - assert_eq!(frem_algebraic(a, b), a % b); -} - -#[inline(never)] -pub fn test_operations_f32(a: f32, b: f32) { - // make sure they all map to the correct operation - assert_eq!(fadd_algebraic(a, b), a + b); - assert_eq!(fsub_algebraic(a, b), a - b); - assert_eq!(fmul_algebraic(a, b), a * b); - assert_eq!(fdiv_algebraic(a, b), a / b); - assert_eq!(frem_algebraic(a, b), a % b); -} - -fn main() { - test_operations_f64(1., 2.); - test_operations_f64(10., 5.); - test_operations_f32(11., 2.); - test_operations_f32(10., 15.); -} From 01b5430b28eab188b2df9b09bfdd909eb200a670 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 12 May 2024 10:22:59 +0200 Subject: [PATCH 05/24] merge two integer tests --- src/tools/miri/tests/pass/integer-ops.rs | 61 ++++++++++++++++++++++++ src/tools/miri/tests/pass/integers.rs | 58 ---------------------- 2 files changed, 61 insertions(+), 58 deletions(-) delete mode 100644 src/tools/miri/tests/pass/integers.rs diff --git a/src/tools/miri/tests/pass/integer-ops.rs b/src/tools/miri/tests/pass/integer-ops.rs index 0ec1f8e9c69..3f8ac34e7d1 100644 --- a/src/tools/miri/tests/pass/integer-ops.rs +++ b/src/tools/miri/tests/pass/integer-ops.rs @@ -1,7 +1,64 @@ //@compile-flags: -Coverflow-checks=off #![allow(arithmetic_overflow)] +fn basic() { + fn ret() -> i64 { + 1 + } + + fn neg() -> i64 { + -1 + } + + fn add() -> i64 { + 1 + 2 + } + + fn indirect_add() -> i64 { + let x = 1; + let y = 2; + x + y + } + + fn arith() -> i32 { + 3 * 3 + 4 * 4 + } + + fn match_int() -> i16 { + let n = 2; + match n { + 0 => 0, + 1 => 10, + 2 => 20, + 3 => 30, + _ => 100, + } + } + + fn match_int_range() -> i64 { + let n = 42; + match n { + 0..=9 => 0, + 10..=19 => 1, + 20..=29 => 2, + 30..=39 => 3, + 40..=42 => 4, + _ => 5, + } + } + + assert_eq!(ret(), 1); + assert_eq!(neg(), -1); + assert_eq!(add(), 3); + assert_eq!(indirect_add(), 3); + assert_eq!(arith(), 5 * 5); + assert_eq!(match_int(), 20); + assert_eq!(match_int_range(), 4); +} + pub fn main() { + basic(); + // This tests that we do (not) do sign extension properly when loading integers assert_eq!(u32::MAX as i64, 4294967295); assert_eq!(i32::MIN as i64, -2147483648); @@ -152,6 +209,10 @@ pub fn main() { assert_eq!(5i32.overflowing_mul(2), (10, false)); assert_eq!(1_000_000_000i32.overflowing_mul(10), (1410065408, true)); + assert_eq!(i64::MIN.overflowing_mul(-1), (i64::MIN, true)); + assert_eq!(i32::MIN.overflowing_mul(-1), (i32::MIN, true)); + assert_eq!(i16::MIN.overflowing_mul(-1), (i16::MIN, true)); + assert_eq!(i8::MIN.overflowing_mul(-1), (i8::MIN, true)); assert_eq!(5i32.overflowing_div(2), (2, false)); assert_eq!(i32::MIN.overflowing_div(-1), (i32::MIN, true)); diff --git a/src/tools/miri/tests/pass/integers.rs b/src/tools/miri/tests/pass/integers.rs deleted file mode 100644 index c04c6921f3c..00000000000 --- a/src/tools/miri/tests/pass/integers.rs +++ /dev/null @@ -1,58 +0,0 @@ -fn ret() -> i64 { - 1 -} - -fn neg() -> i64 { - -1 -} - -fn add() -> i64 { - 1 + 2 -} - -fn indirect_add() -> i64 { - let x = 1; - let y = 2; - x + y -} - -fn arith() -> i32 { - 3 * 3 + 4 * 4 -} - -fn match_int() -> i16 { - let n = 2; - match n { - 0 => 0, - 1 => 10, - 2 => 20, - 3 => 30, - _ => 100, - } -} - -fn match_int_range() -> i64 { - let n = 42; - match n { - 0..=9 => 0, - 10..=19 => 1, - 20..=29 => 2, - 30..=39 => 3, - 40..=42 => 4, - _ => 5, - } -} - -fn main() { - assert_eq!(ret(), 1); - assert_eq!(neg(), -1); - assert_eq!(add(), 3); - assert_eq!(indirect_add(), 3); - assert_eq!(arith(), 5 * 5); - assert_eq!(match_int(), 20); - assert_eq!(match_int_range(), 4); - assert_eq!(i64::MIN.overflowing_mul(-1), (i64::MIN, true)); - assert_eq!(i32::MIN.overflowing_mul(-1), (i32::MIN, true)); - assert_eq!(i16::MIN.overflowing_mul(-1), (i16::MIN, true)); - assert_eq!(i8::MIN.overflowing_mul(-1), (i8::MIN, true)); -} From 7d565dfa0fa40991df9e3567f991284b4912ab0b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 12 May 2024 15:44:17 +0200 Subject: [PATCH 06/24] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 3636c856d0b..603417b77ee 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -ef15976387ad9c1cdceaabf469e0cf35f5852f6d +b71fa82d786ae1b5866510f1b3a7e5b7e1890e4c From cd7527aa35f3eca2fa00a6d21cdeb9c56de1f480 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 11 May 2024 15:24:03 -0400 Subject: [PATCH 07/24] Don't print unnecessary sysroot messages --- src/tools/miri/cargo-miri/Cargo.lock | 4 +-- src/tools/miri/cargo-miri/Cargo.toml | 2 +- src/tools/miri/cargo-miri/src/setup.rs | 50 ++++++++++++++++---------- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/tools/miri/cargo-miri/Cargo.lock b/src/tools/miri/cargo-miri/Cargo.lock index b8ead460249..7d965dce07d 100644 --- a/src/tools/miri/cargo-miri/Cargo.lock +++ b/src/tools/miri/cargo-miri/Cargo.lock @@ -178,9 +178,9 @@ dependencies = [ [[package]] name = "rustc-build-sysroot" -version = "0.4.7" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab1dbbd1bdf65fdac44c885f6cca147ba179108ce284b60a08ccc04b1f1dbac0" +checksum = "de6077473f0c46779b49e4587a81f1b8919e0ec26630409ecfda0ba3259efb43" dependencies = [ "anyhow", "rustc_version", diff --git a/src/tools/miri/cargo-miri/Cargo.toml b/src/tools/miri/cargo-miri/Cargo.toml index b16068b6d19..a68854de625 100644 --- a/src/tools/miri/cargo-miri/Cargo.toml +++ b/src/tools/miri/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.6" +rustc-build-sysroot = "0.5.0" # Enable some feature flags that dev-dependencies need but dependencies # do not. This makes `./miri install` after `./miri build` faster. diff --git a/src/tools/miri/cargo-miri/src/setup.rs b/src/tools/miri/cargo-miri/src/setup.rs index 9a58e6fa018..508d3045365 100644 --- a/src/tools/miri/cargo-miri/src/setup.rs +++ b/src/tools/miri/cargo-miri/src/setup.rs @@ -6,7 +6,7 @@ use std::path::PathBuf; use std::process::{self, Command}; -use rustc_build_sysroot::{BuildMode, SysrootBuilder, SysrootConfig}; +use rustc_build_sysroot::{BuildMode, SysrootBuilder, SysrootConfig, SysrootStatus}; use rustc_version::VersionMeta; use crate::util::*; @@ -137,32 +137,52 @@ pub fn setup( // not apply `RUSTFLAGS` to the sysroot either. let rustflags = &["-Cdebug-assertions=off", "-Coverflow-checks=on"]; - // Do the build. - if print_sysroot || quiet { - // Be silent. - } else { + let notify = || { let mut msg = String::new(); write!(msg, "Preparing a sysroot for Miri (target: {target})").unwrap(); if verbose > 0 { write!(msg, " in {}", sysroot_dir.display()).unwrap(); } write!(msg, "...").unwrap(); - if only_setup { + + if print_sysroot || quiet { + // Be silent. + } else if only_setup { // We want to be explicit. eprintln!("{msg}"); } else { // We want to be quiet, but still let the user know that something is happening. eprint!("{msg} "); } - } - SysrootBuilder::new(&sysroot_dir, target) + }; + + // Do the build. + let status = SysrootBuilder::new(&sysroot_dir, target) .build_mode(BuildMode::Check) .rustc_version(rustc_version.clone()) .sysroot_config(sysroot_config) .rustflags(rustflags) .cargo(cargo_cmd) - .build_from_source(&rust_src) - .unwrap_or_else(|err| { + .when_build_required(notify) + .build_from_source(&rust_src); + match status { + Ok(SysrootStatus::AlreadyCached) => + if only_setup && !(print_sysroot || quiet) { + eprintln!( + "A sysroot for Miri is already available in `{}`.", + sysroot_dir.display() + ); + }, + Ok(SysrootStatus::SysrootBuilt) => { + if print_sysroot || quiet { + // Be silent. + } else if only_setup { + eprintln!("A sysroot for Miri is now available in `{}`.", sysroot_dir.display()); + } else { + eprintln!("done"); + } + } + Err(err) => if print_sysroot { show_error!("failed to build sysroot") } else if only_setup { @@ -171,15 +191,9 @@ pub fn setup( show_error!( "failed to build sysroot; run `cargo miri setup` to see the error details" ) - } - }); - if print_sysroot || quiet { - // Be silent. - } else if only_setup { - eprintln!("A sysroot for Miri is now available in `{}`.", sysroot_dir.display()); - } else { - eprintln!("done"); + }, } + if print_sysroot { // Print just the sysroot and nothing else to stdout; this way we do not need any escaping. println!("{}", sysroot_dir.display()); From 10acfd9f778a3570e4624f31c788a6427862a1a7 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 5 May 2024 22:50:22 +0000 Subject: [PATCH 08/24] further illumos/solaris support. fixing part of `miri test alloc/hashmap`. --- src/tools/miri/ci/ci.sh | 4 ++-- src/tools/miri/src/shims/extern_static.rs | 4 ++++ src/tools/miri/src/shims/unix/foreign_items.rs | 3 ++- src/tools/miri/tests/fail/environ-gets-deallocated.rs | 7 ++++++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index f5fbb05d896..6292d1033b6 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -148,8 +148,8 @@ case $HOST_TARGET in BASIC="$VERY_BASIC hello hashmap alloc align" # ensures we have the shims for stdout and basic data structures TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus - TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $VERY_BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random - TEST_TARGET=x86_64-pc-solaris run_tests_minimal $VERY_BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random + TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random env + TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random env TEST_TARGET=aarch64-linux-android run_tests_minimal $VERY_BASIC hello panic/panic TEST_TARGET=wasm32-wasi run_tests_minimal $VERY_BASIC wasm TEST_TARGET=wasm32-unknown-unknown run_tests_minimal $VERY_BASIC wasm diff --git a/src/tools/miri/src/shims/extern_static.rs b/src/tools/miri/src/shims/extern_static.rs index c3c7ef7c1fd..12fede39e27 100644 --- a/src/tools/miri/src/shims/extern_static.rs +++ b/src/tools/miri/src/shims/extern_static.rs @@ -76,6 +76,10 @@ pub fn init_extern_statics(this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult< Self::null_ptr_extern_statics(this, &["bsd_signal"])?; Self::weak_symbol_extern_statics(this, &["signal"])?; } + "solaris" | "illumos" => { + let environ = this.machine.env_vars.unix().environ(); + Self::add_extern_static(this, "environ", environ); + } "windows" => { // "_tls_used" // This is some obscure hack that is part of the Windows TLS story. It's a `u8`. diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index b5165548d1b..5434951d9e4 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -714,8 +714,9 @@ fn emulate_foreign_item_inner( this.write_int(super::UID, dest)?; } - "getpwuid_r" + "getpwuid_r" | "__posix_getpwuid_r" if this.frame_in_std() => { + // getpwuid_r is the standard name, __posix_getpwuid_r is used on solarish let [uid, pwd, buf, buflen, result] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; this.check_no_isolation("`getpwuid_r`")?; diff --git a/src/tools/miri/tests/fail/environ-gets-deallocated.rs b/src/tools/miri/tests/fail/environ-gets-deallocated.rs index 6dcf1fdb1c7..08545a72567 100644 --- a/src/tools/miri/tests/fail/environ-gets-deallocated.rs +++ b/src/tools/miri/tests/fail/environ-gets-deallocated.rs @@ -1,6 +1,11 @@ //@ignore-target-windows: Windows does not have a global environ list that the program can access directly -#[cfg(any(target_os = "linux", target_os = "freebsd"))] +#[cfg(any( + target_os = "linux", + target_os = "freebsd", + target_os = "solaris", + target_os = "illumos" +))] fn get_environ() -> *const *const u8 { extern "C" { static mut environ: *const *const u8; From c6a0e2ca4c1b728a39a5aa525d95ef6db4b4ee19 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 13 May 2024 08:21:05 +0200 Subject: [PATCH 09/24] intrinsics: just panic when they get used incorrectly --- src/tools/miri/src/intrinsics/atomic.rs | 66 ++++++++++++------------- src/tools/miri/src/shims/x86/avx2.rs | 2 +- src/tools/miri/src/shims/x86/mod.rs | 4 +- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/tools/miri/src/intrinsics/atomic.rs b/src/tools/miri/src/intrinsics/atomic.rs index 40f6b8a64ef..0c212c45dbd 100644 --- a/src/tools/miri/src/intrinsics/atomic.rs +++ b/src/tools/miri/src/intrinsics/atomic.rs @@ -25,93 +25,93 @@ fn emulate_atomic_intrinsic( let intrinsic_structure: Vec<_> = intrinsic_name.split('_').collect(); - fn read_ord<'tcx>(ord: &str) -> InterpResult<'tcx, AtomicReadOrd> { - Ok(match ord { + fn read_ord(ord: &str) -> AtomicReadOrd { + match ord { "seqcst" => AtomicReadOrd::SeqCst, "acquire" => AtomicReadOrd::Acquire, "relaxed" => AtomicReadOrd::Relaxed, - _ => throw_unsup_format!("unsupported read ordering `{ord}`"), - }) + _ => panic!("invalid read ordering `{ord}`"), + } } - fn write_ord<'tcx>(ord: &str) -> InterpResult<'tcx, AtomicWriteOrd> { - Ok(match ord { + fn write_ord(ord: &str) -> AtomicWriteOrd { + match ord { "seqcst" => AtomicWriteOrd::SeqCst, "release" => AtomicWriteOrd::Release, "relaxed" => AtomicWriteOrd::Relaxed, - _ => throw_unsup_format!("unsupported write ordering `{ord}`"), - }) + _ => panic!("invalid write ordering `{ord}`"), + } } - fn rw_ord<'tcx>(ord: &str) -> InterpResult<'tcx, AtomicRwOrd> { - Ok(match ord { + fn rw_ord(ord: &str) -> AtomicRwOrd { + match ord { "seqcst" => AtomicRwOrd::SeqCst, "acqrel" => AtomicRwOrd::AcqRel, "acquire" => AtomicRwOrd::Acquire, "release" => AtomicRwOrd::Release, "relaxed" => AtomicRwOrd::Relaxed, - _ => throw_unsup_format!("unsupported read-write ordering `{ord}`"), - }) + _ => panic!("invalid read-write ordering `{ord}`"), + } } - fn fence_ord<'tcx>(ord: &str) -> InterpResult<'tcx, AtomicFenceOrd> { - Ok(match ord { + fn fence_ord(ord: &str) -> AtomicFenceOrd { + match ord { "seqcst" => AtomicFenceOrd::SeqCst, "acqrel" => AtomicFenceOrd::AcqRel, "acquire" => AtomicFenceOrd::Acquire, "release" => AtomicFenceOrd::Release, - _ => throw_unsup_format!("unsupported fence ordering `{ord}`"), - }) + _ => panic!("invalid fence ordering `{ord}`"), + } } match &*intrinsic_structure { - ["load", ord] => this.atomic_load(args, dest, read_ord(ord)?)?, - ["store", ord] => this.atomic_store(args, write_ord(ord)?)?, + ["load", ord] => this.atomic_load(args, dest, read_ord(ord))?, + ["store", ord] => this.atomic_store(args, write_ord(ord))?, - ["fence", ord] => this.atomic_fence_intrinsic(args, fence_ord(ord)?)?, - ["singlethreadfence", ord] => this.compiler_fence_intrinsic(args, fence_ord(ord)?)?, + ["fence", ord] => this.atomic_fence_intrinsic(args, fence_ord(ord))?, + ["singlethreadfence", ord] => this.compiler_fence_intrinsic(args, fence_ord(ord))?, - ["xchg", ord] => this.atomic_exchange(args, dest, rw_ord(ord)?)?, + ["xchg", ord] => this.atomic_exchange(args, dest, rw_ord(ord))?, ["cxchg", ord1, ord2] => - this.atomic_compare_exchange(args, dest, rw_ord(ord1)?, read_ord(ord2)?)?, + this.atomic_compare_exchange(args, dest, rw_ord(ord1), read_ord(ord2))?, ["cxchgweak", ord1, ord2] => - this.atomic_compare_exchange_weak(args, dest, rw_ord(ord1)?, read_ord(ord2)?)?, + this.atomic_compare_exchange_weak(args, dest, rw_ord(ord1), read_ord(ord2))?, ["or", ord] => - this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::BitOr, false), rw_ord(ord)?)?, + this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::BitOr, false), rw_ord(ord))?, ["xor", ord] => - this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::BitXor, false), rw_ord(ord)?)?, + this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::BitXor, false), rw_ord(ord))?, ["and", ord] => - this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::BitAnd, false), rw_ord(ord)?)?, + this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::BitAnd, false), rw_ord(ord))?, ["nand", ord] => - this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::BitAnd, true), rw_ord(ord)?)?, + this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::BitAnd, true), rw_ord(ord))?, ["xadd", ord] => - this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::Add, false), rw_ord(ord)?)?, + this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::Add, false), rw_ord(ord))?, ["xsub", ord] => - this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::Sub, false), rw_ord(ord)?)?, + this.atomic_rmw_op(args, dest, AtomicOp::MirOp(BinOp::Sub, false), rw_ord(ord))?, ["min", ord] => { // Later we will use the type to indicate signed vs unsigned, // so make sure it matches the intrinsic name. assert!(matches!(args[1].layout.ty.kind(), ty::Int(_))); - this.atomic_rmw_op(args, dest, AtomicOp::Min, rw_ord(ord)?)?; + this.atomic_rmw_op(args, dest, AtomicOp::Min, rw_ord(ord))?; } ["umin", ord] => { // Later we will use the type to indicate signed vs unsigned, // so make sure it matches the intrinsic name. assert!(matches!(args[1].layout.ty.kind(), ty::Uint(_))); - this.atomic_rmw_op(args, dest, AtomicOp::Min, rw_ord(ord)?)?; + this.atomic_rmw_op(args, dest, AtomicOp::Min, rw_ord(ord))?; } ["max", ord] => { // Later we will use the type to indicate signed vs unsigned, // so make sure it matches the intrinsic name. assert!(matches!(args[1].layout.ty.kind(), ty::Int(_))); - this.atomic_rmw_op(args, dest, AtomicOp::Max, rw_ord(ord)?)?; + this.atomic_rmw_op(args, dest, AtomicOp::Max, rw_ord(ord))?; } ["umax", ord] => { // Later we will use the type to indicate signed vs unsigned, // so make sure it matches the intrinsic name. assert!(matches!(args[1].layout.ty.kind(), ty::Uint(_))); - this.atomic_rmw_op(args, dest, AtomicOp::Max, rw_ord(ord)?)?; + this.atomic_rmw_op(args, dest, AtomicOp::Max, rw_ord(ord))?; } _ => return Ok(EmulateItemResult::NotSupported), diff --git a/src/tools/miri/src/shims/x86/avx2.rs b/src/tools/miri/src/shims/x86/avx2.rs index bbde5b49588..adecf7b8924 100644 --- a/src/tools/miri/src/shims/x86/avx2.rs +++ b/src/tools/miri/src/shims/x86/avx2.rs @@ -81,7 +81,7 @@ fn emulate_x86_avx2_intrinsic( let scale = this.read_scalar(scale)?.to_i8()?; if !matches!(scale, 1 | 2 | 4 | 8) { - throw_unsup_format!("invalid gather scale {scale}"); + panic!("invalid gather scale {scale}"); } let scale = i64::from(scale); diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index e519fa5508d..58d6db1886a 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/src/shims/x86/mod.rs @@ -200,7 +200,7 @@ fn cmp_from_imm<'tcx>( ) -> InterpResult<'tcx, Self> { // Only bits 0..=4 are used, remaining should be zero. if imm & !0b1_1111 != 0 { - throw_unsup_format!("invalid `imm` parameter of {intrinsic}: 0x{imm:x}"); + panic!("invalid `imm` parameter of {intrinsic}: 0x{imm:x}"); } // Bit 4 specifies whether the operation is quiet or signaling, which // we do not care in Miri. @@ -683,7 +683,7 @@ fn rounding_from_imm<'tcx>(rounding: i32) -> InterpResult<'tcx, rustc_apfloat::R // 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}"), + rounding => panic!("invalid rounding mode 0x{rounding:02x}"), } } From 99c6b2e604125f99bd7e66326dcf1f31a81febac Mon Sep 17 00:00:00 2001 From: Luv-Ray Date: Mon, 13 May 2024 16:17:56 +0800 Subject: [PATCH 10/24] Give `FileDescription::{read, write}` access to the `InterpCx` --- src/tools/miri/src/shims/unix/fd.rs | 37 ++++++++++++------- src/tools/miri/src/shims/unix/fs.rs | 5 +-- .../miri/src/shims/unix/linux/eventfd.rs | 5 +-- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index a53cd607ef0..8159960f826 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -7,7 +7,6 @@ use std::io::{self, ErrorKind, IsTerminal, Read, SeekFrom, Write}; use std::rc::Rc; -use rustc_middle::ty::TyCtxt; use rustc_target::abi::Size; use crate::shims::unix::*; @@ -22,7 +21,7 @@ fn read<'tcx>( &mut self, _communicate_allowed: bool, _bytes: &mut [u8], - _tcx: TyCtxt<'tcx>, + _ecx: &mut MiriInterpCx<'_, 'tcx>, ) -> InterpResult<'tcx, io::Result> { throw_unsup_format!("cannot read from {}", self.name()); } @@ -32,7 +31,7 @@ fn write<'tcx>( &mut self, _communicate_allowed: bool, _bytes: &[u8], - _tcx: TyCtxt<'tcx>, + _ecx: &mut MiriInterpCx<'_, 'tcx>, ) -> InterpResult<'tcx, io::Result> { throw_unsup_format!("cannot write to {}", self.name()); } @@ -82,7 +81,7 @@ fn read<'tcx>( &mut self, communicate_allowed: bool, bytes: &mut [u8], - _tcx: TyCtxt<'tcx>, + _ecx: &mut MiriInterpCx<'_, 'tcx>, ) -> InterpResult<'tcx, io::Result> { if !communicate_allowed { // We want isolation mode to be deterministic, so we have to disallow all reads, even stdin. @@ -105,7 +104,7 @@ fn write<'tcx>( &mut self, _communicate_allowed: bool, bytes: &[u8], - _tcx: TyCtxt<'tcx>, + _ecx: &mut MiriInterpCx<'_, 'tcx>, ) -> InterpResult<'tcx, io::Result> { // We allow writing to stderr even with isolation enabled. let result = Write::write(self, bytes); @@ -133,7 +132,7 @@ fn write<'tcx>( &mut self, _communicate_allowed: bool, bytes: &[u8], - _tcx: TyCtxt<'tcx>, + _ecx: &mut MiriInterpCx<'_, 'tcx>, ) -> InterpResult<'tcx, io::Result> { // We allow writing to stderr even with isolation enabled. // No need to flush, stderr is not buffered. @@ -158,7 +157,7 @@ fn write<'tcx>( &mut self, _communicate_allowed: bool, bytes: &[u8], - _tcx: TyCtxt<'tcx>, + _ecx: &mut MiriInterpCx<'_, 'tcx>, ) -> InterpResult<'tcx, io::Result> { // We just don't write anything, but report to the user that we did. Ok(Ok(bytes.len())) @@ -173,6 +172,14 @@ pub fn new(fd: T) -> Self { FileDescriptor(Rc::new(RefCell::new(Box::new(fd)))) } + pub fn borrow(&self) -> Ref<'_, dyn FileDescription> { + Ref::map(self.0.borrow(), |fd| fd.as_ref()) + } + + pub fn borrow_mut(&self) -> RefMut<'_, dyn FileDescription> { + RefMut::map(self.0.borrow_mut(), |fd| fd.as_mut()) + } + pub fn close<'ctx>(self, communicate_allowed: bool) -> InterpResult<'ctx, io::Result<()>> { // Destroy this `Rc` using `into_inner` so we can call `close` instead of // implicitly running the destructor of the file description. @@ -242,12 +249,12 @@ pub fn insert_fd_with_min_fd(&mut self, file_handle: FileDescriptor, min_fd: i32 pub fn get(&self, fd: i32) -> Option> { let fd = self.fds.get(&fd)?; - Some(Ref::map(fd.0.borrow(), |fd| fd.as_ref())) + Some(fd.borrow()) } pub fn get_mut(&self, fd: i32) -> Option> { let fd = self.fds.get(&fd)?; - Some(RefMut::map(fd.0.borrow_mut(), |fd| fd.as_mut())) + Some(fd.borrow_mut()) } pub fn dup(&self, fd: i32) -> Option { @@ -370,7 +377,8 @@ fn read( .min(u64::try_from(isize::MAX).unwrap()); let communicate = this.machine.communicate(); - let Some(mut file_descriptor) = this.machine.fds.get_mut(fd) else { + // We temporarily dup the FD to be able to retain mutable access to `this`. + let Some(file_descriptor) = this.machine.fds.dup(fd) else { trace!("read: FD not found"); return this.fd_not_found(); }; @@ -383,7 +391,8 @@ fn read( // `File::read` never returns a value larger than `count`, // so this cannot fail. let result = file_descriptor - .read(communicate, &mut bytes, *this.tcx)? + .borrow_mut() + .read(communicate, &mut bytes, this)? .map(|c| i64::try_from(c).unwrap()); drop(file_descriptor); @@ -421,12 +430,14 @@ fn write( let communicate = this.machine.communicate(); let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?.to_owned(); - let Some(mut file_descriptor) = this.machine.fds.get_mut(fd) else { + // We temporarily dup the FD to be able to retain mutable access to `this`. + let Some(file_descriptor) = this.machine.fds.dup(fd) else { return this.fd_not_found(); }; let result = file_descriptor - .write(communicate, &bytes, *this.tcx)? + .borrow_mut() + .write(communicate, &bytes, this)? .map(|c| i64::try_from(c).unwrap()); drop(file_descriptor); diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index eb241556a56..82bc49536b0 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -9,7 +9,6 @@ use std::time::SystemTime; use rustc_data_structures::fx::FxHashMap; -use rustc_middle::ty::TyCtxt; use rustc_target::abi::Size; use crate::shims::os_str::bytes_to_os_str; @@ -34,7 +33,7 @@ fn read<'tcx>( &mut self, communicate_allowed: bool, bytes: &mut [u8], - _tcx: TyCtxt<'tcx>, + _ecx: &mut MiriInterpCx<'_, 'tcx>, ) -> InterpResult<'tcx, io::Result> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); Ok(self.file.read(bytes)) @@ -44,7 +43,7 @@ fn write<'tcx>( &mut self, communicate_allowed: bool, bytes: &[u8], - _tcx: TyCtxt<'tcx>, + _ecx: &mut MiriInterpCx<'_, 'tcx>, ) -> InterpResult<'tcx, io::Result> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); Ok(self.file.write(bytes)) diff --git a/src/tools/miri/src/shims/unix/linux/eventfd.rs b/src/tools/miri/src/shims/unix/linux/eventfd.rs index a865f2efff9..f31c2bb84ae 100644 --- a/src/tools/miri/src/shims/unix/linux/eventfd.rs +++ b/src/tools/miri/src/shims/unix/linux/eventfd.rs @@ -2,7 +2,6 @@ //! Currently just a stub. use std::io; -use rustc_middle::ty::TyCtxt; use rustc_target::abi::Endian; use crate::shims::unix::*; @@ -52,11 +51,11 @@ fn write<'tcx>( &mut self, _communicate_allowed: bool, bytes: &[u8], - tcx: TyCtxt<'tcx>, + ecx: &mut MiriInterpCx<'_, 'tcx>, ) -> InterpResult<'tcx, io::Result> { let bytes: [u8; 8] = bytes.try_into().unwrap(); // FIXME fail gracefully when this has the wrong size // Convert from target endianness to host endianness. - let num = match tcx.sess.target.endian { + let num = match ecx.tcx.sess.target.endian { Endian::Little => u64::from_le_bytes(bytes), Endian::Big => u64::from_be_bytes(bytes), }; From b1b278b17bd75d5336141366ca4e57a8d64993f5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 16 May 2024 10:40:05 +0200 Subject: [PATCH 11/24] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 603417b77ee..fad70bd6885 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -b71fa82d786ae1b5866510f1b3a7e5b7e1890e4c +b71e8cbaf2c7cae4d36898fff1d0ba19d9233082 From 6d314f3b11f50db58974746c1c68234c26259ca8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 16 May 2024 10:43:38 +0200 Subject: [PATCH 12/24] alloc now works on wasi (and some formatting) --- src/tools/miri/ci/ci.sh | 15 ++++---- src/tools/miri/src/machine.rs | 15 ++++---- src/tools/miri/src/shims/alloc.rs | 26 ++++++++++++++ src/tools/miri/src/shims/foreign_items.rs | 5 +++ src/tools/miri/src/shims/mod.rs | 11 +++--- .../miri/src/shims/unix/foreign_items.rs | 22 ++---------- .../miri/src/shims/wasi/foreign_items.rs | 34 +++++++++++++++++++ src/tools/miri/src/shims/wasi/mod.rs | 1 + .../miri/tests/pass-dep/libc/libc-mem.rs | 6 ++-- src/tools/miri/tests/pass/empty_main.rs | 3 ++ 10 files changed, 97 insertions(+), 41 deletions(-) create mode 100644 src/tools/miri/src/shims/wasi/foreign_items.rs create mode 100644 src/tools/miri/src/shims/wasi/mod.rs create mode 100644 src/tools/miri/tests/pass/empty_main.rs diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 6292d1033b6..2a6ca8f4783 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -144,16 +144,15 @@ case $HOST_TARGET in TEST_TARGET=arm-unknown-linux-gnueabi run_tests TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice # Partially supported targets (tier 2) - VERY_BASIC="integer vec string btreemap" # common things we test on all of them (if they have std), requires no target-specific shims - BASIC="$VERY_BASIC hello hashmap alloc align" # ensures we have the shims for stdout and basic data structures + BASIC="empty_main integer vec string btreemap hello hashmap heap_alloc align" # ensures we have the basics: stdout/stderr, system allocator, randomness (for HashMap initialization) TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus - TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random env - TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random env - TEST_TARGET=aarch64-linux-android run_tests_minimal $VERY_BASIC hello panic/panic - TEST_TARGET=wasm32-wasi run_tests_minimal $VERY_BASIC wasm - TEST_TARGET=wasm32-unknown-unknown run_tests_minimal $VERY_BASIC wasm - TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std + TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random env + TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random env + TEST_TARGET=aarch64-linux-android run_tests_minimal empty_main hello panic/panic + TEST_TARGET=wasm32-wasi run_tests_minimal empty_main wasm heap_alloc libc-mem + TEST_TARGET=wasm32-unknown-unknown run_tests_minimal empty_main wasm + TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std # Custom target JSON file TEST_TARGET=tests/avr.json MIRI_NO_STD=1 run_tests_minimal no_std ;; diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 8854b185280..6fdd126ff24 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -30,14 +30,13 @@ use rustc_target::spec::abi::Abi; use crate::{ - concurrency::{data_race, weak_memory}, - shims::unix, + concurrency::{ + data_race::{self, NaReadType, NaWriteType}, + weak_memory, + }, *, }; -use self::concurrency::data_race::NaReadType; -use self::concurrency::data_race::NaWriteType; - /// First real-time signal. /// `signal(7)` says this must be between 32 and 64 and specifies 34 or 35 /// as typical values. @@ -464,9 +463,9 @@ pub struct MiriMachine<'mir, 'tcx> { pub(crate) validate: bool, /// The table of file descriptors. - pub(crate) fds: unix::FdTable, + pub(crate) fds: shims::FdTable, /// The table of directory descriptors. - pub(crate) dirs: unix::DirTable, + pub(crate) dirs: shims::DirTable, /// This machine's monotone clock. pub(crate) clock: Clock, @@ -641,7 +640,7 @@ pub(crate) fn new(config: &MiriConfig, layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) tls: TlsData::default(), isolated_op: config.isolated_op, validate: config.validate, - fds: unix::FdTable::new(config.mute_stdout_stderr), + fds: shims::FdTable::new(config.mute_stdout_stderr), dirs: Default::default(), layouts, threads: ThreadManager::default(), diff --git a/src/tools/miri/src/shims/alloc.rs b/src/tools/miri/src/shims/alloc.rs index 1deb9a5654e..d0f36bd4757 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/src/shims/alloc.rs @@ -111,6 +111,32 @@ fn malloc( Ok(ptr.into()) } + fn posix_memalign( + &mut self, + memptr: &OpTy<'tcx, Provenance>, + align: &OpTy<'tcx, Provenance>, + size: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + let memptr = this.deref_pointer(memptr)?; + let align = this.read_target_usize(align)?; + let size = this.read_target_usize(size)?; + + // Align must be power of 2, and also at least ptr-sized (POSIX rules). + // But failure to adhere to this is not UB, it's an error condition. + if !align.is_power_of_two() || align < this.pointer_size().bytes() { + Ok(this.eval_libc("EINVAL")) + } else { + let ptr = this.allocate_ptr( + Size::from_bytes(size), + Align::from_bytes(align).unwrap(), + MiriMemoryKind::C.into(), + )?; + this.write_pointer(ptr, &memptr)?; + Ok(Scalar::from_i32(0)) + } + } + fn free(&mut self, ptr: Pointer>) -> InterpResult<'tcx> { let this = self.eval_context_mut(); if !this.ptr_is_null(ptr)? { diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index d431c28d55a..a65da823e24 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -108,6 +108,7 @@ fn is_dyn_sym(&self, name: &str) -> bool { let this = self.eval_context_ref(); match this.tcx.sess.target.os.as_ref() { os if this.target_os_is_unix() => shims::unix::foreign_items::is_dyn_sym(name, os), + "wasi" => shims::wasi::foreign_items::is_dyn_sym(name), "windows" => shims::windows::foreign_items::is_dyn_sym(name), _ => false, } @@ -947,6 +948,10 @@ fn emulate_foreign_item_inner( shims::unix::foreign_items::EvalContextExt::emulate_foreign_item_inner( this, link_name, abi, args, dest, ), + "wasi" => + shims::wasi::foreign_items::EvalContextExt::emulate_foreign_item_inner( + this, link_name, abi, args, dest, + ), "windows" => shims::windows::foreign_items::EvalContextExt::emulate_foreign_item_inner( this, link_name, abi, args, dest, diff --git a/src/tools/miri/src/shims/mod.rs b/src/tools/miri/src/shims/mod.rs index aaa3c69b92d..d9c4a2282c1 100644 --- a/src/tools/miri/src/shims/mod.rs +++ b/src/tools/miri/src/shims/mod.rs @@ -2,20 +2,23 @@ mod alloc; mod backtrace; -pub mod foreign_items; #[cfg(target_os = "linux")] -pub mod native_lib; -pub mod unix; -pub mod windows; +mod native_lib; +mod unix; +mod wasi; +mod windows; mod x86; pub mod env; pub mod extern_static; +pub mod foreign_items; pub mod os_str; pub mod panic; pub mod time; pub mod tls; +pub use unix::{DirTable, FdTable}; + /// What needs to be done after emulating an item (a shim or an intrinsic) is done. pub enum EmulateItemResult { /// The caller is expected to jump to the return block. diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 5434951d9e4..78d297d4b04 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -3,7 +3,6 @@ use rustc_middle::ty::layout::LayoutOf; use rustc_span::Symbol; -use rustc_target::abi::{Align, Size}; use rustc_target::spec::abi::Abi; use crate::shims::alloc::EvalContextExt as _; @@ -249,24 +248,9 @@ fn emulate_foreign_item_inner( // Allocation "posix_memalign" => { - let [ret, align, size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let ret = this.deref_pointer(ret)?; - let align = this.read_target_usize(align)?; - let size = this.read_target_usize(size)?; - // Align must be power of 2, and also at least ptr-sized (POSIX rules). - // But failure to adhere to this is not UB, it's an error condition. - if !align.is_power_of_two() || align < this.pointer_size().bytes() { - let einval = this.eval_libc_i32("EINVAL"); - this.write_int(einval, dest)?; - } else { - let ptr = this.allocate_ptr( - Size::from_bytes(size), - Align::from_bytes(align).unwrap(), - MiriMemoryKind::C.into(), - )?; - this.write_pointer(ptr, &ret)?; - this.write_null(dest)?; - } + let [memptr, align, size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let result = this.posix_memalign(memptr, align, size)?; + this.write_scalar(result, dest)?; } "mmap" => { diff --git a/src/tools/miri/src/shims/wasi/foreign_items.rs b/src/tools/miri/src/shims/wasi/foreign_items.rs new file mode 100644 index 00000000000..12bf0490932 --- /dev/null +++ b/src/tools/miri/src/shims/wasi/foreign_items.rs @@ -0,0 +1,34 @@ +use rustc_span::Symbol; +use rustc_target::spec::abi::Abi; + +use crate::shims::alloc::EvalContextExt as _; +use crate::*; + +pub fn is_dyn_sym(_name: &str) -> bool { + false +} + +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( + &mut self, + link_name: Symbol, + abi: Abi, + args: &[OpTy<'tcx, Provenance>], + dest: &MPlaceTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, EmulateItemResult> { + let this = self.eval_context_mut(); + match link_name.as_str() { + // Allocation + "posix_memalign" => { + let [memptr, align, size] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let result = this.posix_memalign(memptr, align, size)?; + this.write_scalar(result, dest)?; + } + + _ => return Ok(EmulateItemResult::NotSupported), + } + Ok(EmulateItemResult::NeedsJumping) + } +} diff --git a/src/tools/miri/src/shims/wasi/mod.rs b/src/tools/miri/src/shims/wasi/mod.rs new file mode 100644 index 00000000000..09c6507b24f --- /dev/null +++ b/src/tools/miri/src/shims/wasi/mod.rs @@ -0,0 +1 @@ +pub mod foreign_items; diff --git a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs index b36fb436b57..5bd205dd085 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs @@ -226,7 +226,8 @@ fn test_memalign() { target_os = "windows", target_os = "macos", target_os = "illumos", - target_os = "solaris" + target_os = "solaris", + target_os = "wasi", )))] fn test_reallocarray() { unsafe { @@ -249,7 +250,8 @@ fn main() { target_os = "windows", target_os = "macos", target_os = "illumos", - target_os = "solaris" + target_os = "solaris", + target_os = "wasi", )))] test_reallocarray(); diff --git a/src/tools/miri/tests/pass/empty_main.rs b/src/tools/miri/tests/pass/empty_main.rs new file mode 100644 index 00000000000..d081b6db670 --- /dev/null +++ b/src/tools/miri/tests/pass/empty_main.rs @@ -0,0 +1,3 @@ +// This may look trivial, but a bunch of code runs in std before +// `main` is called, so we are ensuring that that all works. +fn main() {} From 983fb093ff10a98e8416c10e540f362d50c93f6a Mon Sep 17 00:00:00 2001 From: Strophox Date: Wed, 3 Apr 2024 20:11:44 +0200 Subject: [PATCH 13/24] start implementing MiriAllocBytes attempt changing Bytes in MiriMachine to MiriAllocBytes rename miri_alloc_bytes to alloc_bytes generalize impl VisitProvenance for Allocation for any Bytes: AllocBytes mend MiriAllocBytes -> Self::Bytes fix Invariant documentation and bugs (drop), impl Clone Update MiriAllocBytes description Co-authored-by: Ralf Jung Rephrase MiriAllocBytes ptr invariant Co-authored-by: Ralf Jung Update MiriAllocBytes ptr documentation Co-authored-by: Ralf Jung fix safety comment in MiriAllocBytes::clone fix safety comment in MiriAllocBytes::from_bytes try implementing clone without unsafe remove derive(PartialEq,Eq,Hash), fix fmt move ptr.is_null() check inside only necessary branch use std::ptr::without_provenance_mut, requiring feature(strict_provenance) align.bytes_usize() instead of align.bytes().try_into().unwrap() Update src/provenance_gc.rs Co-authored-by: Ralf Jung fix clippy error on deref --- src/tools/miri/src/alloc_bytes.rs | 109 ++++++++++++++++++++++++++++ src/tools/miri/src/diagnostics.rs | 2 +- src/tools/miri/src/lib.rs | 3 + src/tools/miri/src/machine.rs | 9 ++- src/tools/miri/src/provenance_gc.rs | 2 +- 5 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 src/tools/miri/src/alloc_bytes.rs diff --git a/src/tools/miri/src/alloc_bytes.rs b/src/tools/miri/src/alloc_bytes.rs new file mode 100644 index 00000000000..7952abdf9f4 --- /dev/null +++ b/src/tools/miri/src/alloc_bytes.rs @@ -0,0 +1,109 @@ +use std::alloc; +use std::alloc::Layout; +use std::borrow::Cow; +use std::slice; + +use rustc_middle::mir::interpret::AllocBytes; +use rustc_target::abi::{Align, Size}; + +/// Allocation bytes that explicitly handle the layout of the data they're storing. +/// This is necessary to interface with native code that accesses the program store in Miri. +#[derive(Debug)] +pub struct MiriAllocBytes { + /// Stored layout information about the allocation. + layout: alloc::Layout, + /// Pointer to the allocation contents. + /// Invariant: + /// * If `self.layout.size() == 0`, then `self.ptr` is some suitably aligned pointer + /// without provenance (and no actual memory was allocated). + /// * Otherwise, `self.ptr` points to memory allocated with `self.layout`. + ptr: *mut u8, +} + +impl Clone for MiriAllocBytes { + fn clone(&self) -> Self { + let bytes: Cow<'_, [u8]> = Cow::Borrowed(self); + let align = Align::from_bytes(self.layout.align().try_into().unwrap()).unwrap(); + MiriAllocBytes::from_bytes(bytes, align) + } +} + +impl Drop for MiriAllocBytes { + fn drop(&mut self) { + if self.layout.size() != 0 { + // SAFETY: Invariant, `self.ptr` points to memory allocated with `self.layout`. + unsafe { alloc::dealloc(self.ptr, self.layout) } + } + } +} + +impl std::ops::Deref for MiriAllocBytes { + type Target = [u8]; + + fn deref(&self) -> &Self::Target { + // SAFETY: `ptr` is non-null, properly aligned, and valid for reading out `self.layout.size()`-many bytes. + // Note that due to the invariant this is true even if `self.layout.size() == 0`. + unsafe { slice::from_raw_parts(self.ptr, self.layout.size()) } + } +} + +impl std::ops::DerefMut for MiriAllocBytes { + fn deref_mut(&mut self) -> &mut Self::Target { + // SAFETY: `ptr` is non-null, properly aligned, and valid for reading out `self.layout.size()`-many bytes. + // Note that due to the invariant this is true even if `self.layout.size() == 0`. + unsafe { slice::from_raw_parts_mut(self.ptr, self.layout.size()) } + } +} + +impl MiriAllocBytes { + /// This method factors out how a `MiriAllocBytes` object is allocated, + /// specifically given an allocation function `alloc_fn`. + /// `alloc_fn` is only used if `size != 0`. + /// Returns `Err(layout)` if the allocation function returns a `ptr` that is `ptr.is_null()`. + fn alloc_with( + size: usize, + align: usize, + alloc_fn: impl FnOnce(Layout) -> *mut u8, + ) -> Result { + let layout = Layout::from_size_align(size, align).unwrap(); + let ptr = if size == 0 { + std::ptr::without_provenance_mut(align) + } else { + let ptr = alloc_fn(layout); + if ptr.is_null() { + return Err(layout); + } + ptr + }; + // SAFETY: All `MiriAllocBytes` invariants are fulfilled. + Ok(Self { ptr, layout }) + } +} + +impl AllocBytes for MiriAllocBytes { + fn from_bytes<'a>(slice: impl Into>, align: Align) -> Self { + let slice = slice.into(); + let size = slice.len(); + let align = align.bytes_usize(); + // SAFETY: `alloc_fn` will only be used if `size != 0`. + let alloc_fn = |layout| unsafe { alloc::alloc(layout) }; + let alloc_bytes = MiriAllocBytes::alloc_with(size, align, alloc_fn) + .unwrap_or_else(|layout| alloc::handle_alloc_error(layout)); + // SAFETY: `alloc_bytes.ptr` and `slice.as_ptr()` are non-null, properly aligned + // and valid for the `size`-many bytes to be copied. + unsafe { alloc_bytes.ptr.copy_from(slice.as_ptr(), size) }; + alloc_bytes + } + + fn zeroed(size: Size, align: Align) -> Option { + let size = size.bytes_usize(); + let align = align.bytes_usize(); + // SAFETY: `alloc_fn` will only be used if `size != 0`. + let alloc_fn = |layout| unsafe { alloc::alloc_zeroed(layout) }; + MiriAllocBytes::alloc_with(size, align, alloc_fn).ok() + } + + fn as_mut_ptr(&mut self) -> *mut u8 { + self.ptr + } +} diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index cb5ed27b6cf..189c4a20bf6 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -459,7 +459,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, MiriAllocBytes>)>, ) { let mut any_pruned = false; for (id, kind, mut alloc) in leaks { diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 54eb6a3bd66..e4879f2f531 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -13,6 +13,7 @@ #![feature(lint_reasons)] #![feature(trait_upcasting)] #![feature(strict_overflow_ops)] +#![feature(strict_provenance)] // Configure clippy and other lints #![allow( clippy::collapsible_else_if, @@ -74,6 +75,7 @@ extern crate rustc_driver; mod alloc_addresses; +mod alloc_bytes; mod borrow_tracker; mod clock; mod concurrency; @@ -107,6 +109,7 @@ pub use crate::shims::EmulateItemResult; pub use crate::alloc_addresses::{EvalContextExt as _, ProvenanceMode}; +pub use crate::alloc_bytes::MiriAllocBytes; pub use crate::borrow_tracker::stacked_borrows::{ EvalContextExt as _, Item, Permission, Stack, Stacks, }; diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 8854b185280..1309bfccb3a 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -862,7 +862,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { type Provenance = Provenance; type ProvenanceExtra = ProvenanceExtra; - type Bytes = Box<[u8]>; + type Bytes = MiriAllocBytes; type MemoryMap = MonoHashMap)>; @@ -1088,8 +1088,9 @@ fn adjust_allocation<'b>( id: AllocId, alloc: Cow<'b, Allocation>, kind: Option, - ) -> InterpResult<'tcx, Cow<'b, Allocation>> { - let kind = kind.expect("we set our GLOBAL_KIND so this cannot be None"); + ) -> 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) { ecx.emit_diagnostic(NonHaltingDiagnostic::CreatedAlloc( id, @@ -1126,7 +1127,7 @@ fn adjust_allocation<'b>( Some(ecx.generate_stacktrace()) }; - let alloc: Allocation = alloc.adjust_from_tcx( + let alloc: Allocation = alloc.adjust_from_tcx( &ecx.tcx, AllocExtra { borrow_tracker, diff --git a/src/tools/miri/src/provenance_gc.rs b/src/tools/miri/src/provenance_gc.rs index f23d7dfd52d..ecd614bf467 100644 --- a/src/tools/miri/src/provenance_gc.rs +++ b/src/tools/miri/src/provenance_gc.rs @@ -122,7 +122,7 @@ fn visit_provenance(&self, visit: &mut VisitWith<'_>) { } } -impl VisitProvenance for Allocation> { +impl VisitProvenance for Allocation, MiriAllocBytes> { fn visit_provenance(&self, visit: &mut VisitWith<'_>) { for prov in self.provenance().provenances() { prov.visit_provenance(visit); From 318a0fe5865e9f0713ae3b739403fd361c42a299 Mon Sep 17 00:00:00 2001 From: marc0246 <40955683+marc0246@users.noreply.github.com> Date: Fri, 17 May 2024 22:58:25 +0200 Subject: [PATCH 14/24] Ignore the Helix configuration directory --- src/tools/miri/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/miri/.gitignore b/src/tools/miri/.gitignore index 97e006e8b1b..03c5591b787 100644 --- a/src/tools/miri/.gitignore +++ b/src/tools/miri/.gitignore @@ -5,6 +5,7 @@ tex/*/out *.out *.rs.bk .vscode +.helix *.mm_profdata perf.data perf.data.old From 5ea21ca486794f1f1ae038d420bdaee92ff5be5d Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 7 May 2024 23:11:48 +0000 Subject: [PATCH 15/24] support aligned_alloc for unixes support. --- src/tools/miri/src/shims/alloc.rs | 38 ++++++++++++++++++ .../miri/src/shims/unix/foreign_items.rs | 11 +++++ .../miri/src/shims/wasi/foreign_items.rs | 9 +++++ .../libc/aligned_alloc_size_zero_leak.rs | 15 +++++++ .../libc/aligned_alloc_size_zero_leak.stderr | 15 +++++++ .../miri/tests/pass-dep/libc/libc-mem.rs | 40 +++++++++++++++++++ 6 files changed, 128 insertions(+) create mode 100644 src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.rs create mode 100644 src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr diff --git a/src/tools/miri/src/shims/alloc.rs b/src/tools/miri/src/shims/alloc.rs index d0f36bd4757..6c18989caad 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/src/shims/alloc.rs @@ -172,4 +172,42 @@ fn realloc( } } } + + fn aligned_alloc( + &mut self, + align: u64, + size: u64, + ) -> InterpResult<'tcx, Pointer>> { + let this = self.eval_context_mut(); + // Alignment must be a power of 2, and "supported by the implementation". + // We decide that "supported by the implementation" means that the + // size must be a multiple of the alignment. (This restriction seems common + // enough that it is stated on + // as a general rule, but the actual standard has no such rule.) + // If any of these are violated, we have to return NULL. + // All fundamental alignments must be supported. + // + // macOS and Illumos are buggy in that they require the alignment + // to be at least the size of a pointer, so they do not support all fundamental + // alignments. We do not emulate those platform bugs. + // + // Linux also sets errno to EINVAL, but that's non-standard behavior that we do not + // emulate. + // FreeBSD says some of these cases are UB but that's violating the C standard. + // http://en.cppreference.com/w/cpp/memory/c/aligned_alloc + // Linux: https://linux.die.net/man/3/aligned_alloc + // FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=aligned_alloc&apropos=0&sektion=3&manpath=FreeBSD+9-current&format=html + match size.checked_rem(align) { + Some(0) if align.is_power_of_two() => { + let align = align.max(this.malloc_align(size).bytes()); + let ptr = this.allocate_ptr( + Size::from_bytes(size), + Align::from_bytes(align).unwrap(), + MiriMemoryKind::C.into(), + )?; + Ok(ptr.into()) + } + _ => Ok(Pointer::null()), + } + } } diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 78d297d4b04..ca59e6e6501 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -295,6 +295,17 @@ fn emulate_foreign_item_inner( } } } + "aligned_alloc" => { + // This is a C11 function, we assume all Unixes have it. + // (MSVC explicitly does not support this.) + let [align, size] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let align = this.read_target_usize(align)?; + let size = this.read_target_usize(size)?; + + let res = this.aligned_alloc(align, size)?; + this.write_pointer(res, dest)?; + } // Dynamic symbol loading "dlsym" => { diff --git a/src/tools/miri/src/shims/wasi/foreign_items.rs b/src/tools/miri/src/shims/wasi/foreign_items.rs index 12bf0490932..9c0a8e66390 100644 --- a/src/tools/miri/src/shims/wasi/foreign_items.rs +++ b/src/tools/miri/src/shims/wasi/foreign_items.rs @@ -26,6 +26,15 @@ fn emulate_foreign_item_inner( let result = this.posix_memalign(memptr, align, size)?; this.write_scalar(result, dest)?; } + "aligned_alloc" => { + let [align, size] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let align = this.read_target_usize(align)?; + let size = this.read_target_usize(size)?; + + let res = this.aligned_alloc(align, size)?; + this.write_pointer(res, dest)?; + } _ => return Ok(EmulateItemResult::NotSupported), } diff --git a/src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.rs b/src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.rs new file mode 100644 index 00000000000..9a33cdccd27 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.rs @@ -0,0 +1,15 @@ +//@ignore-target-windows: Windows does not support the standard C11 aligned_alloc. + +fn main() { + // libc doesn't have this function (https://github.com/rust-lang/libc/issues/3689), + // so we declare it ourselves. + extern "C" { + fn aligned_alloc(alignment: libc::size_t, size: libc::size_t) -> *mut libc::c_void; + } + + // Make sure even zero-sized allocations need to be freed. + + unsafe { + aligned_alloc(2, 0); //~ERROR: memory leaked + } +} diff --git a/src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr b/src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr new file mode 100644 index 00000000000..b0756d57212 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr @@ -0,0 +1,15 @@ +error: memory leaked: ALLOC (C heap, size: 0, align: 2), allocated here: + --> $DIR/aligned_alloc_size_zero_leak.rs:LL:CC + | +LL | aligned_alloc(2, 0); + | ^^^^^^^^^^^^^^^^^^^ + | + = note: BACKTRACE: + = note: inside `main` at $DIR/aligned_alloc_size_zero_leak.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs index 5bd205dd085..25aef0a183a 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs @@ -241,6 +241,44 @@ fn test_reallocarray() { } } +#[cfg(not(target_os = "windows"))] +fn test_aligned_alloc() { + // libc doesn't have this function (https://github.com/rust-lang/libc/issues/3689), + // so we declare it ourselves. + extern "C" { + fn aligned_alloc(alignment: libc::size_t, size: libc::size_t) -> *mut libc::c_void; + } + // size not a multiple of the alignment + unsafe { + let p = aligned_alloc(16, 3); + assert_eq!(p, ptr::null_mut()); + } + + // alignment not power of 2 + unsafe { + let p = aligned_alloc(63, 8); + assert_eq!(p, ptr::null_mut()); + } + + // alignment lesser than a word but still a successful allocation + unsafe { + let p = aligned_alloc(1, 4); + assert!(!p.is_null()); + assert!(p.is_aligned_to(4)); + libc::free(p); + } + + // repeated tests on correct alignment/size + for _ in 0..16 { + unsafe { + let p = aligned_alloc(16, 16); + assert!(!p.is_null()); + assert!(p.is_aligned_to(16)); + libc::free(p); + } + } +} + fn main() { test_malloc(); test_calloc(); @@ -254,6 +292,8 @@ fn main() { target_os = "wasi", )))] test_reallocarray(); + #[cfg(not(target_os = "windows"))] + test_aligned_alloc(); test_memcpy(); test_strcpy(); From 1ba83f2dc11ce0f2249e597bb59839bb8ea8df6b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 19 May 2024 10:35:38 +0200 Subject: [PATCH 16/24] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index fad70bd6885..207ef6c5de7 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -b71e8cbaf2c7cae4d36898fff1d0ba19d9233082 +6579ed89f0fcc26da71afdd11d30d63f6f812a0a From 430298c3adc16e09d9fbd8f295b404fdb682d9eb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 19 May 2024 11:48:51 +0200 Subject: [PATCH 17/24] a bit of refactoring and tweak the aligned-allocation tests --- src/tools/miri/src/shims/alloc.rs | 7 +- .../miri/src/shims/unix/foreign_items.rs | 3 - .../miri/src/shims/wasi/foreign_items.rs | 3 - .../miri/tests/pass-dep/libc/libc-mem.rs | 110 +++++++++--------- 4 files changed, 61 insertions(+), 62 deletions(-) diff --git a/src/tools/miri/src/shims/alloc.rs b/src/tools/miri/src/shims/alloc.rs index 6c18989caad..bd84de81e69 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/src/shims/alloc.rs @@ -175,10 +175,13 @@ fn realloc( fn aligned_alloc( &mut self, - align: u64, - size: u64, + align: &OpTy<'tcx, Provenance>, + size: &OpTy<'tcx, Provenance>, ) -> InterpResult<'tcx, Pointer>> { let this = self.eval_context_mut(); + let align = this.read_target_usize(align)?; + let size = this.read_target_usize(size)?; + // Alignment must be a power of 2, and "supported by the implementation". // We decide that "supported by the implementation" means that the // size must be a multiple of the alignment. (This restriction seems common diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index ca59e6e6501..74fb2fb4b2b 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -300,9 +300,6 @@ fn emulate_foreign_item_inner( // (MSVC explicitly does not support this.) let [align, size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let align = this.read_target_usize(align)?; - let size = this.read_target_usize(size)?; - let res = this.aligned_alloc(align, size)?; this.write_pointer(res, dest)?; } diff --git a/src/tools/miri/src/shims/wasi/foreign_items.rs b/src/tools/miri/src/shims/wasi/foreign_items.rs index 9c0a8e66390..d911f43eb62 100644 --- a/src/tools/miri/src/shims/wasi/foreign_items.rs +++ b/src/tools/miri/src/shims/wasi/foreign_items.rs @@ -29,9 +29,6 @@ fn emulate_foreign_item_inner( "aligned_alloc" => { let [align, size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let align = this.read_target_usize(align)?; - let size = this.read_target_usize(size)?; - let res = this.aligned_alloc(align, size)?; this.write_pointer(res, dest)?; } diff --git a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs index 25aef0a183a..aa383a99bc2 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs @@ -148,53 +148,55 @@ fn test_calloc() { #[cfg(not(target_os = "windows"))] fn test_memalign() { - // A normal allocation. - unsafe { - let mut ptr: *mut libc::c_void = ptr::null_mut(); - let align = 8; - let size = 64; - assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); - assert!(!ptr.is_null()); - assert!(ptr.is_aligned_to(align)); - ptr.cast::().write_bytes(1, size); - libc::free(ptr); - } + for _ in 0..16 { + // A normal allocation. + unsafe { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 8; + let size = 64; + assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); + assert!(!ptr.is_null()); + assert!(ptr.is_aligned_to(align)); + ptr.cast::().write_bytes(1, size); + libc::free(ptr); + } - // Align > size. - unsafe { - let mut ptr: *mut libc::c_void = ptr::null_mut(); - let align = 64; - let size = 8; - assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); - assert!(!ptr.is_null()); - assert!(ptr.is_aligned_to(align)); - ptr.cast::().write_bytes(1, size); - libc::free(ptr); - } + // Align > size. + unsafe { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 64; + let size = 8; + assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); + assert!(!ptr.is_null()); + assert!(ptr.is_aligned_to(align)); + ptr.cast::().write_bytes(1, size); + libc::free(ptr); + } - // Size not multiple of align - unsafe { - let mut ptr: *mut libc::c_void = ptr::null_mut(); - let align = 16; - let size = 31; - assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); - assert!(!ptr.is_null()); - assert!(ptr.is_aligned_to(align)); - ptr.cast::().write_bytes(1, size); - libc::free(ptr); - } + // Size not multiple of align + unsafe { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 16; + let size = 31; + assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); + assert!(!ptr.is_null()); + assert!(ptr.is_aligned_to(align)); + ptr.cast::().write_bytes(1, size); + libc::free(ptr); + } - // Size == 0 - unsafe { - let mut ptr: *mut libc::c_void = ptr::null_mut(); - let align = 64; - let size = 0; - assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); - // Non-null pointer is returned if size == 0. - // (This is not a guarantee, it just reflects our current behavior.) - assert!(!ptr.is_null()); - assert!(ptr.is_aligned_to(align)); - libc::free(ptr); + // Size == 0 + unsafe { + let mut ptr: *mut libc::c_void = ptr::null_mut(); + let align = 64; + let size = 0; + assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0); + // Non-null pointer is returned if size == 0. + // (This is not a guarantee, it just reflects our current behavior.) + assert!(!ptr.is_null()); + assert!(ptr.is_aligned_to(align)); + libc::free(ptr); + } } // Non-power of 2 align @@ -260,20 +262,20 @@ fn test_aligned_alloc() { assert_eq!(p, ptr::null_mut()); } - // alignment lesser than a word but still a successful allocation - unsafe { - let p = aligned_alloc(1, 4); - assert!(!p.is_null()); - assert!(p.is_aligned_to(4)); - libc::free(p); - } - // repeated tests on correct alignment/size for _ in 0..16 { + // alignment 1, size 4 should succeed and actually must align to 4 (because C says so...) unsafe { - let p = aligned_alloc(16, 16); + let p = aligned_alloc(1, 4); assert!(!p.is_null()); - assert!(p.is_aligned_to(16)); + assert!(p.is_aligned_to(4)); + libc::free(p); + } + + unsafe { + let p = aligned_alloc(64, 64); + assert!(!p.is_null()); + assert!(p.is_aligned_to(64)); libc::free(p); } } From 844de64396bfa0d545513f35ef6091fe63ce03e0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 19 May 2024 11:12:09 +0200 Subject: [PATCH 18/24] make basic things work on Android --- src/tools/miri/ci/ci.sh | 11 ++++--- src/tools/miri/src/shims/extern_static.rs | 18 ++++------- .../src/shims/unix/android/foreign_items.rs | 32 +++++++++++++++++++ src/tools/miri/src/shims/unix/android/mod.rs | 1 + .../miri/src/shims/unix/foreign_items.rs | 11 ++++--- src/tools/miri/src/shims/unix/mod.rs | 1 + .../tests/fail/environ-gets-deallocated.rs | 14 -------- .../miri/tests/pass-dep/libc/libc-misc.rs | 20 +++++++++++- src/tools/miri/tests/pass/shims/env/home.rs | 8 +++-- 9 files changed, 79 insertions(+), 37 deletions(-) create mode 100644 src/tools/miri/src/shims/unix/android/foreign_items.rs create mode 100644 src/tools/miri/src/shims/unix/android/mod.rs diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 2a6ca8f4783..50ac4539c26 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -145,11 +145,12 @@ case $HOST_TARGET in TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice # Partially supported targets (tier 2) BASIC="empty_main integer vec string btreemap hello hashmap heap_alloc align" # ensures we have the basics: stdout/stderr, system allocator, randomness (for HashMap initialization) - TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus - TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus - TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random env - TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random env - TEST_TARGET=aarch64-linux-android run_tests_minimal empty_main hello panic/panic + UNIX="panic/panic concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there + TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs + TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs + TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX pthread-sync + TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC $UNIX pthread-sync + TEST_TARGET=aarch64-linux-android run_tests_minimal $BASIC $UNIX TEST_TARGET=wasm32-wasi run_tests_minimal empty_main wasm heap_alloc libc-mem TEST_TARGET=wasm32-unknown-unknown run_tests_minimal empty_main wasm TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std diff --git a/src/tools/miri/src/shims/extern_static.rs b/src/tools/miri/src/shims/extern_static.rs index 12fede39e27..b9817a18773 100644 --- a/src/tools/miri/src/shims/extern_static.rs +++ b/src/tools/miri/src/shims/extern_static.rs @@ -55,6 +55,12 @@ pub fn init_extern_statics(this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult< let val = ImmTy::from_int(val, this.machine.layouts.u8); Self::alloc_extern_static(this, "__rust_alloc_error_handler_should_panic", val)?; + if this.target_os_is_unix() { + // "environ" is mandated by POSIX. + let environ = this.machine.env_vars.unix().environ(); + Self::add_extern_static(this, "environ", environ); + } + match this.tcx.sess.target.os.as_ref() { "linux" => { Self::null_ptr_extern_statics( @@ -62,23 +68,13 @@ pub fn init_extern_statics(this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult< &["__cxa_thread_atexit_impl", "__clock_gettime64"], )?; Self::weak_symbol_extern_statics(this, &["getrandom", "statx"])?; - // "environ" - let environ = this.machine.env_vars.unix().environ(); - Self::add_extern_static(this, "environ", environ); } "freebsd" => { Self::null_ptr_extern_statics(this, &["__cxa_thread_atexit_impl"])?; - // "environ" - let environ = this.machine.env_vars.unix().environ(); - Self::add_extern_static(this, "environ", environ); } "android" => { Self::null_ptr_extern_statics(this, &["bsd_signal"])?; - Self::weak_symbol_extern_statics(this, &["signal"])?; - } - "solaris" | "illumos" => { - let environ = this.machine.env_vars.unix().environ(); - Self::add_extern_static(this, "environ", environ); + Self::weak_symbol_extern_statics(this, &["signal", "getrandom"])?; } "windows" => { // "_tls_used" diff --git a/src/tools/miri/src/shims/unix/android/foreign_items.rs b/src/tools/miri/src/shims/unix/android/foreign_items.rs new file mode 100644 index 00000000000..f85b8a725f2 --- /dev/null +++ b/src/tools/miri/src/shims/unix/android/foreign_items.rs @@ -0,0 +1,32 @@ +use rustc_span::Symbol; +use rustc_target::spec::abi::Abi; + +use crate::*; + +pub fn is_dyn_sym(_name: &str) -> bool { + false +} + +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( + &mut self, + link_name: Symbol, + abi: Abi, + args: &[OpTy<'tcx, Provenance>], + dest: &MPlaceTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, EmulateItemResult> { + let this = self.eval_context_mut(); + match link_name.as_str() { + // Miscellaneous + "__errno" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let errno_place = this.last_error_place()?; + this.write_scalar(errno_place.to_ref(this).to_scalar(), dest)?; + } + + _ => return Ok(EmulateItemResult::NotSupported), + } + Ok(EmulateItemResult::NeedsJumping) + } +} diff --git a/src/tools/miri/src/shims/unix/android/mod.rs b/src/tools/miri/src/shims/unix/android/mod.rs new file mode 100644 index 00000000000..09c6507b24f --- /dev/null +++ b/src/tools/miri/src/shims/unix/android/mod.rs @@ -0,0 +1 @@ +pub mod foreign_items; diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 78d297d4b04..9daaebecc3b 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -9,6 +9,7 @@ use crate::shims::unix::*; use crate::*; +use shims::unix::android::foreign_items as android; use shims::unix::freebsd::foreign_items as freebsd; use shims::unix::linux::foreign_items as linux; use shims::unix::macos::foreign_items as macos; @@ -26,6 +27,7 @@ pub fn is_dyn_sym(name: &str, target_os: &str) -> bool { // Give specific OSes a chance to allow their symbols. _ => match target_os { + "android" => android::is_dyn_sym(name), "freebsd" => freebsd::is_dyn_sym(name), "linux" => linux::is_dyn_sym(name), "macos" => macos::is_dyn_sym(name), @@ -267,7 +269,7 @@ fn emulate_foreign_item_inner( "reallocarray" => { // Currently this function does not exist on all Unixes, e.g. on macOS. - if !matches!(&*this.tcx.sess.target.os, "linux" | "freebsd") { + if !matches!(&*this.tcx.sess.target.os, "linux" | "freebsd" | "android") { throw_unsup_format!( "`reallocarray` is not supported on {}", this.tcx.sess.target.os @@ -585,7 +587,7 @@ fn emulate_foreign_item_inner( "getentropy" => { // This function is non-standard but exists with the same signature and behavior on // Linux, macOS, FreeBSD and Solaris/Illumos. - if !matches!(&*this.tcx.sess.target.os, "linux" | "macos" | "freebsd" | "illumos" | "solaris") { + if !matches!(&*this.tcx.sess.target.os, "linux" | "macos" | "freebsd" | "illumos" | "solaris" | "android") { throw_unsup_format!( "`getentropy` is not supported on {}", this.tcx.sess.target.os @@ -614,9 +616,9 @@ fn emulate_foreign_item_inner( "getrandom" => { // This function is non-standard but exists with the same signature and behavior on // Linux, FreeBSD and Solaris/Illumos. - if !matches!(&*this.tcx.sess.target.os, "linux" | "freebsd" | "illumos" | "solaris") { + if !matches!(&*this.tcx.sess.target.os, "linux" | "freebsd" | "illumos" | "solaris" | "android") { throw_unsup_format!( - "`getentropy` is not supported on {}", + "`getrandom` is not supported on {}", this.tcx.sess.target.os ); } @@ -740,6 +742,7 @@ fn emulate_foreign_item_inner( _ => { let target_os = &*this.tcx.sess.target.os; return match target_os { + "android" => android::EvalContextExt::emulate_foreign_item_inner(this, link_name, abi, args, dest), "freebsd" => freebsd::EvalContextExt::emulate_foreign_item_inner(this, link_name, abi, args, dest), "linux" => linux::EvalContextExt::emulate_foreign_item_inner(this, link_name, abi, args, dest), "macos" => macos::EvalContextExt::emulate_foreign_item_inner(this, link_name, abi, args, dest), diff --git a/src/tools/miri/src/shims/unix/mod.rs b/src/tools/miri/src/shims/unix/mod.rs index 6dee30d895c..dc9068fddde 100644 --- a/src/tools/miri/src/shims/unix/mod.rs +++ b/src/tools/miri/src/shims/unix/mod.rs @@ -8,6 +8,7 @@ mod sync; mod thread; +mod android; mod freebsd; mod linux; mod macos; diff --git a/src/tools/miri/tests/fail/environ-gets-deallocated.rs b/src/tools/miri/tests/fail/environ-gets-deallocated.rs index 08545a72567..5391a9176d0 100644 --- a/src/tools/miri/tests/fail/environ-gets-deallocated.rs +++ b/src/tools/miri/tests/fail/environ-gets-deallocated.rs @@ -1,11 +1,5 @@ //@ignore-target-windows: Windows does not have a global environ list that the program can access directly -#[cfg(any( - target_os = "linux", - target_os = "freebsd", - target_os = "solaris", - target_os = "illumos" -))] fn get_environ() -> *const *const u8 { extern "C" { static mut environ: *const *const u8; @@ -13,14 +7,6 @@ fn get_environ() -> *const *const u8 { unsafe { environ } } -#[cfg(target_os = "macos")] -fn get_environ() -> *const *const u8 { - extern "C" { - fn _NSGetEnviron() -> *mut *const *const u8; - } - unsafe { *_NSGetEnviron() } -} - fn main() { let pointer = get_environ(); let _x = unsafe { *pointer }; diff --git a/src/tools/miri/tests/pass-dep/libc/libc-misc.rs b/src/tools/miri/tests/pass-dep/libc/libc-misc.rs index 736e0bf8eb7..f7e1d9faa6a 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-misc.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-misc.rs @@ -10,9 +10,11 @@ fn test_thread_local_errno() { #[cfg(any(target_os = "illumos", target_os = "solaris"))] use libc::___errno as __errno_location; + #[cfg(target_os = "android")] + use libc::__errno as __errno_location; #[cfg(target_os = "linux")] use libc::__errno_location; - #[cfg(any(target_os = "macos", target_os = "freebsd"))] + #[cfg(any(target_os = "freebsd", target_os = "macos"))] use libc::__error as __errno_location; unsafe { @@ -28,6 +30,21 @@ fn test_thread_local_errno() { } } +fn test_environ() { + // Just a smoke test for now, checking that the extern static exists. + extern "C" { + static mut environ: *const *const libc::c_char; + } + + unsafe { + let mut e = environ; + // Iterate to the end. + while !(*e).is_null() { + e = e.add(1); + } + } +} + #[cfg(target_os = "linux")] fn test_sigrt() { let min = libc::SIGRTMIN(); @@ -60,6 +77,7 @@ fn test_dlsym() { fn main() { test_thread_local_errno(); + test_environ(); test_dlsym(); diff --git a/src/tools/miri/tests/pass/shims/env/home.rs b/src/tools/miri/tests/pass/shims/env/home.rs index c237f9ed9ff..8b4b907a51d 100644 --- a/src/tools/miri/tests/pass/shims/env/home.rs +++ b/src/tools/miri/tests/pass/shims/env/home.rs @@ -2,8 +2,12 @@ use std::env; fn main() { - env::remove_var("HOME"); // make sure we enter the interesting codepath - env::remove_var("USERPROFILE"); // Windows also looks as this env var + // Remove the env vars to hit the underlying shim -- except + // on android where the env var is all we have. + #[cfg(not(target_os = "android"))] + env::remove_var("HOME"); + env::remove_var("USERPROFILE"); + #[allow(deprecated)] env::home_dir().unwrap(); } From 2b9c1caa18f524d69258a9ec6d55c9df70c7e832 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 19 May 2024 14:01:52 +0200 Subject: [PATCH 19/24] properly print error in 'cargo miri setup --print-sysroot' --- src/tools/miri/cargo-miri/Cargo.lock | 4 +- src/tools/miri/cargo-miri/Cargo.toml | 2 +- src/tools/miri/cargo-miri/src/setup.rs | 65 +++++++++------------- src/tools/miri/miri-script/src/commands.rs | 23 +++----- 4 files changed, 36 insertions(+), 58 deletions(-) diff --git a/src/tools/miri/cargo-miri/Cargo.lock b/src/tools/miri/cargo-miri/Cargo.lock index 7d965dce07d..8bd8f103053 100644 --- a/src/tools/miri/cargo-miri/Cargo.lock +++ b/src/tools/miri/cargo-miri/Cargo.lock @@ -178,9 +178,9 @@ dependencies = [ [[package]] name = "rustc-build-sysroot" -version = "0.5.0" +version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "de6077473f0c46779b49e4587a81f1b8919e0ec26630409ecfda0ba3259efb43" +checksum = "fa3ca63cc537c1cb69e4c2c0afc5fda2ccd36ac84c97d5a4ae05e69b1c834afb" dependencies = [ "anyhow", "rustc_version", diff --git a/src/tools/miri/cargo-miri/Cargo.toml b/src/tools/miri/cargo-miri/Cargo.toml index a68854de625..6acdbc46f64 100644 --- a/src/tools/miri/cargo-miri/Cargo.toml +++ b/src/tools/miri/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.5.0" +rustc-build-sysroot = "0.5.2" # Enable some feature flags that dev-dependencies need but dependencies # do not. This makes `./miri install` after `./miri build` faster. diff --git a/src/tools/miri/cargo-miri/src/setup.rs b/src/tools/miri/cargo-miri/src/setup.rs index 508d3045365..fe67aad465c 100644 --- a/src/tools/miri/cargo-miri/src/setup.rs +++ b/src/tools/miri/cargo-miri/src/setup.rs @@ -2,7 +2,6 @@ use std::env; use std::ffi::OsStr; -use std::fmt::Write; use std::path::PathBuf; use std::process::{self, Command}; @@ -24,6 +23,7 @@ pub fn setup( let only_setup = matches!(subcommand, MiriCommand::Setup); let ask_user = !only_setup; let print_sysroot = only_setup && has_arg_flag("--print-sysroot"); // whether we just print the sysroot path + let show_setup = only_setup && !print_sysroot; if !only_setup { if let Some(sysroot) = std::env::var_os("MIRI_SYSROOT") { // Skip setup step if MIRI_SYSROOT is explicitly set, *unless* we are `cargo miri setup`. @@ -115,18 +115,16 @@ pub fn setup( // `config.toml`. command.env("RUSTC_WRAPPER", ""); - if only_setup && !print_sysroot { + if show_setup { // Forward output. Even make it verbose, if requested. + command.stdout(process::Stdio::inherit()); + command.stderr(process::Stdio::inherit()); for _ in 0..verbose { command.arg("-v"); } if quiet { command.arg("--quiet"); } - } else { - // Suppress output. - command.stdout(process::Stdio::null()); - command.stderr(process::Stdio::null()); } command @@ -137,22 +135,25 @@ pub fn setup( // not apply `RUSTFLAGS` to the sysroot either. let rustflags = &["-Cdebug-assertions=off", "-Coverflow-checks=on"]; + let mut after_build_output = String::new(); // what should be printed when the build is done. let notify = || { - let mut msg = String::new(); - write!(msg, "Preparing a sysroot for Miri (target: {target})").unwrap(); - if verbose > 0 { - write!(msg, " in {}", sysroot_dir.display()).unwrap(); - } - write!(msg, "...").unwrap(); - - if print_sysroot || quiet { - // Be silent. - } else if only_setup { - // We want to be explicit. - eprintln!("{msg}"); - } else { - // We want to be quiet, but still let the user know that something is happening. - eprint!("{msg} "); + if !quiet { + eprint!("Preparing a sysroot for Miri (target: {target})"); + if verbose > 0 { + eprint!(" in {}", sysroot_dir.display()); + } + if show_setup { + // Cargo will print things, so we need to finish this line. + eprintln!("..."); + after_build_output = format!( + "A sysroot for Miri is now available in `{}`.\n", + sysroot_dir.display() + ); + } else { + // Keep all output on a single line. + eprint!("... "); + after_build_output = format!("done\n"); + } } }; @@ -167,31 +168,17 @@ pub fn setup( .build_from_source(&rust_src); match status { Ok(SysrootStatus::AlreadyCached) => - if only_setup && !(print_sysroot || quiet) { + if !quiet && show_setup { eprintln!( "A sysroot for Miri is already available in `{}`.", sysroot_dir.display() ); }, Ok(SysrootStatus::SysrootBuilt) => { - if print_sysroot || quiet { - // Be silent. - } else if only_setup { - eprintln!("A sysroot for Miri is now available in `{}`.", sysroot_dir.display()); - } else { - eprintln!("done"); - } + // Print what `notify` prepared. + eprint!("{after_build_output}"); } - Err(err) => - if print_sysroot { - show_error!("failed to build sysroot") - } else if only_setup { - show_error!("failed to build sysroot: {err:?}") - } else { - show_error!( - "failed to build sysroot; run `cargo miri setup` to see the error details" - ) - }, + Err(err) => show_error!("failed to build sysroot: {err:?}"), } if print_sysroot { diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 8e2b07ad805..7b5a047bf75 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -42,28 +42,19 @@ fn build_miri_sysroot(&mut self, quiet: bool, target: Option<&OsStr>) -> Result< let target_flag = &target_flag; if !quiet { + eprint!("$ cargo miri setup"); if let Some(target) = target { - eprintln!("$ (building Miri sysroot for {})", target.to_string_lossy()); - } else { - eprintln!("$ (building Miri sysroot)"); + eprint!(" --target {target}", target = target.to_string_lossy()); } + eprintln!(); } - let output = cmd!(self.sh, + let mut cmd = cmd!(self.sh, "cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- miri setup --print-sysroot {target_flag...}" - ).read(); - let Ok(output) = output else { - // Run it again (without `--print-sysroot` or `--quiet`) so the user can see the error. - cmd!( - self.sh, - "cargo +{toolchain} run {cargo_extra_flags...} --manifest-path {manifest_path} -- - miri setup {target_flag...}" - ) - .run() - .with_context(|| "`cargo miri setup` failed")?; - panic!("`cargo miri setup` didn't fail again the 2nd time?"); - }; + ); + cmd.set_quiet(quiet); + let output = cmd.read()?; self.sh.set_var("MIRI_SYSROOT", &output); Ok(output.into()) } From b8a7c7379283c4d6078871f9024b820642c88856 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 19 May 2024 16:48:48 +0200 Subject: [PATCH 20/24] test wasm32-wasip2 instead of the deprecated wasm32-wasi target --- src/tools/miri/ci/ci.sh | 2 +- src/tools/miri/test_dependencies/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 50ac4539c26..9480663a01d 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -151,7 +151,7 @@ case $HOST_TARGET in TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX pthread-sync TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC $UNIX pthread-sync TEST_TARGET=aarch64-linux-android run_tests_minimal $BASIC $UNIX - TEST_TARGET=wasm32-wasi run_tests_minimal empty_main wasm heap_alloc libc-mem + TEST_TARGET=wasm32-wasip2 run_tests_minimal empty_main wasm heap_alloc libc-mem TEST_TARGET=wasm32-unknown-unknown run_tests_minimal empty_main wasm TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std # Custom target JSON file diff --git a/src/tools/miri/test_dependencies/Cargo.toml b/src/tools/miri/test_dependencies/Cargo.toml index 1894f53ce49..e40dd50a444 100644 --- a/src/tools/miri/test_dependencies/Cargo.toml +++ b/src/tools/miri/test_dependencies/Cargo.toml @@ -11,12 +11,12 @@ edition = "2021" # all dependencies (and their transitive ones) listed here can be used in `tests/`. libc = "0.2" num_cpus = "1.10.1" -tempfile = "3" getrandom_01 = { package = "getrandom", version = "0.1" } getrandom_02 = { package = "getrandom", version = "0.2", features = ["js"] } [target.'cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))'.dependencies] +tempfile = "3" page_size = "0.6" tokio = { version = "1.24", features = ["macros", "rt-multi-thread", "time", "net"] } From 9cba160d524698f6f9292589e45909b2450ff60b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 19 May 2024 18:03:13 +0200 Subject: [PATCH 21/24] use a little arg-parsing helper for miri-script --- src/tools/miri/miri-script/src/args.rs | 136 ++++++++++++++++++ src/tools/miri/miri-script/src/commands.rs | 74 ++++++---- src/tools/miri/miri-script/src/main.rs | 158 +++++++++------------ src/tools/miri/miri-script/src/util.rs | 34 +---- src/tools/miri/src/bin/miri.rs | 5 +- 5 files changed, 261 insertions(+), 146 deletions(-) create mode 100644 src/tools/miri/miri-script/src/args.rs diff --git a/src/tools/miri/miri-script/src/args.rs b/src/tools/miri/miri-script/src/args.rs new file mode 100644 index 00000000000..16a21757b35 --- /dev/null +++ b/src/tools/miri/miri-script/src/args.rs @@ -0,0 +1,136 @@ +use std::env; +use std::iter; + +use anyhow::{bail, Result}; + +pub struct Args { + args: iter::Peekable, + /// Set to `true` once we saw a `--`. + terminated: bool, +} + +impl Args { + pub fn new() -> Self { + let mut args = Args { args: env::args().peekable(), terminated: false }; + args.args.next().unwrap(); // skip program name + args + } + + /// Get the next argument without any interpretation. + pub fn next_raw(&mut self) -> Option { + self.args.next() + } + + /// Consume a `-$f` flag if present. + pub fn get_short_flag(&mut self, flag: char) -> Result { + if self.terminated { + return Ok(false); + } + if let Some(next) = self.args.peek() { + if let Some(next) = next.strip_prefix("-") { + if let Some(next) = next.strip_prefix(flag) { + if next.is_empty() { + self.args.next().unwrap(); // consume this argument + return Ok(true); + } else { + bail!("`-{flag}` followed by value"); + } + } + } + } + Ok(false) + } + + /// Consume a `--$name` flag if present. + pub fn get_long_flag(&mut self, name: &str) -> Result { + if self.terminated { + return Ok(false); + } + if let Some(next) = self.args.peek() { + if let Some(next) = next.strip_prefix("--") { + if next == name { + self.args.next().unwrap(); // consume this argument + return Ok(true); + } + } + } + Ok(false) + } + + /// Consume a `--$name val` or `--$name=val` option if present. + pub fn get_long_opt(&mut self, name: &str) -> Result> { + assert!(!name.is_empty()); + if self.terminated { + return Ok(None); + } + let Some(next) = self.args.peek() else { return Ok(None) }; + let Some(next) = next.strip_prefix("--") else { return Ok(None) }; + let Some(next) = next.strip_prefix(name) else { return Ok(None) }; + // Starts with `--flag`. + Ok(if let Some(val) = next.strip_prefix("=") { + // `--flag=val` form + let val = val.into(); + self.args.next().unwrap(); // consume this argument + Some(val) + } else if next.is_empty() { + // `--flag val` form + self.args.next().unwrap(); // consume this argument + let Some(val) = self.args.next() else { bail!("`--{name}` not followed by value") }; + Some(val) + } else { + // Some unrelated flag, like `--flag-more` or so. + None + }) + } + + /// Consume a `--$name=val` or `--$name` option if present; the latter + /// produces a default value. (`--$name val` is *not* accepted for this form + /// of argument, it understands `val` already as the next argument!) + pub fn get_long_opt_with_default( + &mut self, + name: &str, + default: &str, + ) -> Result> { + assert!(!name.is_empty()); + if self.terminated { + return Ok(None); + } + let Some(next) = self.args.peek() else { return Ok(None) }; + let Some(next) = next.strip_prefix("--") else { return Ok(None) }; + let Some(next) = next.strip_prefix(name) else { return Ok(None) }; + // Starts with `--flag`. + Ok(if let Some(val) = next.strip_prefix("=") { + // `--flag=val` form + let val = val.into(); + self.args.next().unwrap(); // consume this argument + Some(val) + } else if next.is_empty() { + // `--flag` form + self.args.next().unwrap(); // consume this argument + Some(default.into()) + } else { + // Some unrelated flag, like `--flag-more` or so. + None + }) + } + + /// Returns the next free argument or uninterpreted flag, or `None` if there are no more + /// arguments left. `--` is returned as well, but it is interpreted in the sense that no more + /// flags will be parsed after this. + pub fn get_other(&mut self) -> Option { + if self.terminated { + return self.args.next(); + } + let next = self.args.next()?; + if next == "--" { + self.terminated = true; // don't parse any more flags + // This is where our parser is special, we do yield the `--`. + } + Some(next) + } + + /// Return the rest of the aguments entirely unparsed. + pub fn remainder(self) -> Vec { + self.args.collect() + } +} diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 7b5a047bf75..8061f99b5d6 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -25,7 +25,11 @@ impl MiriEnv { /// Returns the location of the sysroot. /// /// If the target is None the sysroot will be built for the host machine. - fn build_miri_sysroot(&mut self, quiet: bool, target: Option<&OsStr>) -> Result { + fn build_miri_sysroot( + &mut self, + quiet: bool, + target: Option>, + ) -> Result { if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") { // Sysroot already set, use that. return Ok(miri_sysroot.into()); @@ -37,14 +41,17 @@ fn build_miri_sysroot(&mut self, quiet: bool, target: Option<&OsStr>) -> Result< self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?; self.build(&manifest_path, &[], quiet)?; - let target_flag = - if let Some(target) = target { vec![OsStr::new("--target"), target] } else { vec![] }; + let target_flag = if let Some(target) = &target { + vec![OsStr::new("--target"), target.as_ref()] + } else { + vec![] + }; let target_flag = &target_flag; if !quiet { eprint!("$ cargo miri setup"); - if let Some(target) = target { - eprint!(" --target {target}", target = target.to_string_lossy()); + if let Some(target) = &target { + eprint!(" --target {target}", target = target.as_ref().to_string_lossy()); } eprintln!(); } @@ -157,8 +164,8 @@ pub fn exec(self) -> Result<()> { Command::Build { flags } => Self::build(flags), Command::Check { flags } => Self::check(flags), Command::Test { bless, flags, target } => Self::test(bless, flags, target), - Command::Run { dep, verbose, many_seeds, flags } => - Self::run(dep, verbose, many_seeds, flags), + Command::Run { dep, verbose, many_seeds, target, edition, flags } => + Self::run(dep, verbose, many_seeds, target, edition, flags), Command::Fmt { flags } => Self::fmt(flags), Command::Clippy { flags } => Self::clippy(flags), Command::Cargo { flags } => Self::cargo(flags), @@ -169,7 +176,7 @@ pub fn exec(self) -> Result<()> { } } - fn toolchain(flags: Vec) -> Result<()> { + fn toolchain(flags: Vec) -> Result<()> { // Make sure rustup-toolchain-install-master is installed. which::which("rustup-toolchain-install-master") .context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?; @@ -364,7 +371,7 @@ fn rustc_push(github_user: String, branch: String) -> Result<()> { Ok(()) } - fn bench(target: Option, benches: Vec) -> Result<()> { + fn bench(target: Option, benches: Vec) -> Result<()> { // The hyperfine to use let hyperfine = env::var("HYPERFINE"); let hyperfine = hyperfine.as_deref().unwrap_or("hyperfine -w 1 -m 5 --shell=none"); @@ -378,14 +385,14 @@ fn bench(target: Option, benches: Vec) -> Result<()> { let sh = Shell::new()?; sh.change_dir(miri_dir()?); let benches_dir = "bench-cargo-miri"; - let benches = if benches.is_empty() { + let benches: Vec = if benches.is_empty() { sh.read_dir(benches_dir)? .into_iter() .filter(|path| path.is_dir()) .map(Into::into) .collect() } else { - benches.to_owned() + benches.into_iter().map(Into::into).collect() }; let target_flag = if let Some(target) = target { let mut flag = OsString::from("--target="); @@ -409,28 +416,28 @@ fn bench(target: Option, benches: Vec) -> Result<()> { Ok(()) } - fn install(flags: Vec) -> Result<()> { + fn install(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; e.install_to_sysroot(e.miri_dir.clone(), &flags)?; e.install_to_sysroot(path!(e.miri_dir / "cargo-miri"), &flags)?; Ok(()) } - fn build(flags: Vec) -> Result<()> { + fn build(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; e.build(path!(e.miri_dir / "Cargo.toml"), &flags, /* quiet */ false)?; e.build(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags, /* quiet */ false)?; Ok(()) } - fn check(flags: Vec) -> Result<()> { + fn check(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; e.check(path!(e.miri_dir / "Cargo.toml"), &flags)?; e.check(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; Ok(()) } - fn clippy(flags: Vec) -> Result<()> { + fn clippy(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; e.clippy(path!(e.miri_dir / "Cargo.toml"), &flags)?; e.clippy(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; @@ -438,7 +445,7 @@ fn clippy(flags: Vec) -> Result<()> { Ok(()) } - fn cargo(flags: Vec) -> Result<()> { + fn cargo(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; let toolchain = &e.toolchain; // We carefully kept the working dir intact, so this will run cargo *on the workspace in the @@ -447,7 +454,7 @@ fn cargo(flags: Vec) -> Result<()> { Ok(()) } - fn test(bless: bool, mut flags: Vec, target: Option) -> Result<()> { + fn test(bless: bool, mut flags: Vec, target: Option) -> Result<()> { let mut e = MiriEnv::new()?; // Prepare a sysroot. @@ -475,21 +482,30 @@ fn run( dep: bool, verbose: bool, many_seeds: Option>, - mut flags: Vec, + target: Option, + edition: Option, + flags: Vec, ) -> Result<()> { let mut e = MiriEnv::new()?; - let target = arg_flag_value(&flags, "--target"); + // More flags that we will pass before `flags` + // (because `flags` may contain `--`). + let mut early_flags = Vec::::new(); - // Scan for "--edition", set one ourselves if that flag is not present. - let have_edition = arg_flag_value(&flags, "--edition").is_some(); - if !have_edition { - flags.push("--edition=2021".into()); // keep in sync with `tests/ui.rs`.` + // Add target, edition to flags. + if let Some(target) = &target { + early_flags.push("--target".into()); + early_flags.push(target.into()); } + if verbose { + early_flags.push("--verbose".into()); + } + early_flags.push("--edition".into()); + early_flags.push(edition.as_deref().unwrap_or("2021").into()); - // Prepare a sysroot, and add it to the flags. + // Prepare a sysroot, add it to the flags. let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref())?; - flags.push("--sysroot".into()); - flags.push(miri_sysroot.into()); + early_flags.push("--sysroot".into()); + early_flags.push(miri_sysroot.into()); // Compute everything needed to run the actual command. Also add MIRIFLAGS. let miri_manifest = path!(e.miri_dir / "Cargo.toml"); @@ -515,7 +531,7 @@ fn run( }; cmd.set_quiet(!verbose); // Add Miri flags - let cmd = cmd.args(&miri_flags).args(seed_flag).args(&flags); + let cmd = cmd.args(&miri_flags).args(&seed_flag).args(&early_flags).args(&flags); // And run the thing. Ok(cmd.run()?) }; @@ -534,7 +550,7 @@ fn run( Ok(()) } - fn fmt(flags: Vec) -> Result<()> { + fn fmt(flags: Vec) -> Result<()> { use itertools::Itertools; let e = MiriEnv::new()?; @@ -556,6 +572,6 @@ fn fmt(flags: Vec) -> Result<()> { .filter_ok(|item| item.file_type().is_file()) .map_ok(|item| item.into_path()); - e.format_files(files, &e.toolchain[..], &config_path, &flags[..]) + e.format_files(files, &e.toolchain[..], &config_path, &flags) } } diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs index a8626ceb45d..d436ef0c5aa 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -1,10 +1,10 @@ #![allow(clippy::needless_question_mark)] +mod args; mod commands; mod util; -use std::ffi::OsString; -use std::{env, ops::Range}; +use std::ops::Range; use anyhow::{anyhow, bail, Context, Result}; @@ -16,26 +16,26 @@ pub enum Command { /// sysroot, to prevent conflicts with other toolchains. Install { /// Flags that are passed through to `cargo install`. - flags: Vec, + flags: Vec, }, /// Just build miri. Build { /// Flags that are passed through to `cargo build`. - flags: Vec, + flags: Vec, }, /// Just check miri. Check { /// Flags that are passed through to `cargo check`. - flags: Vec, + flags: Vec, }, /// Build miri, set up a sysroot and then run the test suite. Test { bless: bool, /// The cross-interpretation target. /// If none then the host is the target. - target: Option, + target: Option, /// Flags that are passed through to the test harness. - flags: Vec, + flags: Vec, }, /// Build miri, set up a sysroot and then run the driver with the given . /// (Also respects MIRIFLAGS environment variable.) @@ -43,33 +43,35 @@ pub enum Command { dep: bool, verbose: bool, many_seeds: Option>, + target: Option, + edition: Option, /// Flags that are passed through to `miri`. - flags: Vec, + flags: Vec, }, /// Format all sources and tests. Fmt { /// Flags that are passed through to `rustfmt`. - flags: Vec, + flags: Vec, }, /// Runs clippy on all sources. Clippy { /// Flags that are passed through to `cargo clippy`. - flags: Vec, + flags: Vec, }, /// Runs just `cargo ` with the Miri-specific environment variables. /// Mainly meant to be invoked by rust-analyzer. - Cargo { flags: Vec }, + Cargo { flags: Vec }, /// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. Bench { - target: Option, + target: Option, /// List of benchmarks to run. By default all benchmarks are run. - benches: Vec, + benches: Vec, }, /// Update and activate the rustup toolchain 'miri' to the commit given in the /// `rust-version` file. /// `rustup-toolchain-install-master` must be installed for this to work. Any extra /// flags are passed to `rustup-toolchain-install-master`. - Toolchain { flags: Vec }, + Toolchain { flags: Vec }, /// Pull and merge Miri changes from the rustc repo. Defaults to fetching the latest /// rustc commit. The fetched commit is stored in the `rust-version` file, so the /// next `./miri toolchain` will install the rustc that just got pulled. @@ -145,113 +147,95 @@ pub enum Command { fn main() -> Result<()> { // We are hand-rolling our own argument parser, since `clap` can't express what we need // (https://github.com/clap-rs/clap/issues/5055). - let mut args = env::args_os().peekable(); - args.next().unwrap(); // skip program name - let command = match args.next().and_then(|s| s.into_string().ok()).as_deref() { - Some("build") => Command::Build { flags: args.collect() }, - Some("check") => Command::Check { flags: args.collect() }, + let mut args = args::Args::new(); + let command = match args.next_raw().as_deref() { + Some("build") => Command::Build { flags: args.remainder() }, + Some("check") => Command::Check { flags: args.remainder() }, Some("test") => { let mut target = None; let mut bless = false; - - while let Some(arg) = args.peek().and_then(|s| s.to_str()) { - match arg { - "--bless" => bless = true, - "--target" => { - // Skip "--target" - args.next().unwrap(); - // Next argument is the target triple. - let val = args.peek().ok_or_else(|| { - anyhow!("`--target` must be followed by target triple") - })?; - target = Some(val.to_owned()); - } - // Only parse the leading flags. - _ => break, + let mut flags = Vec::new(); + loop { + if args.get_long_flag("bless")? { + bless = true; + } else if let Some(val) = args.get_long_opt("target")? { + target = Some(val); + } else if let Some(flag) = args.get_other() { + flags.push(flag); + } else { + break; } - - // Consume the flag, look at the next one. - args.next().unwrap(); } - - Command::Test { bless, flags: args.collect(), target } + Command::Test { bless, flags, target } } Some("run") => { let mut dep = false; let mut verbose = false; let mut many_seeds = None; - while let Some(arg) = args.peek().and_then(|s| s.to_str()) { - if arg == "--dep" { + let mut target = None; + let mut edition = None; + let mut flags = Vec::new(); + loop { + if args.get_long_flag("dep")? { dep = true; - } else if arg == "-v" || arg == "--verbose" { + } else if args.get_long_flag("verbose")? || args.get_short_flag('v')? { verbose = true; - } else if arg == "--many-seeds" { - many_seeds = Some(0..256); - } else if let Some(val) = arg.strip_prefix("--many-seeds=") { + } else if let Some(val) = args.get_long_opt_with_default("many-seeds", "0..256")? { let (from, to) = val.split_once("..").ok_or_else(|| { - anyhow!("invalid format for `--many-seeds`: expected `from..to`") + anyhow!("invalid format for `--many-seeds-range`: expected `from..to`") })?; let from: u32 = if from.is_empty() { 0 } else { - from.parse().context("invalid `from` in `--many-seeds=from..to")? + from.parse().context("invalid `from` in `--many-seeds-range=from..to")? }; - let to: u32 = to.parse().context("invalid `to` in `--many-seeds=from..to")?; + let to: u32 = + to.parse().context("invalid `to` in `--many-seeds-range=from..to")?; many_seeds = Some(from..to); + } else if let Some(val) = args.get_long_opt("target")? { + target = Some(val); + } else if let Some(val) = args.get_long_opt("edition")? { + edition = Some(val); + } else if let Some(flag) = args.get_other() { + flags.push(flag); } else { - break; // not for us + break; } - // Consume the flag, look at the next one. - args.next().unwrap(); } - Command::Run { dep, verbose, many_seeds, flags: args.collect() } + Command::Run { dep, verbose, many_seeds, target, edition, flags } } - Some("fmt") => Command::Fmt { flags: args.collect() }, - Some("clippy") => Command::Clippy { flags: args.collect() }, - Some("cargo") => Command::Cargo { flags: args.collect() }, - Some("install") => Command::Install { flags: args.collect() }, + Some("fmt") => Command::Fmt { flags: args.remainder() }, + Some("clippy") => Command::Clippy { flags: args.remainder() }, + Some("cargo") => Command::Cargo { flags: args.remainder() }, + Some("install") => Command::Install { flags: args.remainder() }, Some("bench") => { let mut target = None; - while let Some(arg) = args.peek().and_then(|s| s.to_str()) { - match arg { - "--target" => { - // Skip "--target" - args.next().unwrap(); - // Next argument is the target triple. - let val = args.peek().ok_or_else(|| { - anyhow!("`--target` must be followed by target triple") - })?; - target = Some(val.to_owned()); - } - // Only parse the leading flags. - _ => break, + let mut benches = Vec::new(); + loop { + if let Some(val) = args.get_long_opt("target")? { + target = Some(val); + } else if let Some(flag) = args.get_other() { + benches.push(flag); + } else { + break; } - - // Consume the flag, look at the next one. - args.next().unwrap(); } - - Command::Bench { target, benches: args.collect() } + Command::Bench { target, benches } } - Some("toolchain") => Command::Toolchain { flags: args.collect() }, + Some("toolchain") => Command::Toolchain { flags: args.remainder() }, Some("rustc-pull") => { - let commit = args.next().map(|a| a.to_string_lossy().into_owned()); - if args.next().is_some() { + let commit = args.next_raw(); + if args.next_raw().is_some() { bail!("Too many arguments for `./miri rustc-pull`"); } Command::RustcPull { commit } } Some("rustc-push") => { - let github_user = args - .next() - .ok_or_else(|| { - anyhow!("Missing first argument for `./miri rustc-push GITHUB_USER [BRANCH]`") - })? - .to_string_lossy() - .into_owned(); - let branch = - args.next().unwrap_or_else(|| "miri-sync".into()).to_string_lossy().into_owned(); - if args.next().is_some() { + let github_user = args.next_raw().ok_or_else(|| { + anyhow!("Missing first argument for `./miri rustc-push GITHUB_USER [BRANCH]`") + })?; + let branch = args.next_raw().unwrap_or_else(|| "miri-sync".into()); + if args.next_raw().is_some() { bail!("Too many arguments for `./miri rustc-push GITHUB_USER BRANCH`"); } Command::RustcPush { github_user, branch } diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index 2b5791f3ea5..e9095a45fce 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -27,30 +27,6 @@ pub fn flagsplit(flags: &str) -> Vec { flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect() } -pub fn arg_flag_value( - args: impl IntoIterator>, - flag: &str, -) -> Option { - let mut args = args.into_iter(); - while let Some(arg) = args.next() { - let arg = arg.as_ref(); - if arg == "--" { - return None; - } - let Some(arg) = arg.to_str() else { - // Skip non-UTF-8 arguments. - continue; - }; - if arg == flag { - // Next one is the value. - return Some(args.next()?.as_ref().to_owned()); - } else if let Some(val) = arg.strip_prefix(flag).and_then(|s| s.strip_prefix("=")) { - return Some(val.to_owned().into()); - } - } - None -} - /// Some extra state we track for building Miri, such as the right RUSTFLAGS. pub struct MiriEnv { /// miri_dir is the root of the miri repository checkout we are working in. @@ -133,7 +109,7 @@ pub fn install_to_sysroot( pub fn build( &self, manifest_path: impl AsRef, - args: &[OsString], + args: &[String], quiet: bool, ) -> Result<()> { let MiriEnv { toolchain, cargo_extra_flags, .. } = self; @@ -149,21 +125,21 @@ pub fn build( Ok(()) } - pub fn check(&self, manifest_path: impl AsRef, args: &[OsString]) -> Result<()> { + pub fn check(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { let MiriEnv { toolchain, cargo_extra_flags, .. } = self; cmd!(self.sh, "cargo +{toolchain} check {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}") .run()?; Ok(()) } - pub fn clippy(&self, manifest_path: impl AsRef, args: &[OsString]) -> Result<()> { + pub fn clippy(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { let MiriEnv { toolchain, cargo_extra_flags, .. } = self; cmd!(self.sh, "cargo +{toolchain} clippy {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}") .run()?; Ok(()) } - pub fn test(&self, manifest_path: impl AsRef, args: &[OsString]) -> Result<()> { + pub fn test(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { let MiriEnv { toolchain, cargo_extra_flags, .. } = self; cmd!( self.sh, @@ -181,7 +157,7 @@ pub fn format_files( files: impl Iterator>, toolchain: &str, config_path: &Path, - flags: &[OsString], + flags: &[String], ) -> anyhow::Result<()> { use itertools::Itertools; diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 305e9cd8d34..d748febeed4 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -405,9 +405,12 @@ fn main() { let mut rustc_args = vec![]; let mut after_dashdash = false; - // If user has explicitly enabled/disabled isolation let mut isolation_enabled: Option = None; + + // Note that we require values to be given with `=`, not with a space. + // This matches how rustc parses `-Z`. + // However, unlike rustc we do not accept a space after `-Z`. for arg in args { if rustc_args.is_empty() { // Very first arg: binary name. From 42cb1ffa3604748073670003fcfeba0277e38c52 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 24 Feb 2024 14:27:27 +0000 Subject: [PATCH 22/24] Directly implement native exception raise methods in miri Windows still needs the old custom ABI as SEH unwinding isn't supported by miri. Unlike DWARF unwinding it preserves all stack frames until right after the do_catch function has executed. Because of this panic_unwind stack allocates the exception object. Miri can't currently model unwinding without destroying stack frames and as such will report a use-after-free of the exception object. --- src/tools/miri/src/intrinsics/mod.rs | 7 +- src/tools/miri/src/shims/foreign_items.rs | 4 + src/tools/miri/src/shims/mod.rs | 2 + .../miri/src/shims/unix/foreign_items.rs | 11 +++ .../miri/tests/pass/panic/unwind_dwarf.rs | 95 +++++++++++++++++++ 5 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 src/tools/miri/tests/pass/panic/unwind_dwarf.rs diff --git a/src/tools/miri/src/intrinsics/mod.rs b/src/tools/miri/src/intrinsics/mod.rs index 7344846d6d9..59b24b0f63e 100644 --- a/src/tools/miri/src/intrinsics/mod.rs +++ b/src/tools/miri/src/intrinsics/mod.rs @@ -26,7 +26,7 @@ fn call_intrinsic( args: &[OpTy<'tcx, Provenance>], dest: &MPlaceTy<'tcx, Provenance>, ret: Option, - _unwind: mir::UnwindAction, + unwind: mir::UnwindAction, ) -> InterpResult<'tcx, Option>> { let this = self.eval_context_mut(); @@ -67,6 +67,11 @@ fn call_intrinsic( this.return_to_block(ret)?; Ok(None) } + EmulateItemResult::NeedsUnwind => { + // Jump to the unwind block to begin unwinding. + this.unwind_to_block(unwind)?; + Ok(None) + } EmulateItemResult::AlreadyJumped => Ok(None), } } diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index a65da823e24..b363531a2dd 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -87,6 +87,10 @@ fn emulate_foreign_item( trace!("{:?}", this.dump_place(&dest.clone().into())); this.return_to_block(ret)?; } + EmulateItemResult::NeedsUnwind => { + // Jump to the unwind block to begin unwinding. + this.unwind_to_block(unwind)?; + } EmulateItemResult::AlreadyJumped => (), EmulateItemResult::NotSupported => { if let Some(body) = this.lookup_exported_symbol(link_name)? { diff --git a/src/tools/miri/src/shims/mod.rs b/src/tools/miri/src/shims/mod.rs index d9c4a2282c1..0e80c142b49 100644 --- a/src/tools/miri/src/shims/mod.rs +++ b/src/tools/miri/src/shims/mod.rs @@ -23,6 +23,8 @@ pub enum EmulateItemResult { /// The caller is expected to jump to the return block. NeedsJumping, + /// The caller is expected to jump to the unwind block. + NeedsUnwind, /// Jumping has already been taken care of. AlreadyJumped, /// The item is not supported. diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 396011cc22a..ed23771d2b5 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -639,6 +639,17 @@ fn emulate_foreign_item_inner( this.gen_random(ptr, len)?; this.write_scalar(Scalar::from_target_usize(len, this), dest)?; } + "_Unwind_RaiseException" => { + trace!("_Unwind_RaiseException: {:?}", this.frame().instance); + + // Get the raw pointer stored in arg[0] (the panic payload). + let [payload] = this.check_shim(abi, Abi::C { unwind: true }, link_name, args)?; + let payload = this.read_scalar(payload)?; + let thread = this.active_thread_mut(); + thread.panic_payloads.push(payload); + + return Ok(EmulateItemResult::NeedsUnwind); + } // 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/src/tools/miri/tests/pass/panic/unwind_dwarf.rs b/src/tools/miri/tests/pass/panic/unwind_dwarf.rs new file mode 100644 index 00000000000..aa5ec05fbc0 --- /dev/null +++ b/src/tools/miri/tests/pass/panic/unwind_dwarf.rs @@ -0,0 +1,95 @@ +//@only-target-linux +#![feature(core_intrinsics, panic_unwind, rustc_attrs)] +#![allow(internal_features)] + +//! Unwinding using `_Unwind_RaiseException` + +extern crate unwind as uw; + +use std::any::Any; +use std::ptr; + +#[repr(C)] +struct Exception { + _uwe: uw::_Unwind_Exception, + cause: Box, +} + +pub fn panic(data: Box) -> u32 { + let exception = Box::new(Exception { + _uwe: uw::_Unwind_Exception { + exception_class: rust_exception_class(), + exception_cleanup, + private: [core::ptr::null(); uw::unwinder_private_data_size], + }, + cause: data, + }); + let exception_param = Box::into_raw(exception) as *mut uw::_Unwind_Exception; + return unsafe { uw::_Unwind_RaiseException(exception_param) as u32 }; + + extern "C" fn exception_cleanup( + _unwind_code: uw::_Unwind_Reason_Code, + _exception: *mut uw::_Unwind_Exception, + ) { + std::process::abort(); + } +} + +pub unsafe fn rust_panic_cleanup(ptr: *mut u8) -> Box { + let exception = ptr as *mut uw::_Unwind_Exception; + if (*exception).exception_class != rust_exception_class() { + std::process::abort(); + } + + let exception = exception.cast::(); + + let exception = Box::from_raw(exception as *mut Exception); + exception.cause +} + +fn rust_exception_class() -> uw::_Unwind_Exception_Class { + // M O Z \0 R U S T -- vendor, language + 0x4d4f5a_00_52555354 +} + +pub fn catch_unwind R>(f: F) -> Result> { + struct Data { + f: Option, + r: Option, + p: Option>, + } + + let mut data = Data { f: Some(f), r: None, p: None }; + + let data_ptr = ptr::addr_of_mut!(data) as *mut u8; + unsafe { + return if std::intrinsics::r#try(do_call::, data_ptr, do_catch::) == 0 { + Ok(data.r.take().unwrap()) + } else { + Err(data.p.take().unwrap()) + }; + } + + fn do_call R, R>(data: *mut u8) { + unsafe { + let data = &mut *data.cast::>(); + let f = data.f.take().unwrap(); + data.r = Some(f()); + } + } + + #[rustc_nounwind] + fn do_catch R, R>(data: *mut u8, payload: *mut u8) { + unsafe { + let obj = rust_panic_cleanup(payload); + (*data.cast::>()).p = Some(obj); + } + } +} + +fn main() { + assert_eq!( + catch_unwind(|| panic(Box::new(42))).unwrap_err().downcast::().unwrap(), + Box::new(42) + ); +} From 5e41ff516fad8f2984cc0c290948c458970e4a86 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 19 May 2024 19:07:53 +0200 Subject: [PATCH 23/24] various small nits - share implementation with miri_starting_unwind - make test use a custom unwinding class - extend comments - use NeedsUnwind more consistently --- src/tools/miri/ci/ci.sh | 2 +- src/tools/miri/src/intrinsics/atomic.rs | 2 +- src/tools/miri/src/intrinsics/mod.rs | 4 +-- src/tools/miri/src/intrinsics/simd.rs | 2 +- src/tools/miri/src/shims/alloc.rs | 2 +- src/tools/miri/src/shims/foreign_items.rs | 19 +++++----- src/tools/miri/src/shims/mod.rs | 4 +-- src/tools/miri/src/shims/panic.rs | 13 +------ .../src/shims/unix/android/foreign_items.rs | 2 +- .../miri/src/shims/unix/foreign_items.rs | 30 +++++++++++----- .../src/shims/unix/freebsd/foreign_items.rs | 2 +- .../src/shims/unix/linux/foreign_items.rs | 2 +- .../src/shims/unix/macos/foreign_items.rs | 2 +- .../src/shims/unix/solarish/foreign_items.rs | 2 +- .../miri/src/shims/wasi/foreign_items.rs | 2 +- .../miri/src/shims/windows/foreign_items.rs | 2 +- src/tools/miri/src/shims/x86/aesni.rs | 2 +- src/tools/miri/src/shims/x86/avx.rs | 2 +- src/tools/miri/src/shims/x86/avx2.rs | 2 +- src/tools/miri/src/shims/x86/mod.rs | 2 +- src/tools/miri/src/shims/x86/sse.rs | 2 +- src/tools/miri/src/shims/x86/sse2.rs | 2 +- src/tools/miri/src/shims/x86/sse3.rs | 2 +- src/tools/miri/src/shims/x86/sse41.rs | 2 +- src/tools/miri/src/shims/x86/ssse3.rs | 2 +- .../miri/tests/pass/panic/unwind_dwarf.rs | 35 ++++++++++--------- 26 files changed, 74 insertions(+), 71 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 9480663a01d..4e92169b3d0 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -145,7 +145,7 @@ case $HOST_TARGET in TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice # Partially supported targets (tier 2) BASIC="empty_main integer vec string btreemap hello hashmap heap_alloc align" # ensures we have the basics: stdout/stderr, system allocator, randomness (for HashMap initialization) - UNIX="panic/panic concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there + UNIX="panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX pthread-sync diff --git a/src/tools/miri/src/intrinsics/atomic.rs b/src/tools/miri/src/intrinsics/atomic.rs index 0c212c45dbd..501dbc1603e 100644 --- a/src/tools/miri/src/intrinsics/atomic.rs +++ b/src/tools/miri/src/intrinsics/atomic.rs @@ -116,7 +116,7 @@ fn fence_ord(ord: &str) -> AtomicFenceOrd { _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/intrinsics/mod.rs b/src/tools/miri/src/intrinsics/mod.rs index 59b24b0f63e..0a7927d0621 100644 --- a/src/tools/miri/src/intrinsics/mod.rs +++ b/src/tools/miri/src/intrinsics/mod.rs @@ -62,7 +62,7 @@ fn call_intrinsic( args: instance.args, })) } - EmulateItemResult::NeedsJumping => { + EmulateItemResult::NeedsReturn => { trace!("{:?}", this.dump_place(&dest.clone().into())); this.return_to_block(ret)?; Ok(None) @@ -446,6 +446,6 @@ fn emulate_intrinsic_by_name( _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/intrinsics/simd.rs b/src/tools/miri/src/intrinsics/simd.rs index d0a78429ca8..4cde364fbc4 100644 --- a/src/tools/miri/src/intrinsics/simd.rs +++ b/src/tools/miri/src/intrinsics/simd.rs @@ -746,7 +746,7 @@ enum Op { _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } fn fminmax_op( diff --git a/src/tools/miri/src/shims/alloc.rs b/src/tools/miri/src/shims/alloc.rs index bd84de81e69..ca672bdc611 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/src/shims/alloc.rs @@ -87,7 +87,7 @@ fn emulate_allocator( } AllocatorKind::Default => { default(this)?; - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } } diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index b363531a2dd..eccccb4a449 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -82,8 +82,8 @@ fn emulate_foreign_item( } // The rest either implements the logic, or falls back to `lookup_exported_symbol`. - match this.emulate_foreign_item_inner(link_name, abi, args, dest, unwind)? { - EmulateItemResult::NeedsJumping => { + match this.emulate_foreign_item_inner(link_name, abi, args, dest)? { + EmulateItemResult::NeedsReturn => { trace!("{:?}", this.dump_place(&dest.clone().into())); this.return_to_block(ret)?; } @@ -210,7 +210,6 @@ fn emulate_foreign_item_inner( abi: Abi, args: &[OpTy<'tcx, Provenance>], dest: &MPlaceTy<'tcx, Provenance>, - unwind: mir::UnwindAction, ) -> InterpResult<'tcx, EmulateItemResult> { let this = self.eval_context_mut(); @@ -222,7 +221,7 @@ fn emulate_foreign_item_inner( // by the specified `.so` file; we should continue and check if it corresponds to // a provided shim. if this.call_native_fn(link_name, dest, args)? { - return Ok(EmulateItemResult::NeedsJumping); + return Ok(EmulateItemResult::NeedsReturn); } } @@ -267,9 +266,9 @@ fn emulate_foreign_item_inner( match link_name.as_str() { // Miri-specific extern functions "miri_start_unwind" => { - // `check_shim` happens inside `handle_miri_start_unwind`. - this.handle_miri_start_unwind(abi, link_name, args, unwind)?; - return Ok(EmulateItemResult::AlreadyJumped); + let [payload] = this.check_shim(abi, Abi::Rust, link_name, args)?; + this.handle_miri_start_unwind(payload)?; + return Ok(EmulateItemResult::NeedsUnwind); } "miri_run_provenance_gc" => { let [] = this.check_shim(abi, Abi::Rust, link_name, args)?; @@ -484,7 +483,7 @@ fn emulate_foreign_item_inner( "__rust_alloc" => return this.emulate_allocator(default), "miri_alloc" => { default(this)?; - return Ok(EmulateItemResult::NeedsJumping); + return Ok(EmulateItemResult::NeedsReturn); } _ => unreachable!(), } @@ -544,7 +543,7 @@ fn emulate_foreign_item_inner( } "miri_dealloc" => { default(this)?; - return Ok(EmulateItemResult::NeedsJumping); + return Ok(EmulateItemResult::NeedsReturn); } _ => unreachable!(), } @@ -965,6 +964,6 @@ fn emulate_foreign_item_inner( }; // We only fall through to here if we did *not* hit the `_` arm above, // i.e., if we actually emulated the function with one of the shims. - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/mod.rs b/src/tools/miri/src/shims/mod.rs index 0e80c142b49..a41a2883c91 100644 --- a/src/tools/miri/src/shims/mod.rs +++ b/src/tools/miri/src/shims/mod.rs @@ -22,10 +22,10 @@ /// What needs to be done after emulating an item (a shim or an intrinsic) is done. pub enum EmulateItemResult { /// The caller is expected to jump to the return block. - NeedsJumping, + NeedsReturn, /// The caller is expected to jump to the unwind block. NeedsUnwind, - /// Jumping has already been taken care of. + /// Jumping to the next block has already been taken care of. AlreadyJumped, /// The item is not supported. NotSupported, diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs index 4444d297469..e0e5396a455 100644 --- a/src/tools/miri/src/shims/panic.rs +++ b/src/tools/miri/src/shims/panic.rs @@ -13,7 +13,6 @@ use rustc_ast::Mutability; use rustc_middle::{mir, ty}; -use rustc_span::Symbol; use rustc_target::spec::abi::Abi; use rustc_target::spec::PanicStrategy; @@ -46,25 +45,15 @@ impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Handles the special `miri_start_unwind` intrinsic, which is called /// by libpanic_unwind to delegate the actual unwinding process to Miri. - fn handle_miri_start_unwind( - &mut self, - abi: Abi, - link_name: Symbol, - args: &[OpTy<'tcx, Provenance>], - unwind: mir::UnwindAction, - ) -> InterpResult<'tcx> { + fn handle_miri_start_unwind(&mut self, payload: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { let this = self.eval_context_mut(); trace!("miri_start_unwind: {:?}", this.frame().instance); - // Get the raw pointer stored in arg[0] (the panic payload). - let [payload] = this.check_shim(abi, Abi::Rust, link_name, args)?; let payload = this.read_scalar(payload)?; let thread = this.active_thread_mut(); thread.panic_payloads.push(payload); - // Jump to the unwind block to begin unwinding. - this.unwind_to_block(unwind)?; Ok(()) } diff --git a/src/tools/miri/src/shims/unix/android/foreign_items.rs b/src/tools/miri/src/shims/unix/android/foreign_items.rs index f85b8a725f2..590a5672f15 100644 --- a/src/tools/miri/src/shims/unix/android/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/android/foreign_items.rs @@ -27,6 +27,6 @@ fn emulate_foreign_item_inner( _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index ed23771d2b5..86dd23f31c7 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -640,14 +640,28 @@ fn emulate_foreign_item_inner( this.write_scalar(Scalar::from_target_usize(len, this), dest)?; } "_Unwind_RaiseException" => { - trace!("_Unwind_RaiseException: {:?}", this.frame().instance); - - // Get the raw pointer stored in arg[0] (the panic payload). + // This is not formally part of POSIX, but it is very wide-spread on POSIX systems. + // It was originally specified as part of the Itanium C++ ABI: + // https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html#base-throw. + // On Linux it is + // documented as part of the LSB: + // https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/baselib--unwind-raiseexception.html + // Basically every other UNIX uses the exact same api though. Arm also references + // back to the Itanium C++ ABI for the definition of `_Unwind_RaiseException` for + // arm64: + // https://github.com/ARM-software/abi-aa/blob/main/cppabi64/cppabi64.rst#toc-entry-35 + // For arm32 they did something custom, but similar enough that the same + // `_Unwind_RaiseException` impl in miri should work: + // https://github.com/ARM-software/abi-aa/blob/main/ehabi32/ehabi32.rst + if !matches!(&*this.tcx.sess.target.os, "linux" | "freebsd" | "illumos" | "solaris" | "android" | "macos") { + throw_unsup_format!( + "`_Unwind_RaiseException` is not supported on {}", + this.tcx.sess.target.os + ); + } + // This function looks and behaves excatly like miri_start_unwind. let [payload] = this.check_shim(abi, Abi::C { unwind: true }, link_name, args)?; - let payload = this.read_scalar(payload)?; - let thread = this.active_thread_mut(); - thread.panic_payloads.push(payload); - + this.handle_miri_start_unwind(payload)?; return Ok(EmulateItemResult::NeedsUnwind); } @@ -771,6 +785,6 @@ fn emulate_foreign_item_inner( } }; - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs index e70cd35dda6..8a7f7e9d1fd 100644 --- a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs @@ -86,6 +86,6 @@ fn emulate_foreign_item_inner( _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 7cd749a4107..b2666101ff2 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -203,6 +203,6 @@ fn emulate_foreign_item_inner( _ => return Ok(EmulateItemResult::NotSupported), }; - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/unix/macos/foreign_items.rs b/src/tools/miri/src/shims/unix/macos/foreign_items.rs index 912623b7225..2b9ce746a56 100644 --- a/src/tools/miri/src/shims/unix/macos/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/macos/foreign_items.rs @@ -177,6 +177,6 @@ fn emulate_foreign_item_inner( _ => return Ok(EmulateItemResult::NotSupported), }; - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/unix/solarish/foreign_items.rs b/src/tools/miri/src/shims/unix/solarish/foreign_items.rs index c216d8afd77..6c5155618c9 100644 --- a/src/tools/miri/src/shims/unix/solarish/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/solarish/foreign_items.rs @@ -45,6 +45,6 @@ fn emulate_foreign_item_inner( _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/wasi/foreign_items.rs b/src/tools/miri/src/shims/wasi/foreign_items.rs index d911f43eb62..774a5e72025 100644 --- a/src/tools/miri/src/shims/wasi/foreign_items.rs +++ b/src/tools/miri/src/shims/wasi/foreign_items.rs @@ -35,6 +35,6 @@ fn emulate_foreign_item_inner( _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index 086abf19c5c..91def80227d 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -762,6 +762,6 @@ fn emulate_foreign_item_inner( _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/x86/aesni.rs b/src/tools/miri/src/shims/x86/aesni.rs index 8dc3748a12d..3a66c431506 100644 --- a/src/tools/miri/src/shims/x86/aesni.rs +++ b/src/tools/miri/src/shims/x86/aesni.rs @@ -127,7 +127,7 @@ fn emulate_x86_aesni_intrinsic( // with an external crate. _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/x86/avx.rs b/src/tools/miri/src/shims/x86/avx.rs index 86ef180a448..b1c61c8b3b2 100644 --- a/src/tools/miri/src/shims/x86/avx.rs +++ b/src/tools/miri/src/shims/x86/avx.rs @@ -344,6 +344,6 @@ fn emulate_x86_avx_intrinsic( } _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/x86/avx2.rs b/src/tools/miri/src/shims/x86/avx2.rs index adecf7b8924..e0bd2298ab8 100644 --- a/src/tools/miri/src/shims/x86/avx2.rs +++ b/src/tools/miri/src/shims/x86/avx2.rs @@ -440,6 +440,6 @@ fn emulate_x86_avx2_intrinsic( } _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 58d6db1886a..1f7138ca338 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/src/shims/x86/mod.rs @@ -144,7 +144,7 @@ fn emulate_x86_intrinsic( _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/x86/sse.rs b/src/tools/miri/src/shims/x86/sse.rs index ea967a8cf72..3636fb2f3fb 100644 --- a/src/tools/miri/src/shims/x86/sse.rs +++ b/src/tools/miri/src/shims/x86/sse.rs @@ -212,6 +212,6 @@ fn emulate_x86_sse_intrinsic( } _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/x86/sse2.rs b/src/tools/miri/src/shims/x86/sse2.rs index 31fa66a496f..54d1e0c803b 100644 --- a/src/tools/miri/src/shims/x86/sse2.rs +++ b/src/tools/miri/src/shims/x86/sse2.rs @@ -388,6 +388,6 @@ fn emulate_x86_sse2_intrinsic( } _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/x86/sse3.rs b/src/tools/miri/src/shims/x86/sse3.rs index 8de339eb9e5..fa1dd07e90b 100644 --- a/src/tools/miri/src/shims/x86/sse3.rs +++ b/src/tools/miri/src/shims/x86/sse3.rs @@ -51,6 +51,6 @@ fn emulate_x86_sse3_intrinsic( } _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/x86/sse41.rs b/src/tools/miri/src/shims/x86/sse41.rs index 011a7a16c68..cd82108678d 100644 --- a/src/tools/miri/src/shims/x86/sse41.rs +++ b/src/tools/miri/src/shims/x86/sse41.rs @@ -176,6 +176,6 @@ fn emulate_x86_sse41_intrinsic( } _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/src/shims/x86/ssse3.rs b/src/tools/miri/src/shims/x86/ssse3.rs index d30de430088..ec625da68c2 100644 --- a/src/tools/miri/src/shims/x86/ssse3.rs +++ b/src/tools/miri/src/shims/x86/ssse3.rs @@ -137,6 +137,6 @@ fn emulate_x86_ssse3_intrinsic( } _ => return Ok(EmulateItemResult::NotSupported), } - Ok(EmulateItemResult::NeedsJumping) + Ok(EmulateItemResult::NeedsReturn) } } diff --git a/src/tools/miri/tests/pass/panic/unwind_dwarf.rs b/src/tools/miri/tests/pass/panic/unwind_dwarf.rs index aa5ec05fbc0..f690be471ba 100644 --- a/src/tools/miri/tests/pass/panic/unwind_dwarf.rs +++ b/src/tools/miri/tests/pass/panic/unwind_dwarf.rs @@ -1,4 +1,4 @@ -//@only-target-linux +//@ignore-target-windows: Windows uses a different unwinding mechanism #![feature(core_intrinsics, panic_unwind, rustc_attrs)] #![allow(internal_features)] @@ -16,28 +16,28 @@ struct Exception { } pub fn panic(data: Box) -> u32 { - let exception = Box::new(Exception { - _uwe: uw::_Unwind_Exception { - exception_class: rust_exception_class(), - exception_cleanup, - private: [core::ptr::null(); uw::unwinder_private_data_size], - }, - cause: data, - }); - let exception_param = Box::into_raw(exception) as *mut uw::_Unwind_Exception; - return unsafe { uw::_Unwind_RaiseException(exception_param) as u32 }; - extern "C" fn exception_cleanup( _unwind_code: uw::_Unwind_Reason_Code, _exception: *mut uw::_Unwind_Exception, ) { std::process::abort(); } + + let exception = Box::new(Exception { + _uwe: uw::_Unwind_Exception { + exception_class: miri_exception_class(), + exception_cleanup: Some(exception_cleanup), + private: [core::ptr::null(); uw::unwinder_private_data_size], + }, + cause: data, + }); + let exception_param = Box::into_raw(exception) as *mut uw::_Unwind_Exception; + return unsafe { uw::_Unwind_RaiseException(exception_param) as u32 }; } pub unsafe fn rust_panic_cleanup(ptr: *mut u8) -> Box { let exception = ptr as *mut uw::_Unwind_Exception; - if (*exception).exception_class != rust_exception_class() { + if (*exception).exception_class != miri_exception_class() { std::process::abort(); } @@ -47,9 +47,10 @@ pub unsafe fn rust_panic_cleanup(ptr: *mut u8) -> Box { exception.cause } -fn rust_exception_class() -> uw::_Unwind_Exception_Class { - // M O Z \0 R U S T -- vendor, language - 0x4d4f5a_00_52555354 +fn miri_exception_class() -> uw::_Unwind_Exception_Class { + // M O Z \0 M I R I -- vendor, language + // (Miri's own exception class is just used for testing) + 0x4d4f5a_00_4d495249 } pub fn catch_unwind R>(f: F) -> Result> { @@ -63,7 +64,7 @@ struct Data { let data_ptr = ptr::addr_of_mut!(data) as *mut u8; unsafe { - return if std::intrinsics::r#try(do_call::, data_ptr, do_catch::) == 0 { + return if std::intrinsics::catch_unwind(do_call::, data_ptr, do_catch::) == 0 { Ok(data.r.take().unwrap()) } else { Err(data.p.take().unwrap()) From e93268eee27a97d1117a887eb233a3d3041d01b0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 19 May 2024 20:31:08 +0200 Subject: [PATCH 24/24] update lockfile --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 186892af21c..a9283d03cb3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3426,9 +3426,9 @@ dependencies = [ [[package]] name = "rustc-build-sysroot" -version = "0.4.7" +version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab1dbbd1bdf65fdac44c885f6cca147ba179108ce284b60a08ccc04b1f1dbac0" +checksum = "fa3ca63cc537c1cb69e4c2c0afc5fda2ccd36ac84c97d5a4ae05e69b1c834afb" dependencies = [ "anyhow", "rustc_version",