From 56f50d53d10bf10da273e1642b6fd8fa3dcad617 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Dec 2023 14:42:41 +0100 Subject: [PATCH 01/13] also test simd_select_bitmask on arrays for less than 8 elements --- src/tools/miri/tests/pass/portable-simd.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/tests/pass/portable-simd.rs b/src/tools/miri/tests/pass/portable-simd.rs index 514e12fffc5dc..84ce372c60d59 100644 --- a/src/tools/miri/tests/pass/portable-simd.rs +++ b/src/tools/miri/tests/pass/portable-simd.rs @@ -266,7 +266,12 @@ fn simd_mask() { } } - // This used to cause an ICE. + // This used to cause an ICE. It exercises simd_select_bitmask with an array as input. + let bitmask = u8x4::from_array([0b00001101, 0, 0, 0]); + assert_eq!( + mask32x4::from_bitmask_vector(bitmask), + mask32x4::from_array([true, false, true, true]), + ); let bitmask = u8x8::from_array([0b01000101, 0, 0, 0, 0, 0, 0, 0]); assert_eq!( mask32x8::from_bitmask_vector(bitmask), From 92b4ffc68856e07efdf9b42de02f88cba000fddd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Dec 2023 14:51:31 +0100 Subject: [PATCH 02/13] handle the array case consistently for simd_select_bitmask and simd_bitmask also move the two next to each other --- src/tools/miri/src/shims/intrinsics/simd.rs | 88 +++++++++++---------- 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/src/tools/miri/src/shims/intrinsics/simd.rs b/src/tools/miri/src/shims/intrinsics/simd.rs index e16d116f621ed..4d30eebb1af04 100644 --- a/src/tools/miri/src/shims/intrinsics/simd.rs +++ b/src/tools/miri/src/shims/intrinsics/simd.rs @@ -405,6 +405,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.write_immediate(*val, &dest)?; } } + // Variant of `select` that takes a bitmask rather than a "vector of bool". "select_bitmask" => { let [mask, yes, no] = check_arg_count(args)?; let (yes, yes_len) = this.operand_to_simd(yes)?; @@ -412,6 +413,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let (dest, dest_len) = this.place_to_simd(dest)?; let bitmask_len = dest_len.max(8); + // The mask must be an integer or an array. + assert!( + mask.layout.ty.is_integral() + || matches!(mask.layout.ty.kind(), ty::Array(elemty, _) if elemty == &this.tcx.types.u8) + ); assert!(bitmask_len <= 64); assert_eq!(bitmask_len, mask.layout.size.bits()); assert_eq!(dest_len, yes_len); @@ -419,23 +425,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let dest_len = u32::try_from(dest_len).unwrap(); let bitmask_len = u32::try_from(bitmask_len).unwrap(); - // The mask can be a single integer or an array. - let mask: u64 = match mask.layout.ty.kind() { - ty::Int(..) | ty::Uint(..) => - this.read_scalar(mask)?.to_bits(mask.layout.size)?.try_into().unwrap(), - ty::Array(elem, _) if matches!(elem.kind(), ty::Uint(ty::UintTy::U8)) => { - let mask_ty = this.machine.layouts.uint(mask.layout.size).unwrap(); - let mask = mask.transmute(mask_ty, this)?; - this.read_scalar(&mask)?.to_bits(mask_ty.size)?.try_into().unwrap() - } - _ => bug!("simd_select_bitmask: invalid mask type {}", mask.layout.ty), - }; + // To read the mask, we transmute it to an integer. + // That does the right thing wrt endianess. + let mask_ty = this.machine.layouts.uint(mask.layout.size).unwrap(); + let mask = mask.transmute(mask_ty, this)?; + let mask: u64 = this.read_scalar(&mask)?.to_bits(mask_ty.size)?.try_into().unwrap(); for i in 0..dest_len { - let mask = mask - & 1u64 - .checked_shl(simd_bitmask_index(i, dest_len, this.data_layout().endian)) - .unwrap(); + let bit_i = simd_bitmask_index(i, dest_len, this.data_layout().endian); + let mask = mask & 1u64.checked_shl(bit_i).unwrap(); let yes = this.read_immediate(&this.project_index(&yes, i.into())?)?; let no = this.read_immediate(&this.project_index(&no, i.into())?)?; let dest = this.project_index(&dest, i.into())?; @@ -445,6 +443,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } for i in dest_len..bitmask_len { // If the mask is "padded", ensure that padding is all-zero. + // This deliberately does not use `simd_bitmask_index`; these bits are outside + // the bitmask. It does not matter in which order we check them. let mask = mask & 1u64.checked_shl(i).unwrap(); if mask != 0 { throw_ub_format!( @@ -453,6 +453,36 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } } + // Converts a "vector of bool" into a bitmask. + "bitmask" => { + let [op] = check_arg_count(args)?; + let (op, op_len) = this.operand_to_simd(op)?; + let bitmask_len = op_len.max(8); + + // Returns either an unsigned integer or array of `u8`. + assert!( + dest.layout.ty.is_integral() + || matches!(dest.layout.ty.kind(), ty::Array(elemty, _) if elemty == &this.tcx.types.u8) + ); + assert!(bitmask_len <= 64); + assert_eq!(bitmask_len, dest.layout.size.bits()); + let op_len = u32::try_from(op_len).unwrap(); + + let mut res = 0u64; + for i in 0..op_len { + let op = this.read_immediate(&this.project_index(&op, i.into())?)?; + if simd_element_to_bool(op)? { + res |= 1u64 + .checked_shl(simd_bitmask_index(i, op_len, this.data_layout().endian)) + .unwrap(); + } + } + // We have to change the type of the place to be able to write `res` into it. This + // transmutes the integer to an array, which does the right thing wrt endianess. + let dest = + dest.transmute(this.machine.layouts.uint(dest.layout.size).unwrap(), this)?; + this.write_int(res, &dest)?; + } "cast" | "as" | "cast_ptr" | "expose_addr" | "from_exposed_addr" => { let [op] = check_arg_count(args)?; let (op, op_len) = this.operand_to_simd(op)?; @@ -635,34 +665,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } } - "bitmask" => { - let [op] = check_arg_count(args)?; - let (op, op_len) = this.operand_to_simd(op)?; - let bitmask_len = op_len.max(8); - - // Returns either an unsigned integer or array of `u8`. - assert!( - dest.layout.ty.is_integral() - || matches!(dest.layout.ty.kind(), ty::Array(elemty, _) if elemty == &this.tcx.types.u8) - ); - assert!(bitmask_len <= 64); - assert_eq!(bitmask_len, dest.layout.size.bits()); - let op_len = u32::try_from(op_len).unwrap(); - - let mut res = 0u64; - for i in 0..op_len { - let op = this.read_immediate(&this.project_index(&op, i.into())?)?; - if simd_element_to_bool(op)? { - res |= 1u64 - .checked_shl(simd_bitmask_index(i, op_len, this.data_layout().endian)) - .unwrap(); - } - } - // We have to force the place type to be an int so that we can write `res` into it. - let mut dest = this.force_allocation(dest)?; - dest.layout = this.machine.layouts.uint(dest.layout.size).unwrap(); - this.write_int(res, &dest)?; - } name => throw_unsup_format!("unimplemented intrinsic: `simd_{name}`"), } From c9463698db637551bccb121955d48bb4018ed3db Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Dec 2023 16:17:12 +0100 Subject: [PATCH 03/13] also test directly calling simd_select_bitmask --- src/tools/miri/tests/pass/portable-simd.rs | 43 ++++++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/tests/pass/portable-simd.rs b/src/tools/miri/tests/pass/portable-simd.rs index 84ce372c60d59..2d59b88cb6ea7 100644 --- a/src/tools/miri/tests/pass/portable-simd.rs +++ b/src/tools/miri/tests/pass/portable-simd.rs @@ -3,10 +3,6 @@ #![allow(incomplete_features, internal_features)] use std::simd::{prelude::*, StdFloat}; -extern "platform-intrinsic" { - pub(crate) fn simd_bitmask(x: T) -> U; -} - fn simd_ops_f32() { let a = f32x4::splat(10.0); let b = f32x4::from_array([1.0, 2.0, 3.0, -4.0]); @@ -218,6 +214,11 @@ fn simd_ops_i32() { } fn simd_mask() { + extern "platform-intrinsic" { + pub(crate) fn simd_bitmask(x: T) -> U; + pub(crate) fn simd_select_bitmask(m: M, yes: T, no: T) -> T; + } + let intmask = Mask::from_int(i32x4::from_array([0, -1, 0, 0])); assert_eq!(intmask, Mask::from_array([false, true, false, false])); assert_eq!(intmask.to_array(), [false, true, false, false]); @@ -286,6 +287,40 @@ fn simd_mask() { true, true, true, ]), ); + + // Also directly call simd_select_bitmask, to test both kinds of argument types. + unsafe { + // These masks are exactly the results we got out above in the `simd_bitmask` tests. + let selected1 = simd_select_bitmask::( + if cfg!(target_endian = "little") { 0b1010001101001001 } else { 0b1001001011000101 }, + i32x16::splat(1), // yes + i32x16::splat(0), // no + ); + let selected2 = simd_select_bitmask::<[u8; 2], _>( + if cfg!(target_endian = "little") { + [0b01001001, 0b10100011] + } else { + [0b10010010, 0b11000101] + }, + i32x16::splat(1), // yes + i32x16::splat(0), // no + ); + assert_eq!(selected1, i32x16::from_array([1, 0, 0, 1, 0, 0, 1, 0, 1, 1, 0, 0, 0, 1, 0, 1])); + assert_eq!(selected2, selected1); + // Also try masks less than a byte long. + let selected1 = simd_select_bitmask::( + if cfg!(target_endian = "little") { 0b1000 } else { 0b0001 }, + i32x4::splat(1), // yes + i32x4::splat(0), // no + ); + let selected2 = simd_select_bitmask::<[u8; 1], _>( + if cfg!(target_endian = "little") { [0b1000] } else { [0b0001] }, + i32x4::splat(1), // yes + i32x4::splat(0), // no + ); + assert_eq!(selected1, i32x4::from_array([0, 0, 0, 1])); + assert_eq!(selected2, selected1); + } } fn simd_cast() { From 6e74d2ad500476007642982cbc6f2db6b6655300 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Dec 2023 16:19:11 +0100 Subject: [PATCH 04/13] disable a test that currently fails on big-endian --- src/tools/miri/tests/pass/portable-simd.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/tools/miri/tests/pass/portable-simd.rs b/src/tools/miri/tests/pass/portable-simd.rs index 2d59b88cb6ea7..184cc3d22e00e 100644 --- a/src/tools/miri/tests/pass/portable-simd.rs +++ b/src/tools/miri/tests/pass/portable-simd.rs @@ -268,11 +268,15 @@ fn simd_mask() { } // This used to cause an ICE. It exercises simd_select_bitmask with an array as input. - let bitmask = u8x4::from_array([0b00001101, 0, 0, 0]); - assert_eq!( - mask32x4::from_bitmask_vector(bitmask), - mask32x4::from_array([true, false, true, true]), - ); + if cfg!(target_endian = "little") { + // FIXME this test currently fails on big-endian: + // + let bitmask = u8x4::from_array([0b00001101, 0, 0, 0]); + assert_eq!( + mask32x4::from_bitmask_vector(bitmask), + mask32x4::from_array([true, false, true, true]), + ); + } let bitmask = u8x8::from_array([0b01000101, 0, 0, 0, 0, 0, 0, 0]); assert_eq!( mask32x8::from_bitmask_vector(bitmask), From 7c6ed0cc19b902f39a27945a652eb5e5e081f0f1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Dec 2023 19:23:58 +0100 Subject: [PATCH 05/13] SIMD bitmasks: use 'round up to multiple of 8' rather than 'clamp to at least 8' --- src/tools/miri/src/helpers.rs | 8 ++++++++ src/tools/miri/src/machine.rs | 5 ----- src/tools/miri/src/shims/intrinsics/simd.rs | 8 +++++--- src/tools/miri/src/shims/unix/mem.rs | 6 +++--- src/tools/miri/tests/pass/portable-simd.rs | 2 -- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 21f1d684924a0..57dc3b4734c46 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -1188,3 +1188,11 @@ pub(crate) fn simd_element_to_bool(elem: ImmTy<'_, Provenance>) -> InterpResult< _ => throw_ub_format!("each element of a SIMD mask must be all-0-bits or all-1-bits"), }) } + +// This looks like something that would be nice to have in the standard library... +pub(crate) fn round_to_next_multiple_of(x: u64, divisor: u64) -> u64 { + assert_ne!(divisor, 0); + // divisor is nonzero; multiplication cannot overflow since we just divided + #[allow(clippy::arithmetic_side_effects)] + return (x.checked_add(divisor - 1).unwrap() / divisor) * divisor; +} diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 66d7dfcf3a108..f066ac1e3ff87 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -768,11 +768,6 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { drop(self.profiler.take()); } - pub(crate) fn round_up_to_multiple_of_page_size(&self, length: u64) -> Option { - #[allow(clippy::arithmetic_side_effects)] // page size is nonzero - (length.checked_add(self.page_size - 1)? / self.page_size).checked_mul(self.page_size) - } - pub(crate) fn page_align(&self) -> Align { Align::from_bytes(self.page_size).unwrap() } diff --git a/src/tools/miri/src/shims/intrinsics/simd.rs b/src/tools/miri/src/shims/intrinsics/simd.rs index 4d30eebb1af04..a514cadcec931 100644 --- a/src/tools/miri/src/shims/intrinsics/simd.rs +++ b/src/tools/miri/src/shims/intrinsics/simd.rs @@ -4,8 +4,10 @@ use rustc_middle::{mir, ty, ty::FloatTy}; use rustc_span::{sym, Symbol}; use rustc_target::abi::{Endian, HasDataLayout}; +use crate::helpers::{ + bool_to_simd_element, check_arg_count, round_to_next_multiple_of, simd_element_to_bool, +}; use crate::*; -use helpers::{bool_to_simd_element, check_arg_count, simd_element_to_bool}; impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { @@ -411,7 +413,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let (yes, yes_len) = this.operand_to_simd(yes)?; let (no, no_len) = this.operand_to_simd(no)?; let (dest, dest_len) = this.place_to_simd(dest)?; - let bitmask_len = dest_len.max(8); + let bitmask_len = round_to_next_multiple_of(dest_len, 8); // The mask must be an integer or an array. assert!( @@ -457,7 +459,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { "bitmask" => { let [op] = check_arg_count(args)?; let (op, op_len) = this.operand_to_simd(op)?; - let bitmask_len = op_len.max(8); + let bitmask_len = round_to_next_multiple_of(op_len, 8); // Returns either an unsigned integer or array of `u8`. assert!( diff --git a/src/tools/miri/src/shims/unix/mem.rs b/src/tools/miri/src/shims/unix/mem.rs index a33d784d16651..cf36d4b191c7b 100644 --- a/src/tools/miri/src/shims/unix/mem.rs +++ b/src/tools/miri/src/shims/unix/mem.rs @@ -7,7 +7,7 @@ //! equivalent. That is the only part we support: no MAP_FIXED or MAP_SHARED or anything //! else that goes beyond a basic allocation API. -use crate::*; +use crate::{helpers::round_to_next_multiple_of, *}; use rustc_target::abi::Size; impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} @@ -89,7 +89,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } let align = this.machine.page_align(); - let map_length = this.machine.round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX); + let map_length = round_to_next_multiple_of(length, this.machine.page_size); let ptr = this.allocate_ptr(Size::from_bytes(map_length), align, MiriMemoryKind::Mmap.into())?; @@ -123,7 +123,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { return Ok(Scalar::from_i32(-1)); } - let length = this.machine.round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX); + let length = round_to_next_multiple_of(length, this.machine.page_size); let ptr = Machine::ptr_from_addr_cast(this, addr)?; diff --git a/src/tools/miri/tests/pass/portable-simd.rs b/src/tools/miri/tests/pass/portable-simd.rs index 184cc3d22e00e..f370e658272f5 100644 --- a/src/tools/miri/tests/pass/portable-simd.rs +++ b/src/tools/miri/tests/pass/portable-simd.rs @@ -253,8 +253,6 @@ fn simd_mask() { let bitmask = mask.to_bitmask(); assert_eq!(bitmask, 0b1000); assert_eq!(Mask::::from_bitmask(bitmask), mask); - - // Also directly call intrinsic, to test both kinds of return types. unsafe { let bitmask1: u8 = simd_bitmask(mask.to_int()); let bitmask2: [u8; 1] = simd_bitmask(mask.to_int()); From da4fd2220fe41c9bee45694922449d4f6fb8aec4 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Mon, 4 Dec 2023 04:56:13 +0000 Subject: [PATCH 06/13] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index a7ae20c1a35ab..f3492c3eb0448 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -225e36cff9809948d6567ab16f75d7b087ea83a7 +c9808f87028e16d134438787cab3d4cc16d05fe2 From 2c545ed69e2220e8d3daff2907730bd0a98e4abc Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Mon, 4 Dec 2023 05:03:55 +0000 Subject: [PATCH 07/13] fmt --- .../fail/unaligned_pointers/promise_alignment.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs index d2d49c604afbb..82753fe803caf 100644 --- a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs +++ b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs @@ -17,11 +17,7 @@ fn main() { let _val = unsafe { buffer.read() }; // Let's find a place to promise alignment 8. - let align8 = if buffer.addr() % 8 == 0 { - buffer - } else { - buffer.wrapping_add(1) - }; + let align8 = if buffer.addr() % 8 == 0 { buffer } else { buffer.wrapping_add(1) }; assert!(align8.addr() % 8 == 0); unsafe { utils::miri_promise_symbolic_alignment(align8.cast(), 8) }; // Promising the alignment down to 1 *again* still must not hurt. @@ -41,11 +37,7 @@ fn main() { #[derive(Copy, Clone)] struct Align16(u128); - let align16 = if align8.addr() % 16 == 0 { - align8 - } else { - align8.wrapping_add(2) - }; + let align16 = if align8.addr() % 16 == 0 { align8 } else { align8.wrapping_add(2) }; assert!(align16.addr() % 16 == 0); let _val = unsafe { align8.cast::().read() }; From 81740452a83d8c4fe60a2c77dd9b3ef7d814e4b9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Dec 2023 08:10:34 +0100 Subject: [PATCH 08/13] fix clippy --- src/tools/miri/src/shims/foreign_items.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 3a0ff7a556724..15432c5dd9c1e 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -567,7 +567,8 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ); }; let (_, addr) = ptr.into_parts(); // we know the offset is absolute - if addr.bytes() % align.bytes() != 0 { + // Cannot panic since `align` is a power of 2 and hence non-zero. + if addr.bytes().checked_rem(align.bytes()).unwrap() != 0 { throw_unsup_format!( "`miri_promise_symbolic_alignment`: pointer is not actually aligned" ); From d651eb9e23a3c9b67c50c847d7ac66965bda475c Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Tue, 5 Dec 2023 05:09:34 +0000 Subject: [PATCH 09/13] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index f3492c3eb0448..c60249f35e10d 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -c9808f87028e16d134438787cab3d4cc16d05fe2 +317d14a56cb8c748bf0e2f2afff89c2249ab4423 From 0d5fdcca6c935f41bc9895bfe40917e8888a1be1 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Tue, 5 Dec 2023 05:17:10 +0000 Subject: [PATCH 10/13] fmt --- .../issues/issue-3200-packed-field-offset.rs | 22 ++++++++++--------- .../issues/issue-3200-packed2-field-offset.rs | 22 ++++++++++--------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/tools/miri/tests/pass/issues/issue-3200-packed-field-offset.rs b/src/tools/miri/tests/pass/issues/issue-3200-packed-field-offset.rs index e58fe60b5f6ed..b396f3fa835cf 100644 --- a/src/tools/miri/tests/pass/issues/issue-3200-packed-field-offset.rs +++ b/src/tools/miri/tests/pass/issues/issue-3200-packed-field-offset.rs @@ -23,13 +23,15 @@ impl PackedSized { } } -fn main() { unsafe { - let p = PackedSized { f: 0, d: [1, 2, 3, 4] }; - let p = p.unsize() as *const PackedUnsized; - // Make sure the size computation does *not* think there is - // any padding in front of the `d` field. - assert_eq!(mem::size_of_val_raw(p), 1 + 4*4); - // And likewise for the offset computation. - let d = std::ptr::addr_of!((*p).d); - assert_eq!(d.cast::().read_unaligned(), 1); -} } +fn main() { + unsafe { + let p = PackedSized { f: 0, d: [1, 2, 3, 4] }; + let p = p.unsize() as *const PackedUnsized; + // Make sure the size computation does *not* think there is + // any padding in front of the `d` field. + assert_eq!(mem::size_of_val_raw(p), 1 + 4 * 4); + // And likewise for the offset computation. + let d = std::ptr::addr_of!((*p).d); + assert_eq!(d.cast::().read_unaligned(), 1); + } +} diff --git a/src/tools/miri/tests/pass/issues/issue-3200-packed2-field-offset.rs b/src/tools/miri/tests/pass/issues/issue-3200-packed2-field-offset.rs index 910b5d94320d0..a5cf337da024a 100644 --- a/src/tools/miri/tests/pass/issues/issue-3200-packed2-field-offset.rs +++ b/src/tools/miri/tests/pass/issues/issue-3200-packed2-field-offset.rs @@ -26,13 +26,15 @@ impl PackedSized { } } -fn main() { unsafe { - let p = PackedSized { f: 0, d: [1, 2, 3, 4] }; - let p = p.unsize() as *const PackedUnsized; - // Make sure the size computation correctly adds exact 1 byte of padding - // in front of the `d` field. - assert_eq!(mem::size_of_val_raw(p), 1 + 1 + 4*4); - // And likewise for the offset computation. - let d = std::ptr::addr_of!((*p).d); - assert_eq!(d.cast::().read_unaligned(), 1); -} } +fn main() { + unsafe { + let p = PackedSized { f: 0, d: [1, 2, 3, 4] }; + let p = p.unsize() as *const PackedUnsized; + // Make sure the size computation correctly adds exact 1 byte of padding + // in front of the `d` field. + assert_eq!(mem::size_of_val_raw(p), 1 + 1 + 4 * 4); + // And likewise for the offset computation. + let d = std::ptr::addr_of!((*p).d); + assert_eq!(d.cast::().read_unaligned(), 1); + } +} From 6237f2381d3efb88ccd8153fa3a618fa6d188076 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Dec 2023 07:32:49 +0100 Subject: [PATCH 11/13] fix typo in comment --- .../miri/tests/pass/issues/issue-3200-packed2-field-offset.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/tests/pass/issues/issue-3200-packed2-field-offset.rs b/src/tools/miri/tests/pass/issues/issue-3200-packed2-field-offset.rs index a5cf337da024a..a8a7387ecdcd5 100644 --- a/src/tools/miri/tests/pass/issues/issue-3200-packed2-field-offset.rs +++ b/src/tools/miri/tests/pass/issues/issue-3200-packed2-field-offset.rs @@ -30,7 +30,7 @@ fn main() { unsafe { let p = PackedSized { f: 0, d: [1, 2, 3, 4] }; let p = p.unsize() as *const PackedUnsized; - // Make sure the size computation correctly adds exact 1 byte of padding + // Make sure the size computation correctly adds exactly 1 byte of padding // in front of the `d` field. assert_eq!(mem::size_of_val_raw(p), 1 + 1 + 4 * 4); // And likewise for the offset computation. From ff95f8b8723be0b6e6413f05d9da8b2aecbea613 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Dec 2023 07:39:52 +0100 Subject: [PATCH 12/13] fix miri_promise_symbolic_alignment for huge alignments --- src/tools/miri/src/shims/foreign_items.rs | 15 ++++++++++++--- .../unaligned_pointers/promise_alignment_zero.rs | 8 ++++++++ .../promise_alignment_zero.stderr | 14 ++++++++++++++ .../miri/tests/pass/align_offset_symbolic.rs | 12 ++++++++++++ 4 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/promise_alignment_zero.rs create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/promise_alignment_zero.stderr diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 15432c5dd9c1e..8ddfc05dd300a 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -558,14 +558,23 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Promises that a pointer has a given symbolic alignment. "miri_promise_symbolic_alignment" => { + use rustc_target::abi::AlignFromBytesError; + let [ptr, align] = this.check_shim(abi, Abi::Rust, link_name, args)?; let ptr = this.read_pointer(ptr)?; let align = this.read_target_usize(align)?; - let Ok(align) = Align::from_bytes(align) else { + if !align.is_power_of_two() { throw_unsup_format!( - "`miri_promise_symbolic_alignment`: alignment must be a power of 2" + "`miri_promise_symbolic_alignment`: alignment must be a power of 2, got {align}" ); - }; + } + let align = Align::from_bytes(align).unwrap_or_else(|err| { + match err { + AlignFromBytesError::NotPowerOfTwo(_) => unreachable!(), + // When the alignment is a power of 2 but too big, clamp it to MAX. + AlignFromBytesError::TooLarge(_) => Align::MAX, + } + }); let (_, addr) = ptr.into_parts(); // we know the offset is absolute // Cannot panic since `align` is a power of 2 and hence non-zero. if addr.bytes().checked_rem(align.bytes()).unwrap() != 0 { diff --git a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment_zero.rs b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment_zero.rs new file mode 100644 index 0000000000000..5b0638b1fdfaf --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment_zero.rs @@ -0,0 +1,8 @@ +#[path = "../../utils/mod.rs"] +mod utils; + +fn main() { + let buffer = [0u32; 128]; + unsafe { utils::miri_promise_symbolic_alignment(buffer.as_ptr().cast(), 0) }; + //~^ERROR: alignment must be a power of 2 +} diff --git a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment_zero.stderr b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment_zero.stderr new file mode 100644 index 0000000000000..b3327e04933d5 --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment_zero.stderr @@ -0,0 +1,14 @@ +error: unsupported operation: `miri_promise_symbolic_alignment`: alignment must be a power of 2, got 0 + --> $DIR/promise_alignment_zero.rs:LL:CC + | +LL | unsafe { utils::miri_promise_symbolic_alignment(buffer.as_ptr().cast(), 0) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `miri_promise_symbolic_alignment`: alignment must be a power of 2, got 0 + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support + = note: BACKTRACE: + = note: inside `main` at $DIR/promise_alignment_zero.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/pass/align_offset_symbolic.rs b/src/tools/miri/tests/pass/align_offset_symbolic.rs index 292858ebc2e53..3e493952d281e 100644 --- a/src/tools/miri/tests/pass/align_offset_symbolic.rs +++ b/src/tools/miri/tests/pass/align_offset_symbolic.rs @@ -90,9 +90,21 @@ fn test_cstr() { std::ffi::CStr::from_bytes_with_nul(b"this is a test that is longer than 16 bytes\0").unwrap(); } +fn huge_align() { + #[cfg(target_pointer_width = "64")] + const SIZE: usize = 1 << 47; + #[cfg(target_pointer_width = "32")] + const SIZE: usize = 1 << 30; + #[cfg(target_pointer_width = "16")] + const SIZE: usize = 1 << 13; + struct HugeSize([u8; SIZE - 1]); + let _ = std::ptr::invalid::(SIZE).align_offset(SIZE); +} + fn main() { test_align_to(); test_from_utf8(); test_u64_array(); test_cstr(); + huge_align(); } From 1fa8fd800f7cdd245b8ea73dd0673a1cf434d925 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Dec 2023 07:42:13 +0100 Subject: [PATCH 13/13] simd numeric intrinsics: share code with scalar intrinsic --- src/tools/miri/src/shims/intrinsics/simd.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/tools/miri/src/shims/intrinsics/simd.rs b/src/tools/miri/src/shims/intrinsics/simd.rs index a514cadcec931..e17c06be9b837 100644 --- a/src/tools/miri/src/shims/intrinsics/simd.rs +++ b/src/tools/miri/src/shims/intrinsics/simd.rs @@ -115,18 +115,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } Op::Numeric(name) => { - assert!(op.layout.ty.is_integral()); - let size = op.layout.size; - let bits = op.to_scalar().to_bits(size).unwrap(); - let extra = 128u128.checked_sub(u128::from(size.bits())).unwrap(); - let bits_out = match name { - sym::ctlz => u128::from(bits.leading_zeros()).checked_sub(extra).unwrap(), - sym::cttz => u128::from((bits << extra).trailing_zeros()).checked_sub(extra).unwrap(), - sym::bswap => (bits << extra).swap_bytes(), - sym::bitreverse => (bits << extra).reverse_bits(), - _ => unreachable!(), - }; - Scalar::from_uint(bits_out, size) + this.numeric_intrinsic(name, op.to_scalar(), op.layout)? } }; this.write_scalar(val, &dest)?;