Skip to content

Miri rename undef uninit #74664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let ptr = Pointer::new(AllocId(0), offset);
alloc
.read_scalar(&bx, ptr, size)
.and_then(|s| s.not_undef())
.and_then(|s| s.check_init())
.unwrap_or_else(|e| {
bx.tcx().sess.span_err(
span,
Expand Down
94 changes: 47 additions & 47 deletions src/librustc_middle/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl<Tag> Allocation<Tag> {
Allocation::from_bytes(slice, Align::from_bytes(1).unwrap())
}

pub fn undef(size: Size, align: Align) -> Self {
pub fn uninit(size: Size, align: Align) -> Self {
Allocation {
bytes: vec![0; size.bytes_usize()],
relocations: Relocations::new(),
Expand Down Expand Up @@ -153,7 +153,7 @@ impl<Tag, Extra> Allocation<Tag, Extra> {
self.size.bytes_usize()
}

/// Looks at a slice which may describe undefined bytes or describe a relocation. This differs
/// Looks at a slice which may describe uninitialized bytes or describe a relocation. This differs
/// from `get_bytes_with_undef_and_ptr` in that it does no relocation checks (even on the
/// edges) at all. It further ignores `AllocationExtra` callbacks.
/// This must not be used for reads affecting the interpreter execution.
Expand Down Expand Up @@ -192,7 +192,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
offset.bytes_usize()..end
}

/// The last argument controls whether we error out when there are undefined
/// The last argument controls whether we error out when there are uninitialized
/// or pointer bytes. You should never call this, call `get_bytes` or
/// `get_bytes_with_undef_and_ptr` instead,
///
Expand All @@ -206,12 +206,12 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size,
check_defined_and_ptr: bool,
check_init_and_ptr: bool,
) -> InterpResult<'tcx, &[u8]> {
let range = self.check_bounds(ptr.offset, size);

if check_defined_and_ptr {
self.check_defined(ptr, size)?;
if check_init_and_ptr {
self.check_init(ptr, size)?;
self.check_relocations(cx, ptr, size)?;
} else {
// We still don't want relocations on the *edges*.
Expand Down Expand Up @@ -239,7 +239,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
self.get_bytes_internal(cx, ptr, size, true)
}

/// It is the caller's responsibility to handle undefined and pointer bytes.
/// It is the caller's responsibility to handle uninitialized and pointer bytes.
/// However, this still checks that there are no relocations on the *edges*.
///
/// It is the caller's responsibility to check bounds and alignment beforehand.
Expand Down Expand Up @@ -267,7 +267,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
) -> InterpResult<'tcx, &mut [u8]> {
let range = self.check_bounds(ptr.offset, size);

self.mark_definedness(ptr, size, true);
self.mark_init(ptr, size, true);
self.clear_relocations(cx, ptr, size)?;

AllocationExtra::memory_written(self, ptr, size)?;
Expand Down Expand Up @@ -303,7 +303,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {

/// Validates that `ptr.offset` and `ptr.offset + size` do not point to the middle of a
/// relocation. If `allow_ptr_and_undef` is `false`, also enforces that the memory in the
/// given range contains neither relocations nor undef bytes.
/// given range contains neither relocations nor uninitialized bytes.
pub fn check_bytes(
&self,
cx: &impl HasDataLayout,
Expand All @@ -313,9 +313,9 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
) -> InterpResult<'tcx> {
// Check bounds and relocations on the edges.
self.get_bytes_with_undef_and_ptr(cx, ptr, size)?;
// Check undef and ptr.
// Check uninit and ptr.
if !allow_ptr_and_undef {
self.check_defined(ptr, size)?;
self.check_init(ptr, size)?;
self.check_relocations(cx, ptr, size)?;
}
Ok(())
Expand Down Expand Up @@ -364,7 +364,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size)?;
// Uninit check happens *after* we established that the alignment is correct.
// We must not return `Ok()` for unaligned pointers!
if self.is_defined(ptr, size).is_err() {
if self.is_init(ptr, size).is_err() {
// This inflates uninitialized bytes to the entire scalar, even if only a few
// bytes are uninitialized.
return Ok(ScalarMaybeUninit::Uninit);
Expand Down Expand Up @@ -416,7 +416,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
let val = match val {
ScalarMaybeUninit::Scalar(scalar) => scalar,
ScalarMaybeUninit::Uninit => {
self.mark_definedness(ptr, type_size, false);
self.mark_init(ptr, type_size, false);
return Ok(());
}
};
Expand Down Expand Up @@ -512,7 +512,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
let start = ptr.offset;
let end = start + size; // `Size` addition

// Mark parts of the outermost relocations as undefined if they partially fall outside the
// Mark parts of the outermost relocations as uninitialized if they partially fall outside the
// given range.
if first < start {
self.init_mask.set_range(first, start, false);
Expand Down Expand Up @@ -542,20 +542,20 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
}
}

/// Undefined bytes.
/// Uninitialized bytes.
impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
/// Checks whether the given range is entirely defined.
/// Checks whether the given range is entirely initialized.
///
/// Returns `Ok(())` if it's defined. Otherwise returns the range of byte
/// indexes of the first contiguous undefined access.
fn is_defined(&self, ptr: Pointer<Tag>, size: Size) -> Result<(), Range<Size>> {
/// Returns `Ok(())` if it's initialized. Otherwise returns the range of byte
/// indexes of the first contiguous uninitialized access.
fn is_init(&self, ptr: Pointer<Tag>, size: Size) -> Result<(), Range<Size>> {
self.init_mask.is_range_initialized(ptr.offset, ptr.offset + size) // `Size` addition
}

/// Checks that a range of bytes is defined. If not, returns the `InvalidUndefBytes`
/// error which will report the first range of bytes which is undefined.
fn check_defined(&self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
self.is_defined(ptr, size).or_else(|idx_range| {
/// Checks that a range of bytes is initialized. If not, returns the `InvalidUninitBytes`
/// error which will report the first range of bytes which is uninitialized.
fn check_init(&self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
self.is_init(ptr, size).or_else(|idx_range| {
throw_ub!(InvalidUninitBytes(Some(Box::new(UninitBytesAccess {
access_ptr: ptr.erase_tag(),
access_size: size,
Expand All @@ -565,44 +565,44 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
})
}

pub fn mark_definedness(&mut self, ptr: Pointer<Tag>, size: Size, new_state: bool) {
pub fn mark_init(&mut self, ptr: Pointer<Tag>, size: Size, is_init: bool) {
if size.bytes() == 0 {
return;
}
self.init_mask.set_range(ptr.offset, ptr.offset + size, new_state);
self.init_mask.set_range(ptr.offset, ptr.offset + size, is_init);
}
}

/// Run-length encoding of the undef mask.
/// Run-length encoding of the uninit mask.
/// Used to copy parts of a mask multiple times to another allocation.
pub struct AllocationDefinedness {
/// The definedness of the first range.
pub struct InitMaskCompressed {
/// Whether the first range is initialized.
initial: bool,
/// The lengths of ranges that are run-length encoded.
/// The definedness of the ranges alternate starting with `initial`.
/// The initialization state of the ranges alternate starting with `initial`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// The initialization state of the ranges alternate starting with `initial`.
/// The initialization state of the ranges alternates starting with `initial`.

ranges: smallvec::SmallVec<[u64; 1]>,
}

impl AllocationDefinedness {
pub fn all_bytes_undef(&self) -> bool {
// The `ranges` are run-length encoded and of alternating definedness.
// So if `ranges.len() > 1` then the second block is a range of defined.
impl InitMaskCompressed {
pub fn no_bytes_init(&self) -> bool {
// The `ranges` are run-length encoded and of alternating initialization state.
// So if `ranges.len() > 1` then the second block is an initialized range.
!self.initial && self.ranges.len() == 1
}
}

/// Transferring the definedness mask to other allocations.
/// Transferring the initialization mask to other allocations.
impl<Tag, Extra> Allocation<Tag, Extra> {
/// Creates a run-length encoding of the undef mask.
pub fn compress_undef_range(&self, src: Pointer<Tag>, size: Size) -> AllocationDefinedness {
/// Creates a run-length encoding of the initialization mask.
pub fn compress_undef_range(&self, src: Pointer<Tag>, size: Size) -> InitMaskCompressed {
// Since we are copying `size` bytes from `src` to `dest + i * size` (`for i in 0..repeat`),
// a naive undef mask copying algorithm would repeatedly have to read the undef mask from
// a naive initialization mask copying algorithm would repeatedly have to read the initialization mask from
// the source and write it to the destination. Even if we optimized the memory accesses,
// we'd be doing all of this `repeat` times.
// Therefore we precompute a compressed version of the undef mask of the source value and
// Therefore we precompute a compressed version of the initialization mask of the source value and
// then write it back `repeat` times without computing any more information from the source.

// A precomputed cache for ranges of defined/undefined bits
// A precomputed cache for ranges of initialized / uninitialized bits
// 0000010010001110 will become
// `[5, 1, 2, 1, 3, 3, 1]`,
// where each element toggles the state.
Expand All @@ -613,7 +613,7 @@ impl<Tag, Extra> Allocation<Tag, Extra> {
let mut cur = initial;

for i in 1..size.bytes() {
// FIXME: optimize to bitshift the current undef block's bits and read the top bit.
// FIXME: optimize to bitshift the current uninitialized block's bits and read the top bit.
if self.init_mask.get(src.offset + Size::from_bytes(i)) == cur {
cur_len += 1;
} else {
Expand All @@ -625,13 +625,13 @@ impl<Tag, Extra> Allocation<Tag, Extra> {

ranges.push(cur_len);

AllocationDefinedness { ranges, initial }
InitMaskCompressed { ranges, initial }
}

/// Applies multiple instances of the run-length encoding to the undef mask.
pub fn mark_compressed_undef_range(
/// Applies multiple instances of the run-length encoding to the initialization mask.
pub fn mark_compressed_init_range(
&mut self,
defined: &AllocationDefinedness,
defined: &InitMaskCompressed,
dest: Pointer<Tag>,
size: Size,
repeat: u64,
Expand Down Expand Up @@ -740,7 +740,7 @@ impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
}

////////////////////////////////////////////////////////////////////////////////
// Undefined byte tracking
// Uninitialized byte tracking
////////////////////////////////////////////////////////////////////////////////

type Block = u64;
Expand Down Expand Up @@ -778,11 +778,11 @@ impl InitMask {

match idx {
Some(idx) => {
let undef_end = (idx.bytes()..end.bytes())
let uninit_end = (idx.bytes()..end.bytes())
.map(Size::from_bytes)
.find(|&i| self.get(i))
.unwrap_or(end);
Err(idx..undef_end)
Err(idx..uninit_end)
}
None => Ok(()),
}
Expand Down
30 changes: 15 additions & 15 deletions src/librustc_middle/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ impl<'tcx, Tag> ScalarMaybeUninit<Tag> {
}

#[inline]
pub fn not_undef(self) -> InterpResult<'static, Scalar<Tag>> {
pub fn check_init(self) -> InterpResult<'static, Scalar<Tag>> {
match self {
ScalarMaybeUninit::Scalar(scalar) => Ok(scalar),
ScalarMaybeUninit::Uninit => throw_ub!(InvalidUninitBytes(None)),
Expand All @@ -615,72 +615,72 @@ impl<'tcx, Tag> ScalarMaybeUninit<Tag> {

#[inline(always)]
pub fn to_bool(self) -> InterpResult<'tcx, bool> {
self.not_undef()?.to_bool()
self.check_init()?.to_bool()
}

#[inline(always)]
pub fn to_char(self) -> InterpResult<'tcx, char> {
self.not_undef()?.to_char()
self.check_init()?.to_char()
}

#[inline(always)]
pub fn to_f32(self) -> InterpResult<'tcx, Single> {
self.not_undef()?.to_f32()
self.check_init()?.to_f32()
}

#[inline(always)]
pub fn to_f64(self) -> InterpResult<'tcx, Double> {
self.not_undef()?.to_f64()
self.check_init()?.to_f64()
}

#[inline(always)]
pub fn to_u8(self) -> InterpResult<'tcx, u8> {
self.not_undef()?.to_u8()
self.check_init()?.to_u8()
}

#[inline(always)]
pub fn to_u16(self) -> InterpResult<'tcx, u16> {
self.not_undef()?.to_u16()
self.check_init()?.to_u16()
}

#[inline(always)]
pub fn to_u32(self) -> InterpResult<'tcx, u32> {
self.not_undef()?.to_u32()
self.check_init()?.to_u32()
}

#[inline(always)]
pub fn to_u64(self) -> InterpResult<'tcx, u64> {
self.not_undef()?.to_u64()
self.check_init()?.to_u64()
}

#[inline(always)]
pub fn to_machine_usize(self, cx: &impl HasDataLayout) -> InterpResult<'tcx, u64> {
self.not_undef()?.to_machine_usize(cx)
self.check_init()?.to_machine_usize(cx)
}

#[inline(always)]
pub fn to_i8(self) -> InterpResult<'tcx, i8> {
self.not_undef()?.to_i8()
self.check_init()?.to_i8()
}

#[inline(always)]
pub fn to_i16(self) -> InterpResult<'tcx, i16> {
self.not_undef()?.to_i16()
self.check_init()?.to_i16()
}

#[inline(always)]
pub fn to_i32(self) -> InterpResult<'tcx, i32> {
self.not_undef()?.to_i32()
self.check_init()?.to_i32()
}

#[inline(always)]
pub fn to_i64(self) -> InterpResult<'tcx, i64> {
self.not_undef()?.to_i64()
self.check_init()?.to_i64()
}

#[inline(always)]
pub fn to_machine_isize(self, cx: &impl HasDataLayout) -> InterpResult<'tcx, i64> {
self.not_undef()?.to_machine_isize(cx)
self.check_init()?.to_machine_isize(cx)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ pub(super) fn op_to_const<'tcx>(
ScalarMaybeUninit::Uninit => to_const_value(op.assert_mem_place(ecx)),
},
Immediate::ScalarPair(a, b) => {
let (data, start) = match a.not_undef().unwrap() {
let (data, start) = match a.check_init().unwrap() {
Scalar::Ptr(ptr) => {
(ecx.tcx.global_alloc(ptr.alloc_id).unwrap_memory(), ptr.offset.bytes())
}
Expand Down
Loading