From 14795f04485a87e6ee1ac6d40ef2be83307d9ccc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 5 Nov 2025 10:51:19 +0100 Subject: [PATCH 01/14] slightly extend PR creation message --- src/tools/miri/triagebot.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/triagebot.toml b/src/tools/miri/triagebot.toml index c747cbb0a5211..060b749676e62 100644 --- a/src/tools/miri/triagebot.toml +++ b/src/tools/miri/triagebot.toml @@ -20,7 +20,7 @@ contributing_url = "https://github.com/rust-lang/miri/blob/master/CONTRIBUTING.m [assign.custom_welcome_messages] welcome-message = "(unused)" welcome-message-no-reviewer = """ -Thank you for contributing to Miri! +Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two. Please remember to not force-push to the PR branch except when you need to rebase due to a conflict or when the reviewer asks you for it. """ From 4384d431dd581a74d650d1accbf737cfee934c63 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Thu, 6 Nov 2025 04:55:11 +0000 Subject: [PATCH 02/14] Prepare for merging from rust-lang/rust This updates the rust-version file to 401ae55427522984e4a89c37cff6562a4ddcf6b7. --- 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 036282b12f5de..5bdb8bd8369f7 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -5f9dd05862d2e4bceb3be1031b6c936e35671501 +401ae55427522984e4a89c37cff6562a4ddcf6b7 From 3d08696807fbd42c6be092e5b798b261a35eff77 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Thu, 6 Nov 2025 05:03:41 +0000 Subject: [PATCH 03/14] fmt --- src/tools/miri/src/shims/foreign_items.rs | 5 +---- src/tools/miri/src/shims/x86/mod.rs | 4 +--- .../fail/intrinsics/simd_masked_load_element_misaligned.rs | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index a92d8f87af818..bffe633f77979 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -800,10 +800,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // Target-specific shims name if name.starts_with("llvm.x86.") - && matches!( - this.tcx.sess.target.arch, - Arch::X86 | Arch::X86_64 - ) => + && matches!(this.tcx.sess.target.arch, Arch::X86 | Arch::X86_64) => { return shims::x86::EvalContextExt::emulate_x86_intrinsic( this, link_name, abi, args, dest, diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 016cb762cece1..91893737b060f 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/src/shims/x86/mod.rs @@ -42,9 +42,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/addcarry-u32-addcarry-u64.html // https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/subborrow-u32-subborrow-u64.html "addcarry.32" | "addcarry.64" | "subborrow.32" | "subborrow.64" => { - if unprefixed_name.ends_with("64") - && this.tcx.sess.target.arch != Arch::X86_64 - { + if unprefixed_name.ends_with("64") && this.tcx.sess.target.arch != Arch::X86_64 { return interp_ok(EmulateItemResult::NotSupported); } diff --git a/src/tools/miri/tests/fail/intrinsics/simd_masked_load_element_misaligned.rs b/src/tools/miri/tests/fail/intrinsics/simd_masked_load_element_misaligned.rs index 3b5e389cf27ee..47a51dbbab518 100644 --- a/src/tools/miri/tests/fail/intrinsics/simd_masked_load_element_misaligned.rs +++ b/src/tools/miri/tests/fail/intrinsics/simd_masked_load_element_misaligned.rs @@ -6,7 +6,7 @@ use std::simd::*; fn main() { unsafe { let buf = [0u32; 5]; - //~v ERROR: accessing memory with alignment + //~v ERROR: accessing memory with alignment simd_masked_load::<_, _, _, { SimdAlign::Element }>( i32x4::splat(-1), // This is not i32-aligned From 8d597aa36528dada3cbd9bcfec889c2da6ecaaac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Tue, 4 Nov 2025 23:01:07 +0100 Subject: [PATCH 04/14] Remove implementation of LLVM SIMD intrinsics that are not needed anymore --- src/tools/miri/src/shims/x86/avx.rs | 46 +++---------- src/tools/miri/src/shims/x86/avx2.rs | 96 +++------------------------ src/tools/miri/src/shims/x86/mod.rs | 52 --------------- src/tools/miri/src/shims/x86/sse.rs | 23 ------- src/tools/miri/src/shims/x86/sse2.rs | 46 ++----------- src/tools/miri/src/shims/x86/sse3.rs | 17 ----- src/tools/miri/src/shims/x86/sse41.rs | 15 ++--- src/tools/miri/src/shims/x86/ssse3.rs | 26 +++----- 8 files changed, 35 insertions(+), 286 deletions(-) diff --git a/src/tools/miri/src/shims/x86/avx.rs b/src/tools/miri/src/shims/x86/avx.rs index ec365aa1b45e4..636d308d78d98 100644 --- a/src/tools/miri/src/shims/x86/avx.rs +++ b/src/tools/miri/src/shims/x86/avx.rs @@ -1,14 +1,12 @@ use rustc_abi::CanonAbi; use rustc_apfloat::ieee::{Double, Single}; -use rustc_middle::mir; use rustc_middle::ty::Ty; use rustc_span::Symbol; use rustc_target::callconv::FnAbi; use super::{ FloatBinOp, FloatUnaryOp, bin_op_simd_float_all, conditional_dot_product, convert_float_to_int, - horizontal_bin_op, mask_load, mask_store, round_all, test_bits_masked, test_high_bits_masked, - unary_op_ps, + mask_load, mask_store, round_all, test_bits_masked, test_high_bits_masked, unary_op_ps, }; use crate::*; @@ -93,21 +91,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { conditional_dot_product(this, left, right, imm, dest)?; } - // Used to implement the _mm256_h{add,sub}_p{s,d} functions. - // Horizontally add/subtract adjacent floating point values - // in `left` and `right`. - "hadd.ps.256" | "hadd.pd.256" | "hsub.ps.256" | "hsub.pd.256" => { - let [left, right] = - this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - - let which = match unprefixed_name { - "hadd.ps.256" | "hadd.pd.256" => mir::BinOp::Add, - "hsub.ps.256" | "hsub.pd.256" => mir::BinOp::Sub, - _ => unreachable!(), - }; - - horizontal_bin_op(this, which, /*saturating*/ false, left, right, dest)?; - } // Used to implement the _mm256_cmp_ps function. // Performs a comparison operation on each component of `left` // and `right`. For each component, returns 0 if false or u32::MAX @@ -251,40 +234,31 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Unaligned copy, which is what we want. this.mem_copy(src_ptr, dest.ptr(), dest.layout.size, /*nonoverlapping*/ true)?; } - // Used to implement the _mm256_testz_si256, _mm256_testc_si256 and - // _mm256_testnzc_si256 functions. - // Tests `op & mask == 0`, `op & mask == mask` or - // `op & mask != 0 && op & mask != mask` - "ptestz.256" | "ptestc.256" | "ptestnzc.256" => { + // Used to implement the _mm256_testnzc_si256 function. + // Tests `op & mask != 0 && op & mask != mask` + "ptestnzc.256" => { let [op, mask] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; let (all_zero, masked_set) = test_bits_masked(this, op, mask)?; - let res = match unprefixed_name { - "ptestz.256" => all_zero, - "ptestc.256" => masked_set, - "ptestnzc.256" => !all_zero && !masked_set, - _ => unreachable!(), - }; + let res = !all_zero && !masked_set; this.write_scalar(Scalar::from_i32(res.into()), dest)?; } // Used to implement the _mm256_testz_pd, _mm256_testc_pd, _mm256_testnzc_pd - // _mm_testz_pd, _mm_testc_pd, _mm_testnzc_pd, _mm256_testz_ps, - // _mm256_testc_ps, _mm256_testnzc_ps, _mm_testz_ps, _mm_testc_ps and + // _mm_testnzc_pd, _mm256_testz_ps, _mm256_testc_ps, _mm256_testnzc_ps and // _mm_testnzc_ps functions. // Calculates two booleans: // `direct`, which is true when the highest bit of each element of `op & mask` is zero. // `negated`, which is true when the highest bit of each element of `!op & mask` is zero. // Return `direct` (testz), `negated` (testc) or `!direct & !negated` (testnzc) - "vtestz.pd.256" | "vtestc.pd.256" | "vtestnzc.pd.256" | "vtestz.pd" | "vtestc.pd" - | "vtestnzc.pd" | "vtestz.ps.256" | "vtestc.ps.256" | "vtestnzc.ps.256" - | "vtestz.ps" | "vtestc.ps" | "vtestnzc.ps" => { + "vtestz.pd.256" | "vtestc.pd.256" | "vtestnzc.pd.256" | "vtestnzc.pd" + | "vtestz.ps.256" | "vtestc.ps.256" | "vtestnzc.ps.256" | "vtestnzc.ps" => { let [op, mask] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; let (direct, negated) = test_high_bits_masked(this, op, mask)?; let res = match unprefixed_name { - "vtestz.pd.256" | "vtestz.pd" | "vtestz.ps.256" | "vtestz.ps" => direct, - "vtestc.pd.256" | "vtestc.pd" | "vtestc.ps.256" | "vtestc.ps" => negated, + "vtestz.pd.256" | "vtestz.ps.256" => direct, + "vtestc.pd.256" | "vtestc.ps.256" => negated, "vtestnzc.pd.256" | "vtestnzc.pd" | "vtestnzc.ps.256" | "vtestnzc.ps" => !direct && !negated, _ => unreachable!(), diff --git a/src/tools/miri/src/shims/x86/avx2.rs b/src/tools/miri/src/shims/x86/avx2.rs index ca80c0eba1e5c..01e1ac6de59d3 100644 --- a/src/tools/miri/src/shims/x86/avx2.rs +++ b/src/tools/miri/src/shims/x86/avx2.rs @@ -5,8 +5,8 @@ use rustc_span::Symbol; use rustc_target::callconv::FnAbi; use super::{ - ShiftOp, horizontal_bin_op, int_abs, mask_load, mask_store, mpsadbw, packssdw, packsswb, - packusdw, packuswb, pmulhrsw, psign, shift_simd_by_scalar, shift_simd_by_simd, + ShiftOp, horizontal_bin_op, mask_load, mask_store, mpsadbw, packssdw, packsswb, packusdw, + packuswb, pmulhrsw, psign, shift_simd_by_scalar, shift_simd_by_simd, }; use crate::*; @@ -25,29 +25,20 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.avx2.").unwrap(); match unprefixed_name { - // Used to implement the _mm256_abs_epi{8,16,32} functions. - // Calculates the absolute value of packed 8/16/32-bit integers. - "pabs.b" | "pabs.w" | "pabs.d" => { - let [op] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - - int_abs(this, op, dest)?; - } - // Used to implement the _mm256_h{add,adds,sub}_epi{16,32} functions. - // Horizontally add / add with saturation / subtract adjacent 16/32-bit + // Used to implement the _mm256_h{adds,subs}_epi16 functions. + // Horizontally add / subtract with saturation adjacent 16-bit // integer values in `left` and `right`. - "phadd.w" | "phadd.sw" | "phadd.d" | "phsub.w" | "phsub.sw" | "phsub.d" => { + "phadd.sw" | "phsub.sw" => { let [left, right] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - let (which, saturating) = match unprefixed_name { - "phadd.w" | "phadd.d" => (mir::BinOp::Add, false), - "phadd.sw" => (mir::BinOp::Add, true), - "phsub.w" | "phsub.d" => (mir::BinOp::Sub, false), - "phsub.sw" => (mir::BinOp::Sub, true), + let which = match unprefixed_name { + "phadd.sw" => mir::BinOp::Add, + "phsub.sw" => mir::BinOp::Sub, _ => unreachable!(), }; - horizontal_bin_op(this, which, saturating, left, right, dest)?; + horizontal_bin_op(this, which, /*saturating*/ true, left, right, dest)?; } // Used to implement `_mm{,_mask}_{i32,i64}gather_{epi32,epi64,pd,ps}` functions // Gathers elements from `slice` using `offsets * scale` as indices. @@ -110,42 +101,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(Scalar::from_int(0, dest.layout.size), &dest)?; } } - // Used to implement the _mm256_madd_epi16 function. - // Multiplies packed signed 16-bit integers in `left` and `right`, producing - // intermediate signed 32-bit integers. Horizontally add adjacent pairs of - // intermediate 32-bit integers, and pack the results in `dest`. - "pmadd.wd" => { - let [left, right] = - this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - - let (left, left_len) = this.project_to_simd(left)?; - let (right, right_len) = this.project_to_simd(right)?; - let (dest, dest_len) = this.project_to_simd(dest)?; - - assert_eq!(left_len, right_len); - assert_eq!(dest_len.strict_mul(2), left_len); - - for i in 0..dest_len { - let j1 = i.strict_mul(2); - let left1 = this.read_scalar(&this.project_index(&left, j1)?)?.to_i16()?; - let right1 = this.read_scalar(&this.project_index(&right, j1)?)?.to_i16()?; - - let j2 = j1.strict_add(1); - let left2 = this.read_scalar(&this.project_index(&left, j2)?)?.to_i16()?; - let right2 = this.read_scalar(&this.project_index(&right, j2)?)?.to_i16()?; - - let dest = this.project_index(&dest, i)?; - - // Multiplications are i16*i16->i32, which will not overflow. - let mul1 = i32::from(left1).strict_mul(right1.into()); - let mul2 = i32::from(left2).strict_mul(right2.into()); - // However, this addition can overflow in the most extreme case - // (-0x8000)*(-0x8000)+(-0x8000)*(-0x8000) = 0x80000000 - let res = mul1.wrapping_add(mul2); - - this.write_scalar(Scalar::from_i32(res), &dest)?; - } - } // Used to implement the _mm256_maddubs_epi16 function. // Multiplies packed 8-bit unsigned integers from `left` and packed // signed 8-bit integers from `right` into 16-bit signed integers. Then, @@ -285,39 +240,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.copy_op(&left, &dest)?; } } - // Used to implement the _mm256_permute2x128_si256 function. - // Shuffles 128-bit blocks of `a` and `b` using `imm` as pattern. - "vperm2i128" => { - let [left, right, imm] = - this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - - assert_eq!(left.layout.size.bits(), 256); - assert_eq!(right.layout.size.bits(), 256); - assert_eq!(dest.layout.size.bits(), 256); - - // Transmute to `[i128; 2]` - - let array_layout = - this.layout_of(Ty::new_array(this.tcx.tcx, this.tcx.types.i128, 2))?; - let left = left.transmute(array_layout, this)?; - let right = right.transmute(array_layout, this)?; - let dest = dest.transmute(array_layout, this)?; - - let imm = this.read_scalar(imm)?.to_u8()?; - - for i in 0..2 { - let dest = this.project_index(&dest, i)?; - let src = match (imm >> i.strict_mul(4)) & 0b11 { - 0 => this.project_index(&left, 0)?, - 1 => this.project_index(&left, 1)?, - 2 => this.project_index(&right, 0)?, - 3 => this.project_index(&right, 1)?, - _ => unreachable!(), - }; - - this.copy_op(&src, &dest)?; - } - } // Used to implement the _mm256_sad_epu8 function. // Compute the absolute differences of packed unsigned 8-bit integers // in `left` and `right`, then horizontally sum each consecutive 8 diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 91893737b060f..63d2b2d044b42 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/src/shims/x86/mod.rs @@ -59,28 +59,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_immediate(*sum, &this.project_field(dest, FieldIdx::ONE)?)?; } - // Used to implement the `_addcarryx_u{32, 64}` functions. They are semantically identical with the `_addcarry_u{32, 64}` functions, - // except for a slightly different type signature and the requirement for the "adx" target feature. - // https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/addcarryx-u32-addcarryx-u64.html - "addcarryx.u32" | "addcarryx.u64" => { - this.expect_target_feature_for_intrinsic(link_name, "adx")?; - - let is_u64 = unprefixed_name.ends_with("64"); - if is_u64 && this.tcx.sess.target.arch != Arch::X86_64 { - return interp_ok(EmulateItemResult::NotSupported); - } - let [c_in, a, b, out] = - this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - let out = this.deref_pointer_as( - out, - if is_u64 { this.machine.layouts.u64 } else { this.machine.layouts.u32 }, - )?; - - let (sum, c_out) = carrying_add(this, c_in, a, b, mir::BinOp::AddWithOverflow)?; - this.write_scalar(c_out, dest)?; - this.write_immediate(*sum, &out)?; - } - // Used to implement the `_mm_pause` function. // The intrinsic is used to hint the processor that the code is in a spin-loop. // It is compiled down to a `pause` instruction. When SSE2 is not available, @@ -719,36 +697,6 @@ fn convert_float_to_int<'tcx>( interp_ok(()) } -/// Calculates absolute value of integers in `op` and stores the result in `dest`. -/// -/// In case of overflow (when the operand is the minimum value), the operation -/// will wrap around. -fn int_abs<'tcx>( - ecx: &mut crate::MiriInterpCx<'tcx>, - op: &OpTy<'tcx>, - dest: &MPlaceTy<'tcx>, -) -> InterpResult<'tcx, ()> { - let (op, op_len) = ecx.project_to_simd(op)?; - let (dest, dest_len) = ecx.project_to_simd(dest)?; - - assert_eq!(op_len, dest_len); - - let zero = ImmTy::from_int(0, op.layout.field(ecx, 0)); - - for i in 0..dest_len { - let op = ecx.read_immediate(&ecx.project_index(&op, i)?)?; - let dest = ecx.project_index(&dest, i)?; - - let lt_zero = ecx.binary_op(mir::BinOp::Lt, &op, &zero)?; - let res = - if lt_zero.to_scalar().to_bool()? { ecx.unary_op(mir::UnOp::Neg, &op)? } else { op }; - - ecx.write_immediate(*res, &dest)?; - } - - interp_ok(()) -} - /// Splits `op` (which must be a SIMD vector) into 128-bit chunks. /// /// Returns a tuple where: diff --git a/src/tools/miri/src/shims/x86/sse.rs b/src/tools/miri/src/shims/x86/sse.rs index 6d8def5b53fca..309fbb61de5a7 100644 --- a/src/tools/miri/src/shims/x86/sse.rs +++ b/src/tools/miri/src/shims/x86/sse.rs @@ -180,29 +180,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_immediate(*res, dest)?; } - // Used to implement the _mm_cvtsi32_ss and _mm_cvtsi64_ss functions. - // Converts `right` from i32/i64 to f32. Returns a SIMD vector with - // the result in the first component and the remaining components - // are copied from `left`. - // https://www.felixcloutier.com/x86/cvtsi2ss - "cvtsi2ss" | "cvtsi642ss" => { - let [left, right] = - this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - - let (left, left_len) = this.project_to_simd(left)?; - let (dest, dest_len) = this.project_to_simd(dest)?; - - assert_eq!(dest_len, left_len); - - let right = this.read_immediate(right)?; - let dest0 = this.project_index(&dest, 0)?; - let res0 = this.int_to_int_or_float(&right, dest0.layout)?; - this.write_immediate(*res0, &dest0)?; - - for i in 1..dest_len { - this.copy_op(&this.project_index(&left, i)?, &this.project_index(&dest, i)?)?; - } - } _ => return interp_ok(EmulateItemResult::NotSupported), } interp_ok(EmulateItemResult::NeedsReturn) diff --git a/src/tools/miri/src/shims/x86/sse2.rs b/src/tools/miri/src/shims/x86/sse2.rs index 8f53adfb5ecf3..9af7f05d3b27e 100644 --- a/src/tools/miri/src/shims/x86/sse2.rs +++ b/src/tools/miri/src/shims/x86/sse2.rs @@ -36,42 +36,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Intrinsincs sufixed with "epiX" or "epuX" operate with X-bit signed or unsigned // vectors. match unprefixed_name { - // Used to implement the _mm_madd_epi16 function. - // Multiplies packed signed 16-bit integers in `left` and `right`, producing - // intermediate signed 32-bit integers. Horizontally add adjacent pairs of - // intermediate 32-bit integers, and pack the results in `dest`. - "pmadd.wd" => { - let [left, right] = - this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - - let (left, left_len) = this.project_to_simd(left)?; - let (right, right_len) = this.project_to_simd(right)?; - let (dest, dest_len) = this.project_to_simd(dest)?; - - assert_eq!(left_len, right_len); - assert_eq!(dest_len.strict_mul(2), left_len); - - for i in 0..dest_len { - let j1 = i.strict_mul(2); - let left1 = this.read_scalar(&this.project_index(&left, j1)?)?.to_i16()?; - let right1 = this.read_scalar(&this.project_index(&right, j1)?)?.to_i16()?; - - let j2 = j1.strict_add(1); - let left2 = this.read_scalar(&this.project_index(&left, j2)?)?.to_i16()?; - let right2 = this.read_scalar(&this.project_index(&right, j2)?)?.to_i16()?; - - let dest = this.project_index(&dest, i)?; - - // Multiplications are i16*i16->i32, which will not overflow. - let mul1 = i32::from(left1).strict_mul(right1.into()); - let mul2 = i32::from(left2).strict_mul(right2.into()); - // However, this addition can overflow in the most extreme case - // (-0x8000)*(-0x8000)+(-0x8000)*(-0x8000) = 0x80000000 - let res = mul1.wrapping_add(mul2); - - this.write_scalar(Scalar::from_i32(res), &dest)?; - } - } // Used to implement the _mm_sad_epu8 function. // Computes the absolute differences of packed unsigned 8-bit integers in `a` // and `b`, then horizontally sum each consecutive 8 differences to produce @@ -320,10 +284,10 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_immediate(*res, dest)?; } - // Used to implement the _mm_cvtsd_ss and _mm_cvtss_sd functions. - // Converts the first f64/f32 from `right` to f32/f64 and copies - // the remaining elements from `left` - "cvtsd2ss" | "cvtss2sd" => { + // Used to implement the _mm_cvtsd_ss function. + // Converts the first f64 from `right` to f32 and copies the remaining + // elements from `left` + "cvtsd2ss" => { let [left, right] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; @@ -336,8 +300,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Convert first element of `right` let right0 = this.read_immediate(&this.project_index(&right, 0)?)?; let dest0 = this.project_index(&dest, 0)?; - // `float_to_float_or_int` here will convert from f64 to f32 (cvtsd2ss) or - // from f32 to f64 (cvtss2sd). let res0 = this.float_to_float_or_int(&right0, dest0.layout)?; this.write_immediate(*res0, &dest0)?; diff --git a/src/tools/miri/src/shims/x86/sse3.rs b/src/tools/miri/src/shims/x86/sse3.rs index 0fd8c3bc389b0..17c8360d33998 100644 --- a/src/tools/miri/src/shims/x86/sse3.rs +++ b/src/tools/miri/src/shims/x86/sse3.rs @@ -1,10 +1,8 @@ use rustc_abi::CanonAbi; -use rustc_middle::mir; use rustc_middle::ty::Ty; use rustc_span::Symbol; use rustc_target::callconv::FnAbi; -use super::horizontal_bin_op; use crate::*; impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} @@ -22,21 +20,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.sse3.").unwrap(); match unprefixed_name { - // Used to implement the _mm_h{add,sub}_p{s,d} functions. - // Horizontally add/subtract adjacent floating point values - // in `left` and `right`. - "hadd.ps" | "hadd.pd" | "hsub.ps" | "hsub.pd" => { - let [left, right] = - this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - - let which = match unprefixed_name { - "hadd.ps" | "hadd.pd" => mir::BinOp::Add, - "hsub.ps" | "hsub.pd" => mir::BinOp::Sub, - _ => unreachable!(), - }; - - horizontal_bin_op(this, which, /*saturating*/ false, left, right, dest)?; - } // Used to implement the _mm_lddqu_si128 function. // Reads a 128-bit vector from an unaligned pointer. This intrinsic // is expected to perform better than a regular unaligned read when diff --git a/src/tools/miri/src/shims/x86/sse41.rs b/src/tools/miri/src/shims/x86/sse41.rs index 7736b5e443d0c..1e8b0f34428d1 100644 --- a/src/tools/miri/src/shims/x86/sse41.rs +++ b/src/tools/miri/src/shims/x86/sse41.rs @@ -157,20 +157,13 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { mpsadbw(this, left, right, imm, dest)?; } - // Used to implement the _mm_testz_si128, _mm_testc_si128 - // and _mm_testnzc_si128 functions. - // Tests `(op & mask) == 0`, `(op & mask) == mask` or - // `(op & mask) != 0 && (op & mask) != mask` - "ptestz" | "ptestc" | "ptestnzc" => { + // Used to implement the _mm_testnzc_si128 function. + // Tests `(op & mask) != 0 && (op & mask) != mask` + "ptestnzc" => { let [op, mask] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; let (all_zero, masked_set) = test_bits_masked(this, op, mask)?; - let res = match unprefixed_name { - "ptestz" => all_zero, - "ptestc" => masked_set, - "ptestnzc" => !all_zero && !masked_set, - _ => unreachable!(), - }; + let res = !all_zero && !masked_set; this.write_scalar(Scalar::from_i32(res.into()), dest)?; } diff --git a/src/tools/miri/src/shims/x86/ssse3.rs b/src/tools/miri/src/shims/x86/ssse3.rs index 52ad6bd441992..398f538e1ba03 100644 --- a/src/tools/miri/src/shims/x86/ssse3.rs +++ b/src/tools/miri/src/shims/x86/ssse3.rs @@ -4,7 +4,7 @@ use rustc_middle::ty::Ty; use rustc_span::Symbol; use rustc_target::callconv::FnAbi; -use super::{horizontal_bin_op, int_abs, pmulhrsw, psign}; +use super::{horizontal_bin_op, pmulhrsw, psign}; use crate::*; impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} @@ -22,13 +22,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.ssse3.").unwrap(); match unprefixed_name { - // Used to implement the _mm_abs_epi{8,16,32} functions. - // Calculates the absolute value of packed 8/16/32-bit integers. - "pabs.b.128" | "pabs.w.128" | "pabs.d.128" => { - let [op] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - - int_abs(this, op, dest)?; - } // Used to implement the _mm_shuffle_epi8 intrinsic. // Shuffles bytes from `left` using `right` as pattern. // https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_mm_shuffle_epi8 @@ -58,23 +51,20 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(res, &dest)?; } } - // Used to implement the _mm_h{add,adds,sub}_epi{16,32} functions. - // Horizontally add / add with saturation / subtract adjacent 16/32-bit + // Used to implement the _mm_h{adds,subs}_epi16 functions. + // Horizontally add / subtract with saturation adjacent 16-bit // integer values in `left` and `right`. - "phadd.w.128" | "phadd.sw.128" | "phadd.d.128" | "phsub.w.128" | "phsub.sw.128" - | "phsub.d.128" => { + "phadd.sw.128" | "phsub.sw.128" => { let [left, right] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - let (which, saturating) = match unprefixed_name { - "phadd.w.128" | "phadd.d.128" => (mir::BinOp::Add, false), - "phadd.sw.128" => (mir::BinOp::Add, true), - "phsub.w.128" | "phsub.d.128" => (mir::BinOp::Sub, false), - "phsub.sw.128" => (mir::BinOp::Sub, true), + let which = match unprefixed_name { + "phadd.sw.128" => mir::BinOp::Add, + "phsub.sw.128" => mir::BinOp::Sub, _ => unreachable!(), }; - horizontal_bin_op(this, which, saturating, left, right, dest)?; + horizontal_bin_op(this, which, /*saturating*/ true, left, right, dest)?; } // Used to implement the _mm_maddubs_epi16 function. // Multiplies packed 8-bit unsigned integers from `left` and packed From d59e58b744d8aaf6e729b4d9378d6c1a6aecfb5a Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 23 Oct 2025 18:06:50 +0000 Subject: [PATCH 05/14] Use the dummy codegen backend This allows getting rid of all the argument mangling hacks as well as the exported_non_generic_symbols override hack by doing cargo build instead of cargo check. This is also a prerequisite for using native jit mode support in miri that I hope to add to rustc in the future to be used with cg_clif too. --- src/tools/miri/cargo-miri/src/phases.rs | 77 ++----------------- src/tools/miri/cargo-miri/src/setup.rs | 2 +- src/tools/miri/src/bin/miri.rs | 40 ++++------ src/tools/miri/src/lib.rs | 1 + src/tools/miri/test-cargo-miri/Cargo.lock | 8 -- src/tools/miri/test-cargo-miri/Cargo.toml | 1 - .../miri/test-cargo-miri/cdylib/Cargo.toml | 12 --- .../miri/test-cargo-miri/cdylib/src/lib.rs | 6 -- .../run.local_crate.stdout.ref | 2 +- 9 files changed, 28 insertions(+), 121 deletions(-) delete mode 100644 src/tools/miri/test-cargo-miri/cdylib/Cargo.toml delete mode 100644 src/tools/miri/test-cargo-miri/cdylib/src/lib.rs diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 0716f4add9d0d..0f04397b72d22 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -52,18 +52,6 @@ fn show_version() { println!(); } -fn forward_patched_extern_arg(args: &mut impl Iterator, cmd: &mut Command) { - cmd.arg("--extern"); // always forward flag, but adjust filename: - let path = args.next().expect("`--extern` should be followed by a filename"); - if let Some(lib) = path.strip_suffix(".rlib") { - // If this is an rlib, make it an rmeta. - cmd.arg(format!("{lib}.rmeta")); - } else { - // Some other extern file (e.g. a `.so`). Forward unchanged. - cmd.arg(path); - } -} - pub fn phase_cargo_miri(mut args: impl Iterator) { // Require a subcommand before any flags. // We cannot know which of those flags take arguments and which do not, @@ -276,7 +264,7 @@ pub enum RustcPhase { Rustdoc, } -pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { +pub fn phase_rustc(args: impl Iterator, phase: RustcPhase) { /// Determines if we are being invoked (as rustc) to build a crate for /// the "target" architecture, in contrast to the "host" architecture. /// Host crates are for build scripts and proc macros and still need to @@ -444,7 +432,6 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { } let mut cmd = miri(); - let mut emit_link_hack = false; // Arguments are treated very differently depending on whether this crate is // for interpretation by Miri, or for use by a build script / proc macro. if target_crate { @@ -455,7 +442,7 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { } // Forward arguments, but patched. - let emit_flag = "--emit"; + // This hack helps bootstrap run standard library tests in Miri. The issue is as follows: // when running `cargo miri test` on libcore, cargo builds a local copy of core and makes it // a dependency of the integration test crate. This copy duplicates all the lang items, so @@ -471,30 +458,7 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { let replace_librs = env::var_os("MIRI_REPLACE_LIBRS_IF_NOT_TEST").is_some() && !runnable_crate && phase == RustcPhase::Build; - while let Some(arg) = args.next() { - // Patch `--emit`: remove "link" from "--emit" to make this a check-only build. - if let Some(val) = arg.strip_prefix(emit_flag) { - // Patch this argument. First, extract its value. - let val = - val.strip_prefix('=').expect("`cargo` should pass `--emit=X` as one argument"); - let mut val: Vec<_> = val.split(',').collect(); - // Now make sure "link" is not in there, but "metadata" is. - if let Some(i) = val.iter().position(|&s| s == "link") { - emit_link_hack = true; - val.remove(i); - if !val.contains(&"metadata") { - val.push("metadata"); - } - } - cmd.arg(format!("{emit_flag}={}", val.join(","))); - continue; - } - // Patch `--extern` filenames, since Cargo sometimes passes stub `.rlib` files: - // https://github.com/rust-lang/miri/issues/1705 - if arg == "--extern" { - forward_patched_extern_arg(&mut args, &mut cmd); - continue; - } + for arg in args { // If the REPLACE_LIBRS hack is enabled and we are building a `lib.rs` file, and a // `lib.miri.rs` file exists, then build that instead. if replace_librs { @@ -543,17 +507,6 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { eprintln!("[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate}"); } - // Create a stub .rlib file if "link" was requested by cargo. - // This is necessary to prevent cargo from doing rebuilds all the time. - if emit_link_hack { - for filename in out_filenames() { - if verbose > 0 { - eprintln!("[cargo-miri rustc] creating fake lib file at `{}`", filename.display()); - } - File::create(filename).expect("failed to create fake lib file"); - } - } - debug_cmd("[cargo-miri rustc]", verbose, &cmd); exec(cmd); } @@ -624,17 +577,11 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); } // Forward rustc arguments. - // We need to patch "--extern" filenames because we forced a check-only - // build without cargo knowing about that: replace `.rlib` suffix by - // `.rmeta`. - // We also need to remove `--error-format` as cargo specifies that to be JSON, + // We need to remove `--error-format` as cargo specifies that to be JSON, // but when we run here, cargo does not interpret the JSON any more. `--json` // then also needs to be dropped. - let mut args = info.args.iter(); - while let Some(arg) = args.next() { - if arg == "--extern" { - forward_patched_extern_arg(&mut (&mut args).cloned(), &mut cmd); - } else if let Some(suffix) = arg.strip_prefix("--error-format") { + for arg in &info.args { + if let Some(suffix) = arg.strip_prefix("--error-format") { assert!(suffix.starts_with('=')); // Drop this argument. } else if let Some(suffix) = arg.strip_prefix("--json") { @@ -668,7 +615,7 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner } } -pub fn phase_rustdoc(mut args: impl Iterator) { +pub fn phase_rustdoc(args: impl Iterator) { let verbose = env::var("MIRI_VERBOSE") .map_or(0, |verbose| verbose.parse().expect("verbosity flag must be an integer")); @@ -676,15 +623,7 @@ pub fn phase_rustdoc(mut args: impl Iterator) { // of the old value into MIRI_ORIG_RUSTDOC. So that's what we have to invoke now. let rustdoc = env::var("MIRI_ORIG_RUSTDOC").unwrap_or("rustdoc".to_string()); let mut cmd = Command::new(rustdoc); - - while let Some(arg) = args.next() { - if arg == "--extern" { - // Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files. - forward_patched_extern_arg(&mut args, &mut cmd); - } else { - cmd.arg(arg); - } - } + cmd.args(args); // Doctests of `proc-macro` crates (and their dependencies) are always built for the host, // so we are not able to run them in Miri. diff --git a/src/tools/miri/cargo-miri/src/setup.rs b/src/tools/miri/cargo-miri/src/setup.rs index e399f66fbc9cd..c7682093663ed 100644 --- a/src/tools/miri/cargo-miri/src/setup.rs +++ b/src/tools/miri/cargo-miri/src/setup.rs @@ -160,7 +160,7 @@ pub fn setup( // Do the build. let status = SysrootBuilder::new(&sysroot_dir, target) - .build_mode(BuildMode::Check) + .build_mode(BuildMode::Build) // not a real build, since we use dummy codegen .rustc_version(rustc_version.clone()) .sysroot_config(sysroot_config) .rustflags(rustflags) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 6ef6e340b3d92..920fc29481916 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -16,7 +16,6 @@ extern crate rustc_hir; extern crate rustc_hir_analysis; extern crate rustc_interface; extern crate rustc_log; -extern crate rustc_metadata; extern crate rustc_middle; extern crate rustc_session; extern crate rustc_span; @@ -26,10 +25,8 @@ mod log; use std::env; use std::num::NonZero; use std::ops::Range; -use std::path::PathBuf; use std::rc::Rc; use std::str::FromStr; -use std::sync::Arc; use std::sync::atomic::{AtomicI32, AtomicU32, Ordering}; use miri::{ @@ -51,10 +48,8 @@ use rustc_middle::middle::exported_symbols::{ use rustc_middle::query::LocalCrate; use rustc_middle::traits::{ObligationCause, ObligationCauseCode}; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_middle::util::Providers; use rustc_session::EarlyDiagCtxt; use rustc_session::config::{CrateType, ErrorOutputType, OptLevel}; -use rustc_session::search_paths::PathKind; use rustc_span::def_id::DefId; use crate::log::setup::{deinit_loggers, init_early_loggers, init_late_loggers}; @@ -126,21 +121,6 @@ fn entry_fn(tcx: TyCtxt<'_>) -> (DefId, MiriEntryFnType) { } impl rustc_driver::Callbacks for MiriCompilerCalls { - fn config(&mut self, config: &mut Config) { - config.override_queries = Some(|_, providers| { - providers.extern_queries.used_crate_source = |tcx, cnum| { - let mut providers = Providers::default(); - rustc_metadata::provide(&mut providers); - let mut crate_source = (providers.extern_queries.used_crate_source)(tcx, cnum); - // HACK: rustc will emit "crate ... required to be available in rlib format, but - // was not found in this form" errors once we use `tcx.dependency_formats()` if - // there's no rlib provided, so setting a dummy path here to workaround those errors. - Arc::make_mut(&mut crate_source).rlib = Some((PathBuf::new(), PathKind::All)); - crate_source - }; - }); - } - fn after_analysis<'tcx>( &mut self, _: &rustc_interface::interface::Compiler, @@ -253,12 +233,26 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls { #[allow(rustc::potential_query_instability)] // rustc_codegen_ssa (where this code is copied from) also allows this lint fn config(&mut self, config: &mut Config) { if config.opts.prints.is_empty() && self.target_crate { + #[allow(rustc::bad_opt_access)] // tcx does not exist yet + { + let any_crate_types = !config.opts.crate_types.is_empty(); + // Avoid warnings about unsupported crate types. + config + .opts + .crate_types + .retain(|&c| c == CrateType::Executable || c == CrateType::Rlib); + if any_crate_types { + // Assert that we didn't remove all crate types if any crate type was passed on + // the cli. Otherwise we might silently change what kind of crate we are building. + assert!(!config.opts.crate_types.is_empty()); + } + } + // Queries overridden here affect the data stored in `rmeta` files of dependencies, // which will be used later in non-`MIRI_BE_RUSTC` mode. config.override_queries = Some(|_, local_providers| { - // `exported_non_generic_symbols` and `reachable_non_generics` provided by rustc always returns - // an empty result if `tcx.sess.opts.output_types.should_codegen()` is false. - // In addition we need to add #[used] symbols to exported_symbols for `lookup_link_section`. + // We need to add #[used] symbols to exported_symbols for `lookup_link_section`. + // FIXME handle this somehow in rustc itself to avoid this hack. local_providers.exported_non_generic_symbols = |tcx, LocalCrate| { let reachable_set = tcx.with_stable_hashing_context(|hcx| { tcx.reachable_set(()).to_sorted(&hcx, true) diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index b756fbb901bc6..bd0f12fd1896c 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -165,6 +165,7 @@ pub use crate::shims::unwind::{CatchUnwindData, EvalContextExt as _}; /// Also disable the MIR pass that inserts an alignment check on every pointer dereference. Miri /// does that too, and with a better error message. pub const MIRI_DEFAULT_ARGS: &[&str] = &[ + "-Zcodegen-backend=dummy", "--cfg=miri", "-Zalways-encode-mir", "-Zextra-const-ub-checks", diff --git a/src/tools/miri/test-cargo-miri/Cargo.lock b/src/tools/miri/test-cargo-miri/Cargo.lock index 32119426184d4..dd81c3b03bd39 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.lock +++ b/src/tools/miri/test-cargo-miri/Cargo.lock @@ -27,7 +27,6 @@ dependencies = [ "autocfg", "byteorder 0.5.3", "byteorder 1.5.0", - "cdylib", "exported_symbol", "eyre", "issue_1567", @@ -38,13 +37,6 @@ dependencies = [ "proc_macro_crate", ] -[[package]] -name = "cdylib" -version = "0.1.0" -dependencies = [ - "byteorder 1.5.0", -] - [[package]] name = "exported_symbol" version = "0.1.0" diff --git a/src/tools/miri/test-cargo-miri/Cargo.toml b/src/tools/miri/test-cargo-miri/Cargo.toml index f5092a4748f31..3f08f802cf422 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.toml +++ b/src/tools/miri/test-cargo-miri/Cargo.toml @@ -10,7 +10,6 @@ edition = "2024" [dependencies] byteorder = "1.0" -cdylib = { path = "cdylib" } exported_symbol = { path = "exported-symbol" } proc_macro_crate = { path = "proc-macro-crate" } issue_1567 = { path = "issue-1567" } diff --git a/src/tools/miri/test-cargo-miri/cdylib/Cargo.toml b/src/tools/miri/test-cargo-miri/cdylib/Cargo.toml deleted file mode 100644 index 527602e0a888f..0000000000000 --- a/src/tools/miri/test-cargo-miri/cdylib/Cargo.toml +++ /dev/null @@ -1,12 +0,0 @@ -[package] -name = "cdylib" -version = "0.1.0" -authors = ["Miri Team"] -edition = "2018" - -[lib] -# cargo-miri used to handle `cdylib` crate-type specially (https://github.com/rust-lang/miri/pull/1577). -crate-type = ["cdylib"] - -[dependencies] -byteorder = "1.0" # to test dependencies of sub-crates diff --git a/src/tools/miri/test-cargo-miri/cdylib/src/lib.rs b/src/tools/miri/test-cargo-miri/cdylib/src/lib.rs deleted file mode 100644 index e47e588251e4e..0000000000000 --- a/src/tools/miri/test-cargo-miri/cdylib/src/lib.rs +++ /dev/null @@ -1,6 +0,0 @@ -use byteorder::{BigEndian, ByteOrder}; - -#[no_mangle] -extern "C" fn use_the_dependency() { - let _n = ::read_u64(&[1, 2, 3, 4, 5, 6, 7, 8]); -} diff --git a/src/tools/miri/test-cargo-miri/run.local_crate.stdout.ref b/src/tools/miri/test-cargo-miri/run.local_crate.stdout.ref index 1587de9ff3f87..60cd9d371ad6c 100644 --- a/src/tools/miri/test-cargo-miri/run.local_crate.stdout.ref +++ b/src/tools/miri/test-cargo-miri/run.local_crate.stdout.ref @@ -1 +1 @@ -subcrate,issue_1567,exported_symbol_dep,test_local_crate_detection,cargo_miri_test,cdylib,exported_symbol,issue_1691,issue_1705,issue_rust_86261,proc_macro_crate +subcrate,issue_1567,exported_symbol_dep,test_local_crate_detection,cargo_miri_test,exported_symbol,issue_1691,issue_1705,issue_rust_86261,proc_macro_crate From 7711eb9f1e1efae614eee7a044553db53a2daf04 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 6 Nov 2025 08:34:13 +0100 Subject: [PATCH 06/14] lazy_sync: ensure the cookie fits inside the primitive --- src/tools/miri/src/concurrency/sync.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index e4e7fb1d725fe..2ac68fa1eeb60 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -246,12 +246,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc)?; alloc_extra.sync.insert(offset, Box::new(data)); // Mark this as "initialized". + let init_cookie = Scalar::from_u32(LAZY_INIT_COOKIE); + assert!(init_offset + init_cookie.size() <= primitive.layout.size); let init_field = primitive.offset(init_offset, this.machine.layouts.u32, this)?; - this.write_scalar_atomic( - Scalar::from_u32(LAZY_INIT_COOKIE), - &init_field, - AtomicWriteOrd::Relaxed, - )?; + this.write_scalar_atomic(init_cookie, &init_field, AtomicWriteOrd::Relaxed)?; interp_ok(this.get_alloc_extra(alloc)?.get_sync::(offset).unwrap()) } @@ -278,6 +276,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // thread initializing. Needs to be an RMW operation to ensure we read the *latest* value. // So we just try to replace MUTEX_INIT_COOKIE with itself. let init_cookie = Scalar::from_u32(LAZY_INIT_COOKIE); + assert!(init_offset + init_cookie.size() <= primitive.layout.size); let init_field = primitive.offset(init_offset, this.machine.layouts.u32, this)?; let (_init, success) = this .atomic_compare_exchange_scalar( From b8eee76769c3f0b2059f222383be7cb5c8d55396 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 4 Nov 2025 20:50:24 +0100 Subject: [PATCH 07/14] ./miri run: verbose by default, add flag to be quiet --- src/tools/miri/miri-script/src/commands.rs | 12 ++++++------ src/tools/miri/miri-script/src/main.rs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index f1b5229312325..5f50598e203b5 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -112,8 +112,8 @@ impl Command { Command::Check { features, flags } => Self::check(features, flags), Command::Test { bless, target, coverage, features, flags } => Self::test(bless, target, coverage, features, flags), - Command::Run { dep, verbose, target, edition, features, flags } => - Self::run(dep, verbose, target, edition, features, flags), + Command::Run { dep, quiet, target, edition, features, flags } => + Self::run(dep, quiet, target, edition, features, flags), Command::Doc { features, flags } => Self::doc(features, flags), Command::Fmt { flags } => Self::fmt(flags), Command::Clippy { features, flags } => Self::clippy(features, flags), @@ -458,7 +458,7 @@ impl Command { fn run( dep: bool, - verbose: bool, + quiet: bool, target: Option, edition: Option, features: Vec, @@ -468,7 +468,7 @@ impl Command { // Preparation: get a sysroot, and get the miri binary. let miri_sysroot = - e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref(), &features)?; + e.build_miri_sysroot(/* quiet */ quiet, target.as_deref(), &features)?; let miri_bin = e .build_get_binary(".", &features) .context("failed to get filename of miri executable")?; @@ -492,7 +492,7 @@ impl Command { // Compute flags. let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default(); let miri_flags = flagsplit(&miri_flags); - let quiet_flag = if verbose { None } else { Some("--quiet") }; + let quiet_flag = if quiet { Some("--quiet") } else { None }; // Run Miri. // The basic command that executes the Miri driver. @@ -506,7 +506,7 @@ impl Command { } else { cmd!(e.sh, "{miri_bin}") }; - cmd.set_quiet(!verbose); + cmd.set_quiet(quiet); // Add Miri flags let mut cmd = cmd.args(&miri_flags).args(&early_flags).args(&flags); // For `--dep` we also need to set the target in the env var. diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs index 761ec5979fafe..e307014496886 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -78,9 +78,9 @@ pub enum Command { /// Build the program with the dependencies declared in `tests/deps/Cargo.toml`. #[arg(long)] dep: bool, - /// Show build progress. + /// Hide build progress. #[arg(long, short)] - verbose: bool, + quiet: bool, /// The cross-interpretation target. #[arg(long)] target: Option, From 384c05f81dafa2b737dece10a1c1e4515e574ed2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 7 Nov 2025 17:20:29 +0100 Subject: [PATCH 08/14] fix 'cargo miri setup' quietness --- src/tools/miri/miri-script/src/commands.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 5f50598e203b5..5a8bf76befd6e 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -57,7 +57,9 @@ impl MiriEnv { .arg("--") .args(&["miri", "setup", "--print-sysroot"]) .args(target_flag); - cmd.set_quiet(quiet); + if quiet { + cmd = cmd.arg("--quiet"); + } let output = cmd.read()?; self.sh.set_var("MIRI_SYSROOT", &output); Ok(output.into()) From f6b751e3dfbb19dd3f80e6a196d4c29595f89d88 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sat, 8 Nov 2025 04:52:53 +0000 Subject: [PATCH 09/14] Prepare for merging from rust-lang/rust This updates the rust-version file to ceb7df7e6f17c92c7d49f7e4f02df0e68bc9b38b. --- 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 5bdb8bd8369f7..1ce491a50eae4 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -401ae55427522984e4a89c37cff6562a4ddcf6b7 +ceb7df7e6f17c92c7d49f7e4f02df0e68bc9b38b From edc13e6b8e1d3da87ce0e71459f27de816715163 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 6 Nov 2025 12:59:50 +0100 Subject: [PATCH 10/14] pthread: replace INIT_COOKIE by sync object metadata that gets cleared on writes --- src/tools/miri/src/concurrency/sync.rs | 157 +++++++++++++++--- src/tools/miri/src/machine.rs | 28 +++- src/tools/miri/src/shims/unix/freebsd/sync.rs | 4 +- .../miri/src/shims/unix/linux_like/sync.rs | 4 +- src/tools/miri/src/shims/unix/macos/sync.rs | 6 +- src/tools/miri/src/shims/unix/sync.rs | 144 +++++++++------- src/tools/miri/src/shims/windows/sync.rs | 6 +- .../libc_pthread_cond_double_destroy.rs | 2 +- .../libc_pthread_cond_move.init.stderr | 2 +- .../concurrency/libc_pthread_cond_move.rs | 4 +- ...thread_cond_move.static_initializer.stderr | 2 +- .../libc_pthread_mutex_double_destroy.rs | 2 +- .../libc_pthread_mutex_move.init.stderr | 2 +- .../concurrency/libc_pthread_mutex_move.rs | 4 +- ...hread_mutex_move.static_initializer.stderr | 2 +- .../libc_pthread_mutex_overwrite.rs | 14 ++ .../libc_pthread_mutex_overwrite.stderr | 13 ++ .../libc_pthread_rwlock_double_destroy.rs | 2 +- .../concurrency/libx_pthread_rwlock_moved.rs | 2 +- .../libx_pthread_rwlock_moved.stderr | 2 +- .../miri/tests/pass-dep/libc/pthread-sync.rs | 53 +++--- 21 files changed, 326 insertions(+), 129 deletions(-) create mode 100644 src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_overwrite.rs create mode 100644 src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_overwrite.stderr diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 2ac68fa1eeb60..9197fe2756da0 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -1,3 +1,4 @@ +use std::any::Any; use std::cell::RefCell; use std::collections::VecDeque; use std::collections::hash_map::Entry; @@ -5,6 +6,7 @@ use std::default::Default; use std::ops::Not; use std::rc::Rc; use std::time::Duration; +use std::{fmt, iter}; use rustc_abi::Size; use rustc_data_structures::fx::FxHashMap; @@ -12,6 +14,29 @@ use rustc_data_structures::fx::FxHashMap; use super::vector_clock::VClock; use crate::*; +/// A trait for the synchronization metadata that can be attached to a memory location. +pub trait SyncObj: Any { + /// Determines whether this object's metadata shall be deleted when a write to its + /// location occurs. + fn delete_on_write(&self) -> bool { + false + } +} + +impl dyn SyncObj { + #[inline(always)] + pub fn downcast_ref(&self) -> Option<&T> { + let x: &dyn Any = self; + x.downcast_ref() + } +} + +impl fmt::Debug for dyn SyncObj { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("SyncObj").finish_non_exhaustive() + } +} + /// The mutex state. #[derive(Default, Debug)] struct Mutex { @@ -214,15 +239,15 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { impl<'tcx> AllocExtra<'tcx> { fn get_sync(&self, offset: Size) -> Option<&T> { - self.sync.get(&offset).and_then(|s| s.downcast_ref::()) + self.sync_objs.get(&offset).and_then(|s| s.downcast_ref::()) } } -/// We designate an `init`` field in all primitives. -/// If `init` is set to this, we consider the primitive initialized. +/// We designate an `init`` field in all synchronization objects. +/// If `init` is set to this, we consider the object initialized. pub const LAZY_INIT_COOKIE: u32 = 0xcafe_affe; -// Public interface to synchronization primitives. Please note that in most +// Public interface to synchronization objects. Please note that in most // cases, the function calls are infallible and it is the client's (shim // implementation's) responsibility to detect and deal with erroneous // situations. @@ -231,9 +256,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Helper for lazily initialized `alloc_extra.sync` data: /// this forces an immediate init. /// Return a reference to the data in the machine state. - fn lazy_sync_init<'a, T: 'static>( + fn lazy_sync_init<'a, T: SyncObj>( &'a mut self, - primitive: &MPlaceTy<'tcx>, + obj: &MPlaceTy<'tcx>, init_offset: Size, data: T, ) -> InterpResult<'tcx, &'a T> @@ -242,27 +267,28 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { { let this = self.eval_context_mut(); - let (alloc, offset, _) = this.ptr_get_alloc_id(primitive.ptr(), 0)?; - let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc)?; - alloc_extra.sync.insert(offset, Box::new(data)); + let (alloc, offset, _) = this.ptr_get_alloc_id(obj.ptr(), 0)?; // Mark this as "initialized". let init_cookie = Scalar::from_u32(LAZY_INIT_COOKIE); - assert!(init_offset + init_cookie.size() <= primitive.layout.size); - let init_field = primitive.offset(init_offset, this.machine.layouts.u32, this)?; + assert!(init_offset + init_cookie.size() <= obj.layout.size); + let init_field = obj.offset(init_offset, this.machine.layouts.u32, this)?; this.write_scalar_atomic(init_cookie, &init_field, AtomicWriteOrd::Relaxed)?; + // Insert sync obj, and return reference to it. + let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc)?; + alloc_extra.sync_objs.insert(offset, Box::new(data)); interp_ok(this.get_alloc_extra(alloc)?.get_sync::(offset).unwrap()) } /// Helper for lazily initialized `alloc_extra.sync` data: - /// Checks if the primitive is initialized: + /// Checks if the synchronization object is initialized: /// - If yes, fetches the data from `alloc_extra.sync`, or calls `missing_data` if that fails /// and stores that in `alloc_extra.sync`. - /// - Otherwise, calls `new_data` to initialize the primitive. + /// - Otherwise, calls `new_data` to initialize the object. /// /// Return a reference to the data in the machine state. - fn lazy_sync_get_data<'a, T: 'static>( + fn lazy_sync_get_data<'a, T: SyncObj>( &'a mut self, - primitive: &MPlaceTy<'tcx>, + obj: &MPlaceTy<'tcx>, init_offset: Size, missing_data: impl FnOnce() -> InterpResult<'tcx, T>, new_data: impl FnOnce(&mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>, @@ -276,8 +302,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // thread initializing. Needs to be an RMW operation to ensure we read the *latest* value. // So we just try to replace MUTEX_INIT_COOKIE with itself. let init_cookie = Scalar::from_u32(LAZY_INIT_COOKIE); - assert!(init_offset + init_cookie.size() <= primitive.layout.size); - let init_field = primitive.offset(init_offset, this.machine.layouts.u32, this)?; + assert!(init_offset + init_cookie.size() <= obj.layout.size); + let init_field = obj.offset(init_offset, this.machine.layouts.u32, this)?; let (_init, success) = this .atomic_compare_exchange_scalar( &init_field, @@ -290,27 +316,27 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { .to_scalar_pair(); if success.to_bool()? { - // If it is initialized, it must be found in the "sync primitive" table, + // If it is initialized, it must be found in the "sync obj" table, // or else it has been moved illegally. - let (alloc, offset, _) = this.ptr_get_alloc_id(primitive.ptr(), 0)?; + let (alloc, offset, _) = this.ptr_get_alloc_id(obj.ptr(), 0)?; let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc)?; // Due to borrow checker reasons, we have to do the lookup twice. if alloc_extra.get_sync::(offset).is_none() { let data = missing_data()?; - alloc_extra.sync.insert(offset, Box::new(data)); + alloc_extra.sync_objs.insert(offset, Box::new(data)); } interp_ok(alloc_extra.get_sync::(offset).unwrap()) } else { let data = new_data(this)?; - this.lazy_sync_init(primitive, init_offset, data) + this.lazy_sync_init(obj, init_offset, data) } } - /// Get the synchronization primitive associated with the given pointer, + /// Get the synchronization object associated with the given pointer, /// or initialize a new one. /// /// Return `None` if this pointer does not point to at least 1 byte of mutable memory. - fn get_sync_or_init<'a, T: 'static>( + fn get_sync_or_init<'a, T: SyncObj>( &'a mut self, ptr: Pointer, new: impl FnOnce(&'a mut MiriMachine<'tcx>) -> T, @@ -331,11 +357,94 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Due to borrow checker reasons, we have to do the lookup twice. if alloc_extra.get_sync::(offset).is_none() { let new = new(machine); - alloc_extra.sync.insert(offset, Box::new(new)); + alloc_extra.sync_objs.insert(offset, Box::new(new)); } Some(alloc_extra.get_sync::(offset).unwrap()) } + /// Helper for "immovable" synchronization objects: the expected protocol for these objects is + /// that they use a static initializer of `uninit_val`, and we set them to `init_val` upon + /// initialization. At that point we also register a synchronization object, which is expected + /// to have `delete_on_write() == true`. So in the future, if we still see the object, we know + /// the location must still contain `init_val`. If the object is copied somewhere, that will + /// show up as a non-`init_val` value without a synchronization object, which we can then use to + /// error. + /// + /// `new_meta_obj` gets invoked when there is not yet an initialization object. + /// It has to ensure that the in-memory representation indeed matches `uninit_val`. + fn get_immovable_sync_with_static_init<'a, T: SyncObj>( + &'a mut self, + obj: &MPlaceTy<'tcx>, + init_offset: Size, + uninit_val: u8, + init_val: u8, + new_meta_obj: impl FnOnce(&mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>, + ) -> InterpResult<'tcx, &'a T> + where + 'tcx: 'a, + { + let this = self.eval_context_mut(); + this.check_ptr_access(obj.ptr(), obj.layout.size, CheckInAllocMsg::Dereferenceable)?; + assert!(init_offset < obj.layout.size); // ensure our 1-byte flag fits + let init_field = obj.offset(init_offset, this.machine.layouts.u8, this)?; + + let (alloc, offset, _) = this.ptr_get_alloc_id(init_field.ptr(), 0)?; + let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc)?; + // Due to borrow checker reasons, we have to do the lookup twice. + if alloc_extra.get_sync::(offset).is_some() { + let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc).unwrap(); + return interp_ok(alloc_extra.get_sync::(offset).unwrap()); + } + + // There's no sync object there yet. Create one, and try a CAS for uninit_val to init_val. + let meta_obj = new_meta_obj(this)?; + let (_init, success) = this + .atomic_compare_exchange_scalar( + &init_field, + &ImmTy::from_scalar(Scalar::from_u8(uninit_val), this.machine.layouts.u8), + Scalar::from_u8(init_val), + AtomicRwOrd::Relaxed, + AtomicReadOrd::Relaxed, + /* can_fail_spuriously */ false, + )? + .to_scalar_pair(); + assert!(success.to_bool()?, "`new_meta_obj` should have ensured that this CAS succeeds."); + + let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc).unwrap(); + assert!(meta_obj.delete_on_write()); + alloc_extra.sync_objs.insert(offset, Box::new(meta_obj)); + interp_ok(alloc_extra.get_sync::(offset).unwrap()) + } + + /// Explicitly initializes an object that would usually be implicitly initialized with + /// `get_immovable_sync_with_static_init`. + fn init_immovable_sync<'a, T: SyncObj>( + &'a mut self, + obj: &MPlaceTy<'tcx>, + init_offset: Size, + init_val: u8, + new_meta_obj: T, + ) -> InterpResult<'tcx, Option<&'a T>> + where + 'tcx: 'a, + { + let this = self.eval_context_mut(); + this.check_ptr_access(obj.ptr(), obj.layout.size, CheckInAllocMsg::Dereferenceable)?; + assert!(init_offset < obj.layout.size); // ensure our 1-byte flag fits + let init_field = obj.offset(init_offset, this.machine.layouts.u8, this)?; + + // Zero the entire object, and then store `init_val` directly. + this.write_bytes_ptr(obj.ptr(), iter::repeat_n(0, obj.layout.size.bytes_usize()))?; + this.write_scalar(Scalar::from_u8(init_val), &init_field)?; + + // Create meta-level initialization object. + let (alloc, offset, _) = this.ptr_get_alloc_id(init_field.ptr(), 0)?; + let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc).unwrap(); + assert!(new_meta_obj.delete_on_write()); + alloc_extra.sync_objs.insert(offset, Box::new(new_meta_obj)); + interp_ok(Some(alloc_extra.get_sync::(offset).unwrap())) + } + /// Lock by setting the mutex owner and increasing the lock count. fn mutex_lock(&mut self, mutex_ref: &MutexRef) -> InterpResult<'tcx> { let this = self.eval_context_mut(); diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index e91b8d97ef729..07a9e497161c5 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1,9 +1,9 @@ //! Global machine state as well as implementation of the interpreter engine //! `Machine` trait. -use std::any::Any; use std::borrow::Cow; use std::cell::{Cell, RefCell}; +use std::collections::BTreeMap; use std::path::Path; use std::rc::Rc; use std::{fmt, process}; @@ -36,6 +36,7 @@ use rustc_target::spec::Arch; use crate::alloc_addresses::EvalContextExt; use crate::concurrency::cpu_affinity::{self, CpuAffinityMask}; use crate::concurrency::data_race::{self, NaReadType, NaWriteType}; +use crate::concurrency::sync::SyncObj; use crate::concurrency::{ AllocDataRaceHandler, GenmcCtx, GenmcEvalContextExt as _, GlobalDataRaceHandler, weak_memory, }; @@ -399,11 +400,11 @@ pub struct AllocExtra<'tcx> { /// if this allocation is leakable. The backtrace is not /// pruned yet; that should be done before printing it. pub backtrace: Option>>, - /// Synchronization primitives like to attach extra data to particular addresses. We store that + /// Synchronization objects like to attach extra data to particular addresses. We store that /// inside the relevant allocation, to ensure that everything is removed when the allocation is /// freed. /// This maps offsets to synchronization-primitive-specific data. - pub sync: FxHashMap>, + pub sync_objs: BTreeMap>, } // We need a `Clone` impl because the machine passes `Allocation` through `Cow`... @@ -416,7 +417,7 @@ impl<'tcx> Clone for AllocExtra<'tcx> { impl VisitProvenance for AllocExtra<'_> { fn visit_provenance(&self, visit: &mut VisitWith<'_>) { - let AllocExtra { borrow_tracker, data_race, backtrace: _, sync: _ } = self; + let AllocExtra { borrow_tracker, data_race, backtrace: _, sync_objs: _ } = self; borrow_tracker.visit_provenance(visit); data_race.visit_provenance(visit); @@ -991,7 +992,12 @@ impl<'tcx> MiriMachine<'tcx> { .insert(id, (ecx.machine.current_user_relevant_span(), None)); } - interp_ok(AllocExtra { borrow_tracker, data_race, backtrace, sync: FxHashMap::default() }) + interp_ok(AllocExtra { + borrow_tracker, + data_race, + backtrace, + sync_objs: BTreeMap::default(), + }) } } @@ -1581,6 +1587,18 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { if let Some(borrow_tracker) = &mut alloc_extra.borrow_tracker { borrow_tracker.before_memory_write(alloc_id, prov_extra, range, machine)?; } + // Delete sync objects that don't like writes. + // Most of the time, we can just skip this. + if !alloc_extra.sync_objs.is_empty() { + let to_delete = alloc_extra + .sync_objs + .range(range.start..range.end()) + .filter_map(|(offset, obj)| obj.delete_on_write().then_some(*offset)) + .collect::>(); + for offset in to_delete { + alloc_extra.sync_objs.remove(&offset); + } + } interp_ok(()) } diff --git a/src/tools/miri/src/shims/unix/freebsd/sync.rs b/src/tools/miri/src/shims/unix/freebsd/sync.rs index 13d30e05573af..bd1ee31553727 100644 --- a/src/tools/miri/src/shims/unix/freebsd/sync.rs +++ b/src/tools/miri/src/shims/unix/freebsd/sync.rs @@ -4,13 +4,15 @@ use core::time::Duration; use rustc_abi::FieldIdx; -use crate::concurrency::sync::FutexRef; +use crate::concurrency::sync::{FutexRef, SyncObj}; use crate::*; pub struct FreeBsdFutex { futex: FutexRef, } +impl SyncObj for FreeBsdFutex {} + /// Extended variant of the `timespec` struct. pub struct UmtxTime { timeout: Duration, diff --git a/src/tools/miri/src/shims/unix/linux_like/sync.rs b/src/tools/miri/src/shims/unix/linux_like/sync.rs index 5f032c52deeb7..8ff7fe0a4563b 100644 --- a/src/tools/miri/src/shims/unix/linux_like/sync.rs +++ b/src/tools/miri/src/shims/unix/linux_like/sync.rs @@ -1,4 +1,4 @@ -use crate::concurrency::sync::FutexRef; +use crate::concurrency::sync::{FutexRef, SyncObj}; use crate::shims::sig::check_min_vararg_count; use crate::*; @@ -6,6 +6,8 @@ struct LinuxFutex { futex: FutexRef, } +impl SyncObj for LinuxFutex {} + /// Implementation of the SYS_futex syscall. /// `args` is the arguments *including* the syscall number. pub fn futex<'tcx>( diff --git a/src/tools/miri/src/shims/unix/macos/sync.rs b/src/tools/miri/src/shims/unix/macos/sync.rs index c4ddff7805ed2..33af869373927 100644 --- a/src/tools/miri/src/shims/unix/macos/sync.rs +++ b/src/tools/miri/src/shims/unix/macos/sync.rs @@ -15,7 +15,7 @@ use std::time::Duration; use rustc_abi::Size; -use crate::concurrency::sync::FutexRef; +use crate::concurrency::sync::{FutexRef, SyncObj}; use crate::*; #[derive(Clone)] @@ -24,6 +24,8 @@ enum MacOsUnfairLock { Active { mutex_ref: MutexRef }, } +impl SyncObj for MacOsUnfairLock {} + pub enum MacOsFutexTimeout<'a, 'tcx> { None, Relative { clock_op: &'a OpTy<'tcx>, timeout_op: &'a OpTy<'tcx> }, @@ -44,6 +46,8 @@ struct MacOsFutex { shared: Cell, } +impl SyncObj for MacOsFutex {} + impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { fn os_unfair_lock_get_data<'a>( diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index a712279d57628..bb1b97ff8ad18 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -1,13 +1,11 @@ use rustc_abi::Size; -use crate::concurrency::sync::LAZY_INIT_COOKIE; +use crate::concurrency::sync::SyncObj; use crate::*; -/// Do a bytewise comparison of the two places, using relaxed atomic reads. This is used to check if +/// Do a bytewise comparison of the two places. This is used to check if /// a synchronization primitive matches its static initializer value. -/// -/// The reads happen in chunks of 4, so all racing accesses must also use that access size. -fn bytewise_equal_atomic_relaxed<'tcx>( +fn bytewise_equal<'tcx>( ecx: &MiriInterpCx<'tcx>, left: &MPlaceTy<'tcx>, right: &MPlaceTy<'tcx>, @@ -15,25 +13,16 @@ fn bytewise_equal_atomic_relaxed<'tcx>( let size = left.layout.size; assert_eq!(size, right.layout.size); - // We do this in chunks of 4, so that we are okay to race with (sufficiently aligned) - // 4-byte atomic accesses. - assert!(size.bytes().is_multiple_of(4)); - for i in 0..(size.bytes() / 4) { - let offset = Size::from_bytes(i.strict_mul(4)); - let load = |place: &MPlaceTy<'tcx>| { - let byte = place.offset(offset, ecx.machine.layouts.u32, ecx)?; - ecx.read_scalar_atomic(&byte, AtomicReadOrd::Relaxed)?.to_u32() - }; - let left = load(left)?; - let right = load(right)?; - if left != right { - return interp_ok(false); - } - } + let left_bytes = ecx.read_bytes_ptr_strip_provenance(left.ptr(), size)?; + let right_bytes = ecx.read_bytes_ptr_strip_provenance(right.ptr(), size)?; - interp_ok(true) + interp_ok(left_bytes == right_bytes) } +// The in-memory marker values we use to indicate whether objects have been initialized. +const PTHREAD_UNINIT: u8 = 0; +const PTHREAD_INIT: u8 = 1; + // # pthread_mutexattr_t // We store some data directly inside the type, ignoring the platform layout: // - kind: i32 @@ -103,7 +92,7 @@ fn mutexattr_translate_kind<'tcx>( // # pthread_mutex_t // We store some data directly inside the type, ignoring the platform layout: -// - init: u32 +// - init: u8 /// The mutex kind. #[derive(Debug, Clone, Copy)] @@ -120,6 +109,12 @@ struct PthreadMutex { kind: MutexKind, } +impl SyncObj for PthreadMutex { + fn delete_on_write(&self) -> bool { + true + } +} + /// To ensure an initialized mutex that was moved somewhere else can be distinguished from /// a statically initialized mutex that is used the first time, we pick some offset within /// `pthread_mutex_t` and use it as an "initialized" flag. @@ -138,11 +133,11 @@ fn mutex_init_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, Size> let check_static_initializer = |name| { let static_initializer = ecx.eval_path(&["libc", name]); let init_field = - static_initializer.offset(offset, ecx.machine.layouts.u32, ecx).unwrap(); - let init = ecx.read_scalar(&init_field).unwrap().to_u32().unwrap(); - assert_ne!( - init, LAZY_INIT_COOKIE, - "{name} is incompatible with our initialization cookie" + static_initializer.offset(offset, ecx.machine.layouts.u8, ecx).unwrap(); + let init = ecx.read_scalar(&init_field).unwrap().to_u8().unwrap(); + assert_eq!( + init, PTHREAD_UNINIT, + "{name} is incompatible with our initialization logic" ); }; @@ -172,7 +167,7 @@ fn mutex_create<'tcx>( ) -> InterpResult<'tcx, PthreadMutex> { let mutex = ecx.deref_pointer_as(mutex_ptr, ecx.libc_ty_layout("pthread_mutex_t"))?; let data = PthreadMutex { mutex_ref: MutexRef::new(), kind }; - ecx.lazy_sync_init(&mutex, mutex_init_offset(ecx)?, data.clone())?; + ecx.init_immovable_sync(&mutex, mutex_init_offset(ecx)?, PTHREAD_INIT, data.clone())?; interp_ok(data) } @@ -186,10 +181,11 @@ where 'tcx: 'a, { let mutex = ecx.deref_pointer_as(mutex_ptr, ecx.libc_ty_layout("pthread_mutex_t"))?; - ecx.lazy_sync_get_data( + ecx.get_immovable_sync_with_static_init( &mutex, mutex_init_offset(ecx)?, - || throw_ub_format!("`pthread_mutex_t` can't be moved after first use"), + PTHREAD_UNINIT, + PTHREAD_INIT, |ecx| { let kind = mutex_kind_from_static_initializer(ecx, &mutex)?; interp_ok(PthreadMutex { mutex_ref: MutexRef::new(), kind }) @@ -203,8 +199,7 @@ fn mutex_kind_from_static_initializer<'tcx>( mutex: &MPlaceTy<'tcx>, ) -> InterpResult<'tcx, MutexKind> { // All the static initializers recognized here *must* be checked in `mutex_init_offset`! - let is_initializer = - |name| bytewise_equal_atomic_relaxed(ecx, mutex, &ecx.eval_path(&["libc", name])); + let is_initializer = |name| bytewise_equal(ecx, mutex, &ecx.eval_path(&["libc", name])); // PTHREAD_MUTEX_INITIALIZER is recognized on all targets. if is_initializer("PTHREAD_MUTEX_INITIALIZER")? { @@ -220,18 +215,26 @@ fn mutex_kind_from_static_initializer<'tcx>( }, _ => {} } - throw_unsup_format!("unsupported static initializer used for `pthread_mutex_t`"); + throw_ub_format!( + "`pthread_mutex_t` was not properly initialized at this location, or it got overwritten" + ); } // # pthread_rwlock_t // We store some data directly inside the type, ignoring the platform layout: -// - init: u32 +// - init: u8 #[derive(Debug, Clone)] struct PthreadRwLock { rwlock_ref: RwLockRef, } +impl SyncObj for PthreadRwLock { + fn delete_on_write(&self) -> bool { + true + } +} + fn rwlock_init_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, Size> { let offset = match &*ecx.tcx.sess.target.os { "linux" | "illumos" | "solaris" | "freebsd" | "android" => 0, @@ -245,11 +248,11 @@ fn rwlock_init_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, Size // the `init` field must start out not equal to LAZY_INIT_COOKIE. if !ecx.machine.pthread_rwlock_sanity.replace(true) { let static_initializer = ecx.eval_path(&["libc", "PTHREAD_RWLOCK_INITIALIZER"]); - let init_field = static_initializer.offset(offset, ecx.machine.layouts.u32, ecx).unwrap(); - let init = ecx.read_scalar(&init_field).unwrap().to_u32().unwrap(); - assert_ne!( - init, LAZY_INIT_COOKIE, - "PTHREAD_RWLOCK_INITIALIZER is incompatible with our initialization cookie" + let init_field = static_initializer.offset(offset, ecx.machine.layouts.u8, ecx).unwrap(); + let init = ecx.read_scalar(&init_field).unwrap().to_u8().unwrap(); + assert_eq!( + init, PTHREAD_UNINIT, + "PTHREAD_RWLOCK_INITIALIZER is incompatible with our initialization logic" ); } @@ -264,17 +267,20 @@ where 'tcx: 'a, { let rwlock = ecx.deref_pointer_as(rwlock_ptr, ecx.libc_ty_layout("pthread_rwlock_t"))?; - ecx.lazy_sync_get_data( + ecx.get_immovable_sync_with_static_init( &rwlock, rwlock_init_offset(ecx)?, - || throw_ub_format!("`pthread_rwlock_t` can't be moved after first use"), + PTHREAD_UNINIT, + PTHREAD_INIT, |ecx| { - if !bytewise_equal_atomic_relaxed( + if !bytewise_equal( ecx, &rwlock, &ecx.eval_path(&["libc", "PTHREAD_RWLOCK_INITIALIZER"]), )? { - throw_unsup_format!("unsupported static initializer used for `pthread_rwlock_t`"); + throw_ub_format!( + "`pthread_rwlock_t` was not properly initialized at this location, or it got overwritten" + ); } interp_ok(PthreadRwLock { rwlock_ref: RwLockRef::new() }) }, @@ -322,7 +328,7 @@ fn condattr_set_clock_id<'tcx>( // # pthread_cond_t // We store some data directly inside the type, ignoring the platform layout: -// - init: u32 +// - init: u8 fn cond_init_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, Size> { let offset = match &*ecx.tcx.sess.target.os { @@ -337,11 +343,11 @@ fn cond_init_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, Size> // the `init` field must start out not equal to LAZY_INIT_COOKIE. if !ecx.machine.pthread_condvar_sanity.replace(true) { let static_initializer = ecx.eval_path(&["libc", "PTHREAD_COND_INITIALIZER"]); - let init_field = static_initializer.offset(offset, ecx.machine.layouts.u32, ecx).unwrap(); - let init = ecx.read_scalar(&init_field).unwrap().to_u32().unwrap(); - assert_ne!( - init, LAZY_INIT_COOKIE, - "PTHREAD_COND_INITIALIZER is incompatible with our initialization cookie" + let init_field = static_initializer.offset(offset, ecx.machine.layouts.u8, ecx).unwrap(); + let init = ecx.read_scalar(&init_field).unwrap().to_u8().unwrap(); + assert_eq!( + init, PTHREAD_UNINIT, + "PTHREAD_COND_INITIALIZER is incompatible with our initialization logic" ); } @@ -354,6 +360,12 @@ struct PthreadCondvar { clock: TimeoutClock, } +impl SyncObj for PthreadCondvar { + fn delete_on_write(&self) -> bool { + true + } +} + fn cond_create<'tcx>( ecx: &mut MiriInterpCx<'tcx>, cond_ptr: &OpTy<'tcx>, @@ -361,7 +373,7 @@ fn cond_create<'tcx>( ) -> InterpResult<'tcx, PthreadCondvar> { let cond = ecx.deref_pointer_as(cond_ptr, ecx.libc_ty_layout("pthread_cond_t"))?; let data = PthreadCondvar { condvar_ref: CondvarRef::new(), clock }; - ecx.lazy_sync_init(&cond, cond_init_offset(ecx)?, data.clone())?; + ecx.init_immovable_sync(&cond, cond_init_offset(ecx)?, PTHREAD_INIT, data.clone())?; interp_ok(data) } @@ -373,17 +385,20 @@ where 'tcx: 'a, { let cond = ecx.deref_pointer_as(cond_ptr, ecx.libc_ty_layout("pthread_cond_t"))?; - ecx.lazy_sync_get_data( + ecx.get_immovable_sync_with_static_init( &cond, cond_init_offset(ecx)?, - || throw_ub_format!("`pthread_cond_t` can't be moved after first use"), + PTHREAD_UNINIT, + PTHREAD_INIT, |ecx| { - if !bytewise_equal_atomic_relaxed( + if !bytewise_equal( ecx, &cond, &ecx.eval_path(&["libc", "PTHREAD_COND_INITIALIZER"]), )? { - throw_unsup_format!("unsupported static initializer used for `pthread_cond_t`"); + throw_ub_format!( + "`pthread_cond_t` was not properly initialized at this location, or it got overwritten" + ); } // This used the static initializer. The clock there is always CLOCK_REALTIME. interp_ok(PthreadCondvar { @@ -575,11 +590,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_ub_format!("destroyed a locked mutex"); } + // This write also deletes the interpreter state for this mutex. // This might lead to false positives, see comment in pthread_mutexattr_destroy - this.write_uninit( - &this.deref_pointer_as(mutex_op, this.libc_ty_layout("pthread_mutex_t"))?, - )?; - // FIXME: delete interpreter state associated with this mutex. + let mutex_place = + this.deref_pointer_as(mutex_op, this.libc_ty_layout("pthread_mutex_t"))?; + this.write_uninit(&mutex_place)?; interp_ok(()) } @@ -693,11 +708,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_ub_format!("destroyed a locked rwlock"); } + // This write also deletes the interpreter state for this rwlock. // This might lead to false positives, see comment in pthread_mutexattr_destroy - this.write_uninit( - &this.deref_pointer_as(rwlock_op, this.libc_ty_layout("pthread_rwlock_t"))?, - )?; - // FIXME: delete interpreter state associated with this rwlock. + let rwlock_place = + this.deref_pointer_as(rwlock_op, this.libc_ty_layout("pthread_rwlock_t"))?; + this.write_uninit(&rwlock_place)?; interp_ok(()) } @@ -889,9 +904,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_ub_format!("destroying an awaited conditional variable"); } + // This write also deletes the interpreter state for this mutex. // This might lead to false positives, see comment in pthread_mutexattr_destroy - this.write_uninit(&this.deref_pointer_as(cond_op, this.libc_ty_layout("pthread_cond_t"))?)?; - // FIXME: delete interpreter state associated with this condvar. + let cond_place = this.deref_pointer_as(cond_op, this.libc_ty_layout("pthread_cond_t"))?; + this.write_uninit(&cond_place)?; interp_ok(()) } diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index a893999ef8e52..72080c92b7cd8 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -3,7 +3,7 @@ use std::time::Duration; use rustc_abi::Size; use crate::concurrency::init_once::{EvalContextExt as _, InitOnceStatus}; -use crate::concurrency::sync::FutexRef; +use crate::concurrency::sync::{FutexRef, SyncObj}; use crate::*; #[derive(Clone)] @@ -11,10 +11,14 @@ struct WindowsInitOnce { init_once: InitOnceRef, } +impl SyncObj for WindowsInitOnce {} + struct WindowsFutex { futex: FutexRef, } +impl SyncObj for WindowsFutex {} + impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // Windows sync primitives are pointer sized. diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_double_destroy.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_double_destroy.rs index 5778765589de9..5d5ffded5c5c1 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_double_destroy.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_double_destroy.rs @@ -1,6 +1,6 @@ //@ignore-target: windows # No pthreads on Windows //@ normalize-stderr-test: "(\n)ALLOC \(.*\) \{\n(.*\n)*\}(\n)" -> "${1}ALLOC DUMP${3}" -//@ normalize-stderr-test: "\[0x[0-9a-z]..0x[0-9a-z]\]" -> "[0xX..0xY]" +//@ normalize-stderr-test: "\[0x[0-9a-z]+..0x[0-9a-z]+\]" -> "[0xX..0xY]" /// Test that destroying a pthread_cond twice fails, even without a check for number validity diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.init.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.init.stderr index 9a7f0bb79e5fa..f3f64a60a89bb 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.init.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.init.stderr @@ -1,4 +1,4 @@ -error: Undefined Behavior: `pthread_cond_t` can't be moved after first use +error: Undefined Behavior: `pthread_cond_t` was not properly initialized at this location, or it got overwritten --> tests/fail-dep/concurrency/libc_pthread_cond_move.rs:LL:CC | LL | libc::pthread_cond_destroy(cond2.as_mut_ptr()); diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.rs index 4db904ab5e224..ef20a53dd2f1b 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.rs @@ -18,7 +18,7 @@ fn check() { // move pthread_cond_t let mut cond2 = cond; - libc::pthread_cond_destroy(cond2.as_mut_ptr()); //~[init] ERROR: can't be moved after first use + libc::pthread_cond_destroy(cond2.as_mut_ptr()); //~[init] ERROR: not properly initialized } } @@ -32,6 +32,6 @@ fn check() { // move pthread_cond_t let mut cond2 = cond; - libc::pthread_cond_destroy(&mut cond2 as *mut _); //~[static_initializer] ERROR: can't be moved after first use + libc::pthread_cond_destroy(&mut cond2 as *mut _); //~[static_initializer] ERROR: not properly initialized } } diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.static_initializer.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.static_initializer.stderr index ee1fafcf7cb18..4056f7d9d41b5 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.static_initializer.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.static_initializer.stderr @@ -1,4 +1,4 @@ -error: Undefined Behavior: `pthread_cond_t` can't be moved after first use +error: Undefined Behavior: `pthread_cond_t` was not properly initialized at this location, or it got overwritten --> tests/fail-dep/concurrency/libc_pthread_cond_move.rs:LL:CC | LL | libc::pthread_cond_destroy(&mut cond2 as *mut _); diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_double_destroy.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_double_destroy.rs index f04fe8be6b38d..45d16e173d9a6 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_double_destroy.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_double_destroy.rs @@ -1,6 +1,6 @@ //@ignore-target: windows # No pthreads on Windows //@ normalize-stderr-test: "(\n)ALLOC \(.*\) \{\n(.*\n)*\}(\n)" -> "${1}ALLOC DUMP${3}" -//@ normalize-stderr-test: "\[0x[0-9a-z]..0x[0-9a-z]\]" -> "[0xX..0xY]" +//@ normalize-stderr-test: "\[0x[0-9a-z]+..0x[0-9a-z]+\]" -> "[0xX..0xY]" /// Test that destroying a pthread_mutex twice fails, even without a check for number validity diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.init.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.init.stderr index 2e8e411e186a7..a7cba0f00fe97 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.init.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.init.stderr @@ -1,4 +1,4 @@ -error: Undefined Behavior: `pthread_mutex_t` can't be moved after first use +error: Undefined Behavior: `pthread_mutex_t` was not properly initialized at this location, or it got overwritten --> tests/fail-dep/concurrency/libc_pthread_mutex_move.rs:LL:CC | LL | libc::pthread_mutex_lock(&mut m2 as *mut _); diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.rs index 6c1f967b2b03b..028c6ec34dc9e 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.rs @@ -12,7 +12,7 @@ fn check() { assert_eq!(libc::pthread_mutex_init(&mut m as *mut _, std::ptr::null()), 0); let mut m2 = m; // move the mutex - libc::pthread_mutex_lock(&mut m2 as *mut _); //~[init] ERROR: can't be moved after first use + libc::pthread_mutex_lock(&mut m2 as *mut _); //~[init] ERROR: not properly initialized } } @@ -23,6 +23,6 @@ fn check() { libc::pthread_mutex_lock(&mut m as *mut _); let mut m2 = m; // move the mutex - libc::pthread_mutex_unlock(&mut m2 as *mut _); //~[static_initializer] ERROR: can't be moved after first use + libc::pthread_mutex_unlock(&mut m2 as *mut _); //~[static_initializer] ERROR: not properly initialized } } diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.static_initializer.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.static_initializer.stderr index 4fd3bd52ae12b..71f71efa0d96b 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.static_initializer.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_move.static_initializer.stderr @@ -1,4 +1,4 @@ -error: Undefined Behavior: `pthread_mutex_t` can't be moved after first use +error: Undefined Behavior: `pthread_mutex_t` was not properly initialized at this location, or it got overwritten --> tests/fail-dep/concurrency/libc_pthread_mutex_move.rs:LL:CC | LL | libc::pthread_mutex_unlock(&mut m2 as *mut _); diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_overwrite.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_overwrite.rs new file mode 100644 index 0000000000000..95b934ab1b727 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_overwrite.rs @@ -0,0 +1,14 @@ +//@ignore-target: windows # No pthreads on Windows + +fn main() { + unsafe { + let mut m: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER; + libc::pthread_mutex_lock(&mut m as *mut _); + + // Overwrite the mutex with itself. This de-initializes it. + let copy = m; + m = copy; + + libc::pthread_mutex_unlock(&mut m as *mut _); //~ERROR: not properly initialized + } +} diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_overwrite.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_overwrite.stderr new file mode 100644 index 0000000000000..b44792285421c --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_overwrite.stderr @@ -0,0 +1,13 @@ +error: Undefined Behavior: `pthread_mutex_t` was not properly initialized at this location, or it got overwritten + --> tests/fail-dep/concurrency/libc_pthread_mutex_overwrite.rs:LL:CC + | +LL | libc::pthread_mutex_unlock(&mut m as *mut _); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = 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 + +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/concurrency/libc_pthread_rwlock_double_destroy.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_rwlock_double_destroy.rs index 720ba71d23834..3d59d8e399373 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_rwlock_double_destroy.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_rwlock_double_destroy.rs @@ -1,6 +1,6 @@ //@ignore-target: windows # No pthreads on Windows //@ normalize-stderr-test: "(\n)ALLOC \(.*\) \{\n(.*\n)*\}(\n)" -> "${1}ALLOC DUMP${3}" -//@ normalize-stderr-test: "\[0x[0-9a-z]..0x[0-9a-z]\]" -> "[0xX..0xY]" +//@ normalize-stderr-test: "\[0x[0-9a-z]+..0x[0-9a-z]+\]" -> "[0xX..0xY]" /// Test that destroying a pthread_rwlock twice fails, even without a check for number validity diff --git a/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.rs b/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.rs index 6af19b7df9b58..fe507f63ec4e4 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.rs @@ -9,6 +9,6 @@ fn main() { // Move rwlock let mut rw2 = rw; - libc::pthread_rwlock_unlock(&mut rw2 as *mut _); //~ ERROR: can't be moved after first use + libc::pthread_rwlock_unlock(&mut rw2 as *mut _); //~ ERROR: not properly initialized } } diff --git a/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.stderr b/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.stderr index e69da4de99ddf..0625708c256b0 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.stderr @@ -1,4 +1,4 @@ -error: Undefined Behavior: `pthread_rwlock_t` can't be moved after first use +error: Undefined Behavior: `pthread_rwlock_t` was not properly initialized at this location, or it got overwritten --> tests/fail-dep/concurrency/libx_pthread_rwlock_moved.rs:LL:CC | LL | libc::pthread_rwlock_unlock(&mut rw2 as *mut _); diff --git a/src/tools/miri/tests/pass-dep/libc/pthread-sync.rs b/src/tools/miri/tests/pass-dep/libc/pthread-sync.rs index 255944662940d..a79d0656d85c4 100644 --- a/src/tools/miri/tests/pass-dep/libc/pthread-sync.rs +++ b/src/tools/miri/tests/pass-dep/libc/pthread-sync.rs @@ -8,18 +8,21 @@ use std::mem::MaybeUninit; use std::{mem, ptr, thread}; fn main() { + test_mutex(); test_mutex_libc_init_recursive(); test_mutex_libc_init_normal(); test_mutex_libc_init_errorcheck(); - test_rwlock_libc_static_initializer(); #[cfg(target_os = "linux")] test_mutex_libc_static_initializer_recursive(); + #[cfg(target_os = "linux")] + test_mutex_libc_static_initializer_errorcheck(); + + test_cond(); + test_condattr(); - check_mutex(); - check_rwlock_write(); - check_rwlock_read_no_deadlock(); - check_cond(); - check_condattr(); + test_rwlock(); + test_rwlock_write(); + test_rwlock_read_no_deadlock(); } // We want to only use pthread APIs here for easier testing. @@ -107,8 +110,7 @@ fn test_mutex_libc_init_errorcheck() { } } -// Only linux provides PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP, -// libc for macOS just has the default PTHREAD_MUTEX_INITIALIZER. +// Only linux provides PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP. #[cfg(target_os = "linux")] fn test_mutex_libc_static_initializer_recursive() { let mutex = std::cell::UnsafeCell::new(libc::PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP); @@ -126,6 +128,22 @@ fn test_mutex_libc_static_initializer_recursive() { } } +// Only linux provides PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP. +#[cfg(target_os = "linux")] +fn test_mutex_libc_static_initializer_errorcheck() { + let mutex = std::cell::UnsafeCell::new(libc::PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP); + unsafe { + assert_eq!(libc::pthread_mutex_lock(mutex.get()), 0); + assert_eq!(libc::pthread_mutex_trylock(mutex.get()), libc::EBUSY); + assert_eq!(libc::pthread_mutex_lock(mutex.get()), libc::EDEADLK); + assert_eq!(libc::pthread_mutex_unlock(mutex.get()), 0); + assert_eq!(libc::pthread_mutex_trylock(mutex.get()), 0); + assert_eq!(libc::pthread_mutex_unlock(mutex.get()), 0); + assert_eq!(libc::pthread_mutex_unlock(mutex.get()), libc::EPERM); + assert_eq!(libc::pthread_mutex_destroy(mutex.get()), 0); + } +} + struct SendPtr { ptr: *mut T, } @@ -137,7 +155,7 @@ impl Clone for SendPtr { } } -fn check_mutex() { +fn test_mutex() { let bomb = AbortOnDrop; // Specifically *not* using `Arc` to make sure there is no synchronization apart from the mutex. unsafe { @@ -168,7 +186,7 @@ fn check_mutex() { bomb.defuse(); } -fn check_rwlock_write() { +fn test_rwlock_write() { let bomb = AbortOnDrop; unsafe { let data = SyncUnsafeCell::new((libc::PTHREAD_RWLOCK_INITIALIZER, 0)); @@ -209,7 +227,7 @@ fn check_rwlock_write() { bomb.defuse(); } -fn check_rwlock_read_no_deadlock() { +fn test_rwlock_read_no_deadlock() { let bomb = AbortOnDrop; unsafe { let l1 = SyncUnsafeCell::new(libc::PTHREAD_RWLOCK_INITIALIZER); @@ -237,12 +255,11 @@ fn check_rwlock_read_no_deadlock() { bomb.defuse(); } -fn check_cond() { +fn test_cond() { let bomb = AbortOnDrop; unsafe { - let mut cond: MaybeUninit = MaybeUninit::uninit(); - assert_eq!(libc::pthread_cond_init(cond.as_mut_ptr(), ptr::null()), 0); - let cond = SendPtr { ptr: cond.as_mut_ptr() }; + let mut cond: libc::pthread_cond_t = libc::PTHREAD_COND_INITIALIZER; + let cond = SendPtr { ptr: &mut cond }; let mut mutex: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER; let mutex = SendPtr { ptr: &mut mutex }; @@ -286,7 +303,7 @@ fn check_cond() { bomb.defuse(); } -fn check_condattr() { +fn test_condattr() { unsafe { // Just smoke-testing that these functions can be called. let mut attr: MaybeUninit = MaybeUninit::uninit(); @@ -311,9 +328,7 @@ fn check_condattr() { } } -// std::sync::RwLock does not even used pthread_rwlock any more. -// Do some smoke testing of the API surface. -fn test_rwlock_libc_static_initializer() { +fn test_rwlock() { let rw = std::cell::UnsafeCell::new(libc::PTHREAD_RWLOCK_INITIALIZER); unsafe { assert_eq!(libc::pthread_rwlock_rdlock(rw.get()), 0); From ba789c6eb54a655bb8aa3e40ff7fba397aa33d7b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 7 Nov 2025 13:58:02 +0100 Subject: [PATCH 11/14] also use new sync obj metadata scheme for INIT_ONCE and os_unfair_lock --- src/tools/miri/src/concurrency/sync.rs | 18 ++++++- src/tools/miri/src/shims/unix/macos/sync.rs | 51 ++++++++++++------- src/tools/miri/src/shims/windows/sync.rs | 26 +++++++--- .../concurrency/apple-os-unfair-lock.rs | 12 ++--- 4 files changed, 73 insertions(+), 34 deletions(-) diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 9197fe2756da0..81c81b02de9b0 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -372,6 +372,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// /// `new_meta_obj` gets invoked when there is not yet an initialization object. /// It has to ensure that the in-memory representation indeed matches `uninit_val`. + /// + /// The point of storing an `init_val` is so that if this memory gets copied somewhere else, + /// it does not look like the static initializer (i.e., `uninit_val`) any more. For some + /// objects we could just entirely forbid reading their bytes to ensure they don't get copied, + /// but that does not work for objects without a destructor (Windows `InitOnce`, macOS + /// `os_unfair_lock`). fn get_immovable_sync_with_static_init<'a, T: SyncObj>( &'a mut self, obj: &MPlaceTy<'tcx>, @@ -383,6 +389,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { where 'tcx: 'a, { + assert!(init_val != uninit_val); let this = self.eval_context_mut(); this.check_ptr_access(obj.ptr(), obj.layout.size, CheckInAllocMsg::Dereferenceable)?; assert!(init_offset < obj.layout.size); // ensure our 1-byte flag fits @@ -398,7 +405,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // There's no sync object there yet. Create one, and try a CAS for uninit_val to init_val. let meta_obj = new_meta_obj(this)?; - let (_init, success) = this + let (old_init, success) = this .atomic_compare_exchange_scalar( &init_field, &ImmTy::from_scalar(Scalar::from_u8(uninit_val), this.machine.layouts.u8), @@ -408,7 +415,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /* can_fail_spuriously */ false, )? .to_scalar_pair(); - assert!(success.to_bool()?, "`new_meta_obj` should have ensured that this CAS succeeds."); + if !success.to_bool()? { + // This can happen for the macOS lock if it is already marked as initialized. + assert_eq!( + old_init.to_u8()?, + init_val, + "`new_meta_obj` should have ensured that this CAS succeeds" + ); + } let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc).unwrap(); assert!(meta_obj.delete_on_write()); diff --git a/src/tools/miri/src/shims/unix/macos/sync.rs b/src/tools/miri/src/shims/unix/macos/sync.rs index 33af869373927..4024ddc097316 100644 --- a/src/tools/miri/src/shims/unix/macos/sync.rs +++ b/src/tools/miri/src/shims/unix/macos/sync.rs @@ -13,18 +13,22 @@ use std::cell::Cell; use std::time::Duration; -use rustc_abi::Size; +use rustc_abi::{Endian, FieldIdx, Size}; use crate::concurrency::sync::{FutexRef, SyncObj}; use crate::*; #[derive(Clone)] enum MacOsUnfairLock { - Poisoned, Active { mutex_ref: MutexRef }, + PermanentlyLocked, } -impl SyncObj for MacOsUnfairLock {} +impl SyncObj for MacOsUnfairLock { + fn delete_on_write(&self) -> bool { + true + } +} pub enum MacOsFutexTimeout<'a, 'tcx> { None, @@ -57,22 +61,35 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { where 'tcx: 'a, { + // `os_unfair_lock_s` wraps a single `u32` field. We use the first byte to store the "init" + // flag. Due to macOS always being little endian, that's the least significant byte. let this = self.eval_context_mut(); + assert!(this.tcx.data_layout.endian == Endian::Little); + let lock = this.deref_pointer_as(lock_ptr, this.libc_ty_layout("os_unfair_lock_s"))?; - this.lazy_sync_get_data( + this.get_immovable_sync_with_static_init( &lock, Size::ZERO, // offset for init tracking - || { - // If we get here, due to how we reset things to zero in `os_unfair_lock_unlock`, - // this means the lock was moved while locked. This can happen with a `std` lock, - // but then any future attempt to unlock will just deadlock. In practice, terrible - // things can probably happen if you swap two locked locks, since they'd wake up - // from the wrong queue... we just won't catch all UB of this library API then (we - // would need to store some unique identifer in-memory for this, instead of a static - // LAZY_INIT_COOKIE). This can't be hit via `std::sync::Mutex`. - interp_ok(MacOsUnfairLock::Poisoned) + /* uninit_val */ 0, + /* init_val */ 1, + |this| { + let field = this.project_field(&lock, FieldIdx::from_u32(0))?; + let val = this.read_scalar(&field)?.to_u32()?; + if val == 0 { + interp_ok(MacOsUnfairLock::Active { mutex_ref: MutexRef::new() }) + } else if val == 1 { + // This is a lock that got copied while it is initialized. We de-initialize + // locks when they get released, so it got copied while locked. Unfortunately + // that is something `std` needs to support (the guard could have been leaked). + // So we behave like a futex-based lock whose wait queue got pruned: any attempt + // to acquire the lock will just wait forever. + // In practice there actually could be a wait queue there, if someone moves a + // lock *while threads are queued*; this is UB we will not detect. + interp_ok(MacOsUnfairLock::PermanentlyLocked) + } else { + throw_ub_format!("`os_unfair_lock` was not properly initialized at this location, or it got overwritten"); + } }, - |_| interp_ok(MacOsUnfairLock::Active { mutex_ref: MutexRef::new() }), ) } } @@ -336,7 +353,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // The lock is poisoned, who knows who owns it... we'll pretend: someone else. + // A perma-locked lock is definitely not held by us. throw_machine_stop!(TerminationInfo::Abort( "attempted to unlock an os_unfair_lock not owned by the current thread".to_owned() )); @@ -365,7 +382,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // The lock is poisoned, who knows who owns it... we'll pretend: someone else. + // A perma-locked lock is definitely not held by us. throw_machine_stop!(TerminationInfo::Abort( "called os_unfair_lock_assert_owner on an os_unfair_lock not owned by the current thread".to_owned() )); @@ -387,7 +404,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // The lock is poisoned, who knows who owns it... we'll pretend: someone else. + // A perma-locked lock is definitely not held by us. return interp_ok(()); }; let mutex_ref = mutex_ref.clone(); diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 72080c92b7cd8..43d9ba0043a34 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -1,6 +1,6 @@ use std::time::Duration; -use rustc_abi::Size; +use rustc_abi::{FieldIdx, Size}; use crate::concurrency::init_once::{EvalContextExt as _, InitOnceStatus}; use crate::concurrency::sync::{FutexRef, SyncObj}; @@ -11,7 +11,11 @@ struct WindowsInitOnce { init_once: InitOnceRef, } -impl SyncObj for WindowsInitOnce {} +impl SyncObj for WindowsInitOnce { + fn delete_on_write(&self) -> bool { + true + } +} struct WindowsFutex { futex: FutexRef, @@ -22,7 +26,7 @@ impl SyncObj for WindowsFutex {} impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // Windows sync primitives are pointer sized. - // We only use the first 4 bytes for the id. + // We only use the first byte for the "init" flag. fn init_once_get_data<'a>( &'a mut self, @@ -37,13 +41,19 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { this.deref_pointer_as(init_once_ptr, this.windows_ty_layout("INIT_ONCE"))?; let init_offset = Size::ZERO; - this.lazy_sync_get_data( + this.get_immovable_sync_with_static_init( &init_once, init_offset, - || throw_ub_format!("`INIT_ONCE` can't be moved after first use"), - |_| { - // TODO: check that this is still all-zero. - interp_ok(WindowsInitOnce { init_once: InitOnceRef::new() }) + /* uninit_val */ 0, + /* init_val */ 1, + |this| { + let ptr_field = this.project_field(&init_once, FieldIdx::from_u32(0))?; + let val = this.read_target_usize(&ptr_field)?; + if val == 0 { + interp_ok(WindowsInitOnce { init_once: InitOnceRef::new() }) + } else { + throw_ub_format!("`INIT_ONCE` was not properly initialized at this location, or it got overwritten"); + } }, ) } diff --git a/src/tools/miri/tests/pass-dep/concurrency/apple-os-unfair-lock.rs b/src/tools/miri/tests/pass-dep/concurrency/apple-os-unfair-lock.rs index f5b64474f83b6..05765fc3f30d4 100644 --- a/src/tools/miri/tests/pass-dep/concurrency/apple-os-unfair-lock.rs +++ b/src/tools/miri/tests/pass-dep/concurrency/apple-os-unfair-lock.rs @@ -14,12 +14,10 @@ fn main() { libc::os_unfair_lock_assert_not_owner(lock.get()); } - // `os_unfair_lock`s can be moved and leaked. - // In the real implementation, even moving it while locked is possible - // (and "forks" the lock, i.e. old and new location have independent wait queues). - // We only test the somewhat sane case of moving while unlocked that `std` plans to rely on. + // `os_unfair_lock`s can be moved, and even acquired again then. let lock = lock; - let locked = unsafe { libc::os_unfair_lock_trylock(lock.get()) }; - assert!(locked); - let _lock = lock; + assert!(unsafe { libc::os_unfair_lock_trylock(lock.get()) }); + // We can even move it while locked, but then we cannot acquire it any more. + let lock = lock; + assert!(!unsafe { libc::os_unfair_lock_trylock(lock.get()) }); } From 155b09f2b4f97e810b7063b6ced1ac02dc6370c9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 7 Nov 2025 14:34:23 +0100 Subject: [PATCH 12/14] throw an error if a synchronization object is read/written while threads are queued --- src/tools/miri/src/borrow_tracker/mod.rs | 9 ---- src/tools/miri/src/concurrency/init_once.rs | 4 ++ src/tools/miri/src/concurrency/sync.rs | 18 +++++++- src/tools/miri/src/helpers.rs | 11 ++++- src/tools/miri/src/machine.rs | 17 +++++--- src/tools/miri/src/shims/unix/macos/sync.rs | 19 +++++++-- src/tools/miri/src/shims/unix/sync.rs | 29 ++++++++++++- src/tools/miri/src/shims/windows/sync.rs | 9 ++++ .../apple_os_unfair_lock_move_with_queue.rs | 29 +++++++++++++ ...pple_os_unfair_lock_move_with_queue.stderr | 13 ++++++ .../libc_pthread_mutex_read_while_queued.rs | 41 +++++++++++++++++++ ...ibc_pthread_mutex_read_while_queued.stderr | 13 ++++++ .../libc_pthread_mutex_write_while_queued.rs | 41 +++++++++++++++++++ ...bc_pthread_mutex_write_while_queued.stderr | 13 ++++++ 14 files changed, 244 insertions(+), 22 deletions(-) create mode 100644 src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs create mode 100644 src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.stderr create mode 100644 src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.rs create mode 100644 src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.stderr create mode 100644 src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.rs create mode 100644 src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.stderr diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 89bd93edae127..ef137349abb12 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -115,15 +115,6 @@ impl VisitProvenance for GlobalStateInner { /// We need interior mutable access to the global state. pub type GlobalState = RefCell; -impl fmt::Display for AccessKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - AccessKind::Read => write!(f, "read access"), - AccessKind::Write => write!(f, "write access"), - } - } -} - /// Policy on whether to recurse into fields to retag #[derive(Copy, Clone, Debug)] pub enum RetagFields { diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index daea20b3779b2..5c3541ffbe4c1 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -57,6 +57,10 @@ impl InitOnceRef { pub fn begin(&self) { self.0.borrow_mut().begin(); } + + pub fn queue_is_empty(&self) -> bool { + self.0.borrow().waiters.is_empty() + } } impl VisitProvenance for InitOnceRef { diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 81c81b02de9b0..ac3ee3f91c5dc 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -16,6 +16,11 @@ use crate::*; /// A trait for the synchronization metadata that can be attached to a memory location. pub trait SyncObj: Any { + /// Determines whether reads/writes to this object's location are currently permitted. + fn on_access<'tcx>(&self, _access_kind: AccessKind) -> InterpResult<'tcx> { + interp_ok(()) + } + /// Determines whether this object's metadata shall be deleted when a write to its /// location occurs. fn delete_on_write(&self) -> bool { @@ -62,6 +67,10 @@ impl MutexRef { pub fn owner(&self) -> Option { self.0.borrow().owner } + + pub fn queue_is_empty(&self) -> bool { + self.0.borrow().queue.is_empty() + } } impl VisitProvenance for MutexRef { @@ -138,6 +147,11 @@ impl RwLockRef { pub fn is_write_locked(&self) -> bool { self.0.borrow().is_write_locked() } + + pub fn queue_is_empty(&self) -> bool { + let inner = self.0.borrow(); + inner.reader_queue.is_empty() && inner.writer_queue.is_empty() + } } impl VisitProvenance for RwLockRef { @@ -165,8 +179,8 @@ impl CondvarRef { Self(Default::default()) } - pub fn is_awaited(&self) -> bool { - !self.0.borrow().waiters.is_empty() + pub fn queue_is_empty(&self) -> bool { + self.0.borrow().waiters.is_empty() } } diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 7e1fdfa8cdf26..d6cef032b4e7e 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -1,7 +1,7 @@ use std::num::NonZero; use std::sync::Mutex; use std::time::Duration; -use std::{cmp, iter}; +use std::{cmp, fmt, iter}; use rand::RngCore; use rustc_abi::{Align, ExternAbi, FieldIdx, FieldsShape, Size, Variants}; @@ -29,6 +29,15 @@ pub enum AccessKind { Write, } +impl fmt::Display for AccessKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + AccessKind::Read => write!(f, "read access"), + AccessKind::Write => write!(f, "write access"), + } + } +} + /// Gets an instance for a path. /// /// A `None` namespace indicates we are looking for a module. diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 07a9e497161c5..7eff006f418db 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1547,6 +1547,11 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { if let Some(borrow_tracker) = &alloc_extra.borrow_tracker { borrow_tracker.before_memory_read(alloc_id, prov_extra, range, machine)?; } + // Check if there are any sync objects that would like to prevent reading this memory. + for (_offset, obj) in alloc_extra.sync_objs.range(range.start..range.end()) { + obj.on_access(AccessKind::Read)?; + } + interp_ok(()) } @@ -1590,11 +1595,13 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { // Delete sync objects that don't like writes. // Most of the time, we can just skip this. if !alloc_extra.sync_objs.is_empty() { - let to_delete = alloc_extra - .sync_objs - .range(range.start..range.end()) - .filter_map(|(offset, obj)| obj.delete_on_write().then_some(*offset)) - .collect::>(); + let mut to_delete = vec![]; + for (offset, obj) in alloc_extra.sync_objs.range(range.start..range.end()) { + obj.on_access(AccessKind::Write)?; + if obj.delete_on_write() { + to_delete.push(*offset); + } + } for offset in to_delete { alloc_extra.sync_objs.remove(&offset); } diff --git a/src/tools/miri/src/shims/unix/macos/sync.rs b/src/tools/miri/src/shims/unix/macos/sync.rs index 4024ddc097316..da87244118a52 100644 --- a/src/tools/miri/src/shims/unix/macos/sync.rs +++ b/src/tools/miri/src/shims/unix/macos/sync.rs @@ -25,6 +25,17 @@ enum MacOsUnfairLock { } impl SyncObj for MacOsUnfairLock { + fn on_access<'tcx>(&self, access_kind: AccessKind) -> InterpResult<'tcx> { + if let MacOsUnfairLock::Active { mutex_ref } = self + && !mutex_ref.queue_is_empty() + { + throw_ub_format!( + "{access_kind} to `os_unfair_lock` is forbidden while the queue is non-empty" + ); + } + interp_ok(()) + } + fn delete_on_write(&self) -> bool { true } @@ -81,10 +92,10 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // This is a lock that got copied while it is initialized. We de-initialize // locks when they get released, so it got copied while locked. Unfortunately // that is something `std` needs to support (the guard could have been leaked). - // So we behave like a futex-based lock whose wait queue got pruned: any attempt - // to acquire the lock will just wait forever. - // In practice there actually could be a wait queue there, if someone moves a - // lock *while threads are queued*; this is UB we will not detect. + // On the plus side, we know nobody was queued for the lock while it got copied; + // that would have been rejected by our `on_access`. So we behave like a + // futex-based lock would in this case: any attempt to acquire the lock will + // just wait forever, since there's nobody to wake us up. interp_ok(MacOsUnfairLock::PermanentlyLocked) } else { throw_ub_format!("`os_unfair_lock` was not properly initialized at this location, or it got overwritten"); diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index bb1b97ff8ad18..189b475ad8976 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -110,6 +110,15 @@ struct PthreadMutex { } impl SyncObj for PthreadMutex { + fn on_access<'tcx>(&self, access_kind: AccessKind) -> InterpResult<'tcx> { + if !self.mutex_ref.queue_is_empty() { + throw_ub_format!( + "{access_kind} to `pthread_mutex_t` is forbidden while the queue is non-empty" + ); + } + interp_ok(()) + } + fn delete_on_write(&self) -> bool { true } @@ -230,6 +239,15 @@ struct PthreadRwLock { } impl SyncObj for PthreadRwLock { + fn on_access<'tcx>(&self, access_kind: AccessKind) -> InterpResult<'tcx> { + if !self.rwlock_ref.queue_is_empty() { + throw_ub_format!( + "{access_kind} to `pthread_rwlock_t` is forbidden while the queue is non-empty" + ); + } + interp_ok(()) + } + fn delete_on_write(&self) -> bool { true } @@ -361,6 +379,15 @@ struct PthreadCondvar { } impl SyncObj for PthreadCondvar { + fn on_access<'tcx>(&self, access_kind: AccessKind) -> InterpResult<'tcx> { + if !self.condvar_ref.queue_is_empty() { + throw_ub_format!( + "{access_kind} to `pthread_cond_t` is forbidden while the queue is non-empty" + ); + } + interp_ok(()) + } + fn delete_on_write(&self) -> bool { true } @@ -900,7 +927,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Reading the field also has the side-effect that we detect double-`destroy` // since we make the field uninit below. let condvar = &cond_get_data(this, cond_op)?.condvar_ref; - if condvar.is_awaited() { + if !condvar.queue_is_empty() { throw_ub_format!("destroying an awaited conditional variable"); } diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 43d9ba0043a34..c079045908ca0 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -12,6 +12,15 @@ struct WindowsInitOnce { } impl SyncObj for WindowsInitOnce { + fn on_access<'tcx>(&self, access_kind: AccessKind) -> InterpResult<'tcx> { + if !self.init_once.queue_is_empty() { + throw_ub_format!( + "{access_kind} to `INIT_ONCE` is forbidden while the queue is non-empty" + ); + } + interp_ok(()) + } + fn delete_on_write(&self) -> bool { true } diff --git a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs new file mode 100644 index 0000000000000..b0718d3874aa4 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs @@ -0,0 +1,29 @@ +//@only-target: darwin +#![feature(sync_unsafe_cell)] + +use std::cell::SyncUnsafeCell; +use std::sync::atomic::*; +use std::thread; + +fn main() { + let lock = SyncUnsafeCell::new(libc::OS_UNFAIR_LOCK_INIT); + + thread::scope(|s| { + // First thread: grabs the lock. + s.spawn(|| { + unsafe { libc::os_unfair_lock_lock(lock.get()) }; + thread::yield_now(); + unreachable!(); + }); + // Second thread: queues for the lock. + s.spawn(|| { + unsafe { libc::os_unfair_lock_lock(lock.get()) }; + unreachable!(); + }); + // Third thread: tries to read the lock while second thread is queued. + s.spawn(|| { + let atomic_ref = unsafe { &*lock.get().cast::() }; + let _val = atomic_ref.load(Ordering::Relaxed); //~ERROR: read access to `os_unfair_lock` is forbidden while the queue is non-empty + }); + }); +} diff --git a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.stderr b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.stderr new file mode 100644 index 0000000000000..72ce13ac9907a --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.stderr @@ -0,0 +1,13 @@ +error: Undefined Behavior: read access to `os_unfair_lock` is forbidden while the queue is non-empty + --> tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs:LL:CC + | +LL | ... let _val = atomic_ref.load(Ordering::Relaxed); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = 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 + +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/concurrency/libc_pthread_mutex_read_while_queued.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.rs new file mode 100644 index 0000000000000..e1d801ee48687 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.rs @@ -0,0 +1,41 @@ +//@ignore-target: windows # No pthreads on Windows +//@compile-flags: -Zmiri-fixed-schedule + +use std::cell::UnsafeCell; +use std::sync::atomic::*; +use std::thread; + +struct Mutex(UnsafeCell); +impl Mutex { + fn get(&self) -> *mut libc::pthread_mutex_t { + self.0.get() + } +} + +unsafe impl Send for Mutex {} +unsafe impl Sync for Mutex {} + +// The offset to the "sensitive" part of the mutex (that Miri attaches the metadata to). +const OFFSET: usize = if cfg!(target_os = "macos") { 4 } else { 0 }; + +fn main() { + let m = Mutex(UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER)); + thread::scope(|s| { + // First thread: grabs the lock. + s.spawn(|| { + assert_eq!(unsafe { libc::pthread_mutex_lock(m.get()) }, 0); + thread::yield_now(); + unreachable!(); + }); + // Second thread: queues for the lock. + s.spawn(|| { + assert_eq!(unsafe { libc::pthread_mutex_lock(m.get()) }, 0); + unreachable!(); + }); + // Third thread: tries to read the lock while second thread is queued. + s.spawn(|| { + let atomic_ref = unsafe { &*m.get().byte_add(OFFSET).cast::() }; + let _val = atomic_ref.load(Ordering::Relaxed); //~ERROR: read access to `pthread_mutex_t` is forbidden while the queue is non-empty + }); + }); +} diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.stderr new file mode 100644 index 0000000000000..b20426ec59735 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.stderr @@ -0,0 +1,13 @@ +error: Undefined Behavior: read access to `pthread_mutex_t` is forbidden while the queue is non-empty + --> tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.rs:LL:CC + | +LL | ... let _val = atomic_ref.load(Ordering::Relaxed); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = 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 + +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/concurrency/libc_pthread_mutex_write_while_queued.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.rs new file mode 100644 index 0000000000000..6d136a8a87c27 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.rs @@ -0,0 +1,41 @@ +//@ignore-target: windows # No pthreads on Windows +//@compile-flags: -Zmiri-fixed-schedule + +use std::cell::UnsafeCell; +use std::sync::atomic::*; +use std::thread; + +struct Mutex(UnsafeCell); +impl Mutex { + fn get(&self) -> *mut libc::pthread_mutex_t { + self.0.get() + } +} + +unsafe impl Send for Mutex {} +unsafe impl Sync for Mutex {} + +// The offset to the "sensitive" part of the mutex (that Miri attaches the metadata to). +const OFFSET: usize = if cfg!(target_os = "macos") { 4 } else { 0 }; + +fn main() { + let m = Mutex(UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER)); + thread::scope(|s| { + // First thread: grabs the lock. + s.spawn(|| { + assert_eq!(unsafe { libc::pthread_mutex_lock(m.get()) }, 0); + thread::yield_now(); + unreachable!(); + }); + // Second thread: queues for the lock. + s.spawn(|| { + assert_eq!(unsafe { libc::pthread_mutex_lock(m.get()) }, 0); + unreachable!(); + }); + // Third thread: tries to overwrite the lock while second thread is queued. + s.spawn(|| { + let atomic_ref = unsafe { &*m.get().byte_add(OFFSET).cast::() }; + atomic_ref.store(0, Ordering::Relaxed); //~ERROR: write access to `pthread_mutex_t` is forbidden while the queue is non-empty + }); + }); +} diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.stderr new file mode 100644 index 0000000000000..28a79099a6b7e --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.stderr @@ -0,0 +1,13 @@ +error: Undefined Behavior: write access to `pthread_mutex_t` is forbidden while the queue is non-empty + --> tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.rs:LL:CC + | +LL | ... atomic_ref.store(0, Ordering::Relaxed); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = 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 + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + From 3c4b29c874390c72080ace392bcc9216d973df10 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 7 Nov 2025 14:37:25 +0100 Subject: [PATCH 13/14] remove dead code --- src/tools/miri/src/concurrency/sync.rs | 83 -------------------------- 1 file changed, 83 deletions(-) diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index ac3ee3f91c5dc..ad8f43624e9bf 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -257,95 +257,12 @@ impl<'tcx> AllocExtra<'tcx> { } } -/// We designate an `init`` field in all synchronization objects. -/// If `init` is set to this, we consider the object initialized. -pub const LAZY_INIT_COOKIE: u32 = 0xcafe_affe; - // Public interface to synchronization objects. Please note that in most // cases, the function calls are infallible and it is the client's (shim // implementation's) responsibility to detect and deal with erroneous // situations. impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { - /// Helper for lazily initialized `alloc_extra.sync` data: - /// this forces an immediate init. - /// Return a reference to the data in the machine state. - fn lazy_sync_init<'a, T: SyncObj>( - &'a mut self, - obj: &MPlaceTy<'tcx>, - init_offset: Size, - data: T, - ) -> InterpResult<'tcx, &'a T> - where - 'tcx: 'a, - { - let this = self.eval_context_mut(); - - let (alloc, offset, _) = this.ptr_get_alloc_id(obj.ptr(), 0)?; - // Mark this as "initialized". - let init_cookie = Scalar::from_u32(LAZY_INIT_COOKIE); - assert!(init_offset + init_cookie.size() <= obj.layout.size); - let init_field = obj.offset(init_offset, this.machine.layouts.u32, this)?; - this.write_scalar_atomic(init_cookie, &init_field, AtomicWriteOrd::Relaxed)?; - // Insert sync obj, and return reference to it. - let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc)?; - alloc_extra.sync_objs.insert(offset, Box::new(data)); - interp_ok(this.get_alloc_extra(alloc)?.get_sync::(offset).unwrap()) - } - - /// Helper for lazily initialized `alloc_extra.sync` data: - /// Checks if the synchronization object is initialized: - /// - If yes, fetches the data from `alloc_extra.sync`, or calls `missing_data` if that fails - /// and stores that in `alloc_extra.sync`. - /// - Otherwise, calls `new_data` to initialize the object. - /// - /// Return a reference to the data in the machine state. - fn lazy_sync_get_data<'a, T: SyncObj>( - &'a mut self, - obj: &MPlaceTy<'tcx>, - init_offset: Size, - missing_data: impl FnOnce() -> InterpResult<'tcx, T>, - new_data: impl FnOnce(&mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>, - ) -> InterpResult<'tcx, &'a T> - where - 'tcx: 'a, - { - let this = self.eval_context_mut(); - - // Check if this is already initialized. Needs to be atomic because we can race with another - // thread initializing. Needs to be an RMW operation to ensure we read the *latest* value. - // So we just try to replace MUTEX_INIT_COOKIE with itself. - let init_cookie = Scalar::from_u32(LAZY_INIT_COOKIE); - assert!(init_offset + init_cookie.size() <= obj.layout.size); - let init_field = obj.offset(init_offset, this.machine.layouts.u32, this)?; - let (_init, success) = this - .atomic_compare_exchange_scalar( - &init_field, - &ImmTy::from_scalar(init_cookie, this.machine.layouts.u32), - init_cookie, - AtomicRwOrd::Relaxed, - AtomicReadOrd::Relaxed, - /* can_fail_spuriously */ false, - )? - .to_scalar_pair(); - - if success.to_bool()? { - // If it is initialized, it must be found in the "sync obj" table, - // or else it has been moved illegally. - let (alloc, offset, _) = this.ptr_get_alloc_id(obj.ptr(), 0)?; - let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc)?; - // Due to borrow checker reasons, we have to do the lookup twice. - if alloc_extra.get_sync::(offset).is_none() { - let data = missing_data()?; - alloc_extra.sync_objs.insert(offset, Box::new(data)); - } - interp_ok(alloc_extra.get_sync::(offset).unwrap()) - } else { - let data = new_data(this)?; - this.lazy_sync_init(obj, init_offset, data) - } - } - /// Get the synchronization object associated with the given pointer, /// or initialize a new one. /// From d6b01abd4d5aebadf8b4af42bd023dcedfb59e3a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 8 Nov 2025 12:32:36 +0100 Subject: [PATCH 14/14] also inform sync objcts about deallocation; needs separate AccessKind type --- src/tools/miri/src/borrow_tracker/mod.rs | 16 +++++++ .../stacked_borrows/diagnostics.rs | 2 +- .../src/borrow_tracker/stacked_borrows/mod.rs | 2 +- .../tree_borrows/diagnostics.rs | 2 +- .../src/borrow_tracker/tree_borrows/mod.rs | 2 +- .../src/borrow_tracker/tree_borrows/perms.rs | 2 +- .../src/borrow_tracker/tree_borrows/tree.rs | 2 +- src/tools/miri/src/concurrency/sync.rs | 18 +++++++ src/tools/miri/src/diagnostics.rs | 2 +- src/tools/miri/src/helpers.rs | 18 +------ src/tools/miri/src/lib.rs | 2 +- src/tools/miri/src/machine.rs | 13 +++-- src/tools/miri/src/shims/unix/macos/sync.rs | 4 +- src/tools/miri/src/shims/unix/sync.rs | 8 ++-- src/tools/miri/src/shims/windows/sync.rs | 4 +- .../apple_os_unfair_lock_move_with_queue.rs | 2 +- ...pple_os_unfair_lock_move_with_queue.stderr | 6 +-- .../libc_pthread_mutex_free_while_queued.rs | 48 +++++++++++++++++++ ...ibc_pthread_mutex_free_while_queued.stderr | 22 +++++++++ .../libc_pthread_mutex_read_while_queued.rs | 2 +- ...ibc_pthread_mutex_read_while_queued.stderr | 2 +- .../libc_pthread_mutex_write_while_queued.rs | 2 +- ...bc_pthread_mutex_write_while_queued.stderr | 6 +-- 23 files changed, 140 insertions(+), 47 deletions(-) create mode 100644 src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_free_while_queued.rs create mode 100644 src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_free_while_queued.stderr diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index ef137349abb12..ebca7377fdbcc 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -11,6 +11,22 @@ use crate::*; pub mod stacked_borrows; pub mod tree_borrows; +/// Indicates which kind of access is being performed. +#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] +pub enum AccessKind { + Read, + Write, +} + +impl fmt::Display for AccessKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + AccessKind::Read => write!(f, "read access"), + AccessKind::Write => write!(f, "write access"), + } + } +} + /// Tracking pointer provenance #[derive(Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] pub struct BorTag(NonZero); diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs index 997d7799a5f1c..36e574c8e57f7 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -5,7 +5,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_span::{Span, SpanData}; use smallvec::SmallVec; -use crate::borrow_tracker::{GlobalStateInner, ProtectorKind}; +use crate::borrow_tracker::{AccessKind, GlobalStateInner, ProtectorKind}; use crate::*; /// Error reporting diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 76ede552baa2d..fa60f27185f83 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -21,7 +21,7 @@ pub use self::stack::Stack; use crate::borrow_tracker::stacked_borrows::diagnostics::{ AllocHistory, DiagnosticCx, DiagnosticCxBuilder, }; -use crate::borrow_tracker::{GlobalStateInner, ProtectorKind}; +use crate::borrow_tracker::{AccessKind, GlobalStateInner, ProtectorKind}; use crate::concurrency::data_race::{NaReadType, NaWriteType}; use crate::*; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs index 00f921b0f8afb..f2410a08625dd 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -4,10 +4,10 @@ use std::ops::Range; use rustc_data_structures::fx::FxHashMap; use rustc_span::{Span, SpanData}; -use crate::borrow_tracker::ProtectorKind; use crate::borrow_tracker::tree_borrows::perms::{PermTransition, Permission}; use crate::borrow_tracker::tree_borrows::tree::LocationState; use crate::borrow_tracker::tree_borrows::unimap::UniIndex; +use crate::borrow_tracker::{AccessKind, ProtectorKind}; use crate::*; /// Cause of an access: either a real access or one diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 5c905fc161f0f..720c5b239495e 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -5,7 +5,7 @@ use rustc_middle::ty::{self, Ty}; use self::foreign_access_skipping::IdempotentForeignAccess; use self::tree::LocationState; -use crate::borrow_tracker::{GlobalState, GlobalStateInner, ProtectorKind}; +use crate::borrow_tracker::{AccessKind, GlobalState, GlobalStateInner, ProtectorKind}; use crate::concurrency::data_race::NaReadType; use crate::*; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index 968e4961a6355..b84ebd51656c0 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -1,7 +1,7 @@ use std::cmp::{Ordering, PartialOrd}; use std::fmt; -use crate::AccessKind; +use crate::borrow_tracker::AccessKind; use crate::borrow_tracker::tree_borrows::diagnostics::TransitionError; use crate::borrow_tracker::tree_borrows::tree::AccessRelatedness; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 740483844e795..e337fe05e135d 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -25,7 +25,7 @@ use crate::borrow_tracker::tree_borrows::diagnostics::{ use crate::borrow_tracker::tree_borrows::foreign_access_skipping::IdempotentForeignAccess; use crate::borrow_tracker::tree_borrows::perms::PermTransition; use crate::borrow_tracker::tree_borrows::unimap::{UniIndex, UniKeyMap, UniValMap}; -use crate::borrow_tracker::{GlobalState, ProtectorKind}; +use crate::borrow_tracker::{AccessKind, GlobalState, ProtectorKind}; use crate::*; mod tests; diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index ad8f43624e9bf..c529ed5145edd 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -14,6 +14,24 @@ use rustc_data_structures::fx::FxHashMap; use super::vector_clock::VClock; use crate::*; +/// Indicates which kind of access is being performed. +#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] +pub enum AccessKind { + Read, + Write, + Dealloc, +} + +impl fmt::Display for AccessKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + AccessKind::Read => write!(f, "read"), + AccessKind::Write => write!(f, "write"), + AccessKind::Dealloc => write!(f, "deallocation"), + } + } +} + /// A trait for the synchronization metadata that can be attached to a memory location. pub trait SyncObj: Any { /// Determines whether reads/writes to this object's location are currently permitted. diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index d0cfb9c805e15..2ddb3ff49d85e 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -128,7 +128,7 @@ pub enum NonHaltingDiagnostic { PoppedPointerTag(Item, String), TrackingAlloc(AllocId, Size, Align), FreedAlloc(AllocId), - AccessedAlloc(AllocId, AllocRange, AccessKind), + AccessedAlloc(AllocId, AllocRange, borrow_tracker::AccessKind), RejectedIsolatedOp(String), ProgressReport { block_count: u64, // how many basic blocks have been run so far diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index d6cef032b4e7e..18e16ddf1a5da 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -1,7 +1,7 @@ use std::num::NonZero; use std::sync::Mutex; use std::time::Duration; -use std::{cmp, fmt, iter}; +use std::{cmp, iter}; use rand::RngCore; use rustc_abi::{Align, ExternAbi, FieldIdx, FieldsShape, Size, Variants}; @@ -22,22 +22,6 @@ use rustc_symbol_mangling::mangle_internal_symbol; use crate::*; -/// Indicates which kind of access is being performed. -#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] -pub enum AccessKind { - Read, - Write, -} - -impl fmt::Display for AccessKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - AccessKind::Read => write!(f, "read access"), - AccessKind::Write => write!(f, "write access"), - } - } -} - /// Gets an instance for a path. /// /// A `None` namespace indicates we are looking for a module. diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index b756fbb901bc6..8f732adeb6ef3 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -139,7 +139,7 @@ pub use crate::diagnostics::{ EvalContextExt as _, NonHaltingDiagnostic, TerminationInfo, report_error, }; pub use crate::eval::{MiriConfig, MiriEntryFnType, create_ecx, eval_entry}; -pub use crate::helpers::{AccessKind, EvalContextExt as _, ToU64 as _, ToUsize as _}; +pub use crate::helpers::{EvalContextExt as _, ToU64 as _, ToUsize as _}; pub use crate::intrinsics::EvalContextExt as _; pub use crate::machine::{ AlignmentCheck, AllocExtra, BacktraceStyle, DynMachineCallback, FloatRoundingErrorMode, diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 7eff006f418db..7cfde667f8b9d 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1527,7 +1527,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { machine.emit_diagnostic(NonHaltingDiagnostic::AccessedAlloc( alloc_id, range, - AccessKind::Read, + borrow_tracker::AccessKind::Read, )); } // The order of checks is deliberate, to prefer reporting a data race over a borrow tracker error. @@ -1549,7 +1549,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { } // Check if there are any sync objects that would like to prevent reading this memory. for (_offset, obj) in alloc_extra.sync_objs.range(range.start..range.end()) { - obj.on_access(AccessKind::Read)?; + obj.on_access(concurrency::sync::AccessKind::Read)?; } interp_ok(()) @@ -1568,7 +1568,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { machine.emit_diagnostic(NonHaltingDiagnostic::AccessedAlloc( alloc_id, range, - AccessKind::Write, + borrow_tracker::AccessKind::Write, )); } match &machine.data_race { @@ -1597,7 +1597,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { if !alloc_extra.sync_objs.is_empty() { let mut to_delete = vec![]; for (offset, obj) in alloc_extra.sync_objs.range(range.start..range.end()) { - obj.on_access(AccessKind::Write)?; + obj.on_access(concurrency::sync::AccessKind::Write)?; if obj.delete_on_write() { to_delete.push(*offset); } @@ -1642,6 +1642,11 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { if let Some(borrow_tracker) = &mut alloc_extra.borrow_tracker { borrow_tracker.before_memory_deallocation(alloc_id, prove_extra, size, machine)?; } + // Check if there are any sync objects that would like to prevent freeing this memory. + for obj in alloc_extra.sync_objs.values() { + obj.on_access(concurrency::sync::AccessKind::Dealloc)?; + } + if let Some((_, deallocated_at)) = machine.allocation_spans.borrow_mut().get_mut(&alloc_id) { *deallocated_at = Some(machine.current_user_relevant_span()); diff --git a/src/tools/miri/src/shims/unix/macos/sync.rs b/src/tools/miri/src/shims/unix/macos/sync.rs index da87244118a52..d69d373b572bd 100644 --- a/src/tools/miri/src/shims/unix/macos/sync.rs +++ b/src/tools/miri/src/shims/unix/macos/sync.rs @@ -15,7 +15,7 @@ use std::time::Duration; use rustc_abi::{Endian, FieldIdx, Size}; -use crate::concurrency::sync::{FutexRef, SyncObj}; +use crate::concurrency::sync::{AccessKind, FutexRef, SyncObj}; use crate::*; #[derive(Clone)] @@ -30,7 +30,7 @@ impl SyncObj for MacOsUnfairLock { && !mutex_ref.queue_is_empty() { throw_ub_format!( - "{access_kind} to `os_unfair_lock` is forbidden while the queue is non-empty" + "{access_kind} of `os_unfair_lock` is forbidden while the queue is non-empty" ); } interp_ok(()) diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index 189b475ad8976..57dbe2cd333b1 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -1,6 +1,6 @@ use rustc_abi::Size; -use crate::concurrency::sync::SyncObj; +use crate::concurrency::sync::{AccessKind, SyncObj}; use crate::*; /// Do a bytewise comparison of the two places. This is used to check if @@ -113,7 +113,7 @@ impl SyncObj for PthreadMutex { fn on_access<'tcx>(&self, access_kind: AccessKind) -> InterpResult<'tcx> { if !self.mutex_ref.queue_is_empty() { throw_ub_format!( - "{access_kind} to `pthread_mutex_t` is forbidden while the queue is non-empty" + "{access_kind} of `pthread_mutex_t` is forbidden while the queue is non-empty" ); } interp_ok(()) @@ -242,7 +242,7 @@ impl SyncObj for PthreadRwLock { fn on_access<'tcx>(&self, access_kind: AccessKind) -> InterpResult<'tcx> { if !self.rwlock_ref.queue_is_empty() { throw_ub_format!( - "{access_kind} to `pthread_rwlock_t` is forbidden while the queue is non-empty" + "{access_kind} of `pthread_rwlock_t` is forbidden while the queue is non-empty" ); } interp_ok(()) @@ -382,7 +382,7 @@ impl SyncObj for PthreadCondvar { fn on_access<'tcx>(&self, access_kind: AccessKind) -> InterpResult<'tcx> { if !self.condvar_ref.queue_is_empty() { throw_ub_format!( - "{access_kind} to `pthread_cond_t` is forbidden while the queue is non-empty" + "{access_kind} of `pthread_cond_t` is forbidden while the queue is non-empty" ); } interp_ok(()) diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index c079045908ca0..db1860bdfd309 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -3,7 +3,7 @@ use std::time::Duration; use rustc_abi::{FieldIdx, Size}; use crate::concurrency::init_once::{EvalContextExt as _, InitOnceStatus}; -use crate::concurrency::sync::{FutexRef, SyncObj}; +use crate::concurrency::sync::{AccessKind, FutexRef, SyncObj}; use crate::*; #[derive(Clone)] @@ -15,7 +15,7 @@ impl SyncObj for WindowsInitOnce { fn on_access<'tcx>(&self, access_kind: AccessKind) -> InterpResult<'tcx> { if !self.init_once.queue_is_empty() { throw_ub_format!( - "{access_kind} to `INIT_ONCE` is forbidden while the queue is non-empty" + "{access_kind} of `INIT_ONCE` is forbidden while the queue is non-empty" ); } interp_ok(()) diff --git a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs index b0718d3874aa4..1c31236a2f805 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs @@ -23,7 +23,7 @@ fn main() { // Third thread: tries to read the lock while second thread is queued. s.spawn(|| { let atomic_ref = unsafe { &*lock.get().cast::() }; - let _val = atomic_ref.load(Ordering::Relaxed); //~ERROR: read access to `os_unfair_lock` is forbidden while the queue is non-empty + let _val = atomic_ref.load(Ordering::Relaxed); //~ERROR: read of `os_unfair_lock` is forbidden while the queue is non-empty }); }); } diff --git a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.stderr b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.stderr index 72ce13ac9907a..003ddb9b287d2 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: read access to `os_unfair_lock` is forbidden while the queue is non-empty +error: Undefined Behavior: read of `os_unfair_lock` is forbidden while the queue is non-empty --> tests/fail-dep/concurrency/apple_os_unfair_lock_move_with_queue.rs:LL:CC | -LL | ... let _val = atomic_ref.load(Ordering::Relaxed); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here +LL | let _val = atomic_ref.load(Ordering::Relaxed); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here | = 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 diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_free_while_queued.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_free_while_queued.rs new file mode 100644 index 0000000000000..55fcb4c61d480 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_free_while_queued.rs @@ -0,0 +1,48 @@ +//@ignore-target: windows # No pthreads on Windows +//@compile-flags: -Zmiri-deterministic-concurrency +//@error-in-other-file: deallocation of `pthread_mutex_t` is forbidden while the queue is non-empty + +use std::cell::UnsafeCell; +use std::sync::atomic::*; +use std::thread; + +struct Mutex(UnsafeCell); +impl Mutex { + fn get(&self) -> *mut libc::pthread_mutex_t { + self.0.get() + } +} + +unsafe impl Send for Mutex {} +unsafe impl Sync for Mutex {} + +fn main() { + let m = Box::new(Mutex(UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER))); + let initialized = AtomicBool::new(false); + thread::scope(|s| { + // First thread: initializes the lock, and then grabs it. + s.spawn(|| { + // Initialize (so the third thread can happens-after the write that occurs here). + assert_eq!(unsafe { libc::pthread_mutex_lock(m.get()) }, 0); + assert_eq!(unsafe { libc::pthread_mutex_unlock(m.get()) }, 0); + initialized.store(true, Ordering::Release); + // Grab and hold. + assert_eq!(unsafe { libc::pthread_mutex_lock(m.get()) }, 0); + thread::yield_now(); + unreachable!(); + }); + // Second thread: queues for the lock. + s.spawn(|| { + assert_eq!(unsafe { libc::pthread_mutex_lock(m.get()) }, 0); + unreachable!(); + }); + // Third thread: tries to free the lock while second thread is queued. + s.spawn(|| { + // Ensure we happen-after the initialization write. + assert!(initialized.load(Ordering::Acquire)); + // Now drop it. + drop(unsafe { Box::from_raw(m.get().cast::()) }); + }); + }); + unreachable!(); +} diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_free_while_queued.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_free_while_queued.stderr new file mode 100644 index 0000000000000..7b6e05828cea3 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_free_while_queued.stderr @@ -0,0 +1,22 @@ +error: Undefined Behavior: deallocation of `pthread_mutex_t` is forbidden while the queue is non-empty + --> RUSTLIB/alloc/src/boxed.rs:LL:CC + | +LL | self.1.deallocate(From::from(ptr.cast()), layout); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = 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 + = note: BACKTRACE on thread `unnamed-ID`: + = note: inside ` as std::ops::Drop>::drop` at RUSTLIB/alloc/src/boxed.rs:LL:CC + = note: inside `std::ptr::drop_in_place::> - shim(Some(std::boxed::Box))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC + = note: inside `std::mem::drop::>` at RUSTLIB/core/src/mem/mod.rs:LL:CC +note: inside closure + --> tests/fail-dep/concurrency/libc_pthread_mutex_free_while_queued.rs:LL:CC + | +LL | drop(unsafe { Box::from_raw(m.get().cast::()) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +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/concurrency/libc_pthread_mutex_read_while_queued.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.rs index e1d801ee48687..555d765d24ba3 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.rs @@ -35,7 +35,7 @@ fn main() { // Third thread: tries to read the lock while second thread is queued. s.spawn(|| { let atomic_ref = unsafe { &*m.get().byte_add(OFFSET).cast::() }; - let _val = atomic_ref.load(Ordering::Relaxed); //~ERROR: read access to `pthread_mutex_t` is forbidden while the queue is non-empty + let _val = atomic_ref.load(Ordering::Relaxed); //~ERROR: read of `pthread_mutex_t` is forbidden while the queue is non-empty }); }); } diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.stderr index b20426ec59735..42dbd5f02cb3b 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.stderr @@ -1,4 +1,4 @@ -error: Undefined Behavior: read access to `pthread_mutex_t` is forbidden while the queue is non-empty +error: Undefined Behavior: read of `pthread_mutex_t` is forbidden while the queue is non-empty --> tests/fail-dep/concurrency/libc_pthread_mutex_read_while_queued.rs:LL:CC | LL | ... let _val = atomic_ref.load(Ordering::Relaxed); diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.rs index 6d136a8a87c27..00274f7080f33 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.rs @@ -35,7 +35,7 @@ fn main() { // Third thread: tries to overwrite the lock while second thread is queued. s.spawn(|| { let atomic_ref = unsafe { &*m.get().byte_add(OFFSET).cast::() }; - atomic_ref.store(0, Ordering::Relaxed); //~ERROR: write access to `pthread_mutex_t` is forbidden while the queue is non-empty + atomic_ref.store(0, Ordering::Relaxed); //~ERROR: write of `pthread_mutex_t` is forbidden while the queue is non-empty }); }); } diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.stderr index 28a79099a6b7e..4705f9a1b5f02 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: write access to `pthread_mutex_t` is forbidden while the queue is non-empty +error: Undefined Behavior: write of `pthread_mutex_t` is forbidden while the queue is non-empty --> tests/fail-dep/concurrency/libc_pthread_mutex_write_while_queued.rs:LL:CC | -LL | ... atomic_ref.store(0, Ordering::Relaxed); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here +LL | atomic_ref.store(0, Ordering::Relaxed); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here | = 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