Skip to content

Commit 06f4794

Browse files
borsbors
committed
Auto merge of #1804 - RalfJung:ptrless-allocs, r=RalfJung
update for Memory API changes The Miri side of rust-lang/rust#85376.
2 parents cda0b07 + d1e5eee commit 06f4794

File tree

12 files changed

+92
-97
lines changed

12 files changed

+92
-97
lines changed

‎rust-version

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
3e99439f4dacc8ba0d2ca48d221694362d587927
1+
3e827cc21e0734edd26170e8d1481f0d66a1426b

‎src/data_race.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
720720
if let Some(data_race) = &mut this.memory.extra.data_race {
721721
if data_race.multi_threaded.get() {
722722
let alloc_meta =
723-
this.memory.get_raw_mut(ptr.alloc_id)?.extra.data_race.as_mut().unwrap();
723+
this.memory.get_alloc_extra_mut(ptr.alloc_id)?.data_race.as_mut().unwrap();
724724
alloc_meta.reset_clocks(ptr.offset, size);
725725
}
726726
}
@@ -1024,7 +1024,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
10241024
let place_ptr = place.ptr.assert_ptr();
10251025
let size = place.layout.size;
10261026
let alloc_meta =
1027-
&this.memory.get_raw(place_ptr.alloc_id)?.extra.data_race.as_ref().unwrap();
1027+
&this.memory.get_alloc_extra(place_ptr.alloc_id)?.data_race.as_ref().unwrap();
10281028
log::trace!(
10291029
"Atomic op({}) with ordering {:?} on memory({:?}, offset={}, size={})",
10301030
description,

‎src/diagnostics.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,13 @@ pub fn report_error<'tcx, 'mir>(
136136

137137
// Extra output to help debug specific issues.
138138
match e.kind() {
139-
UndefinedBehavior(UndefinedBehaviorInfo::InvalidUninitBytes(Some(access))) => {
139+
UndefinedBehavior(UndefinedBehaviorInfo::InvalidUninitBytes(Some((alloc_id, access)))) => {
140140
eprintln!(
141141
"Uninitialized read occurred at offsets 0x{:x}..0x{:x} into this allocation:",
142-
access.uninit_ptr.offset.bytes(),
143-
access.uninit_ptr.offset.bytes() + access.uninit_size.bytes(),
142+
access.uninit_offset.bytes(),
143+
access.uninit_offset.bytes() + access.uninit_size.bytes(),
144144
);
145-
eprintln!("{:?}", ecx.memory.dump_alloc(access.uninit_ptr.alloc_id));
145+
eprintln!("{:?}", ecx.memory.dump_alloc(*alloc_id));
146146
}
147147
_ => {}
148148
}

‎src/helpers.rs

+10-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use log::trace;
88
use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX};
99
use rustc_middle::mir;
1010
use rustc_middle::ty::{self, layout::TyAndLayout, List, TyCtxt};
11-
use rustc_target::abi::{FieldsShape, LayoutOf, Size, Variants};
11+
use rustc_target::abi::{Align, FieldsShape, LayoutOf, Size, Variants};
1212
use rustc_target::spec::abi::Abi;
1313

1414
use rand::RngCore;
@@ -577,10 +577,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
577577
let ptr = this.force_ptr(sptr)?; // We need to read at least 1 byte, so we can eagerly get a ptr.
578578

579579
// Step 1: determine the length.
580-
let alloc = this.memory.get_raw(ptr.alloc_id)?;
581580
let mut len = Size::ZERO;
582581
loop {
583-
let byte = alloc.read_scalar(this, ptr.offset(len, this)?, size1)?.to_u8()?;
582+
// FIXME: We are re-getting the allocation each time around the loop.
583+
// Would be nice if we could somehow "extend" an existing AllocRange.
584+
let alloc = this.memory.get(ptr.offset(len, this)?.into(), size1, Align::ONE)?.unwrap(); // not a ZST, so we will get a result
585+
let byte = alloc.read_scalar(alloc_range(Size::ZERO, size1))?.to_u8()?;
584586
if byte == 0 {
585587
break;
586588
} else {
@@ -595,12 +597,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
595597
fn read_wide_str(&self, sptr: Scalar<Tag>) -> InterpResult<'tcx, Vec<u16>> {
596598
let this = self.eval_context_ref();
597599
let size2 = Size::from_bytes(2);
600+
let align2 = Align::from_bytes(2).unwrap();
598601

599602
let mut ptr = this.force_ptr(sptr)?; // We need to read at least 1 wchar, so we can eagerly get a ptr.
600603
let mut wchars = Vec::new();
601-
let alloc = this.memory.get_raw(ptr.alloc_id)?;
602604
loop {
603-
let wchar = alloc.read_scalar(this, ptr, size2)?.to_u16()?;
605+
// FIXME: We are re-getting the allocation each time around the loop.
606+
// Would be nice if we could somehow "extend" an existing AllocRange.
607+
let alloc = this.memory.get(ptr.into(), size2, align2)?.unwrap(); // not a ZST, so we will get a result
608+
let wchar = alloc.read_scalar(alloc_range(Size::ZERO, size2))?.to_u16()?;
604609
if wchar == 0 {
605610
break;
606611
} else {

‎src/machine.rs

+48-56
Original file line numberDiff line numberDiff line change
@@ -506,15 +506,57 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
506506
}
507507

508508
#[inline(always)]
509-
fn before_deallocation(
510-
memory_extra: &mut Self::MemoryExtra,
511-
id: AllocId,
509+
fn memory_read(
510+
_memory_extra: &Self::MemoryExtra,
511+
alloc: &Allocation<Tag, AllocExtra>,
512+
ptr: Pointer<Tag>,
513+
size: Size,
512514
) -> InterpResult<'tcx> {
513-
if Some(id) == memory_extra.tracked_alloc_id {
514-
register_diagnostic(NonHaltingDiagnostic::FreedAlloc(id));
515+
if let Some(data_race) = &alloc.extra.data_race {
516+
data_race.read(ptr, size)?;
515517
}
518+
if let Some(stacked_borrows) = &alloc.extra.stacked_borrows {
519+
stacked_borrows.memory_read(ptr, size)
520+
} else {
521+
Ok(())
522+
}
523+
}
516524

517-
Ok(())
525+
#[inline(always)]
526+
fn memory_written(
527+
_memory_extra: &mut Self::MemoryExtra,
528+
alloc: &mut Allocation<Tag, AllocExtra>,
529+
ptr: Pointer<Tag>,
530+
size: Size,
531+
) -> InterpResult<'tcx> {
532+
if let Some(data_race) = &mut alloc.extra.data_race {
533+
data_race.write(ptr, size)?;
534+
}
535+
if let Some(stacked_borrows) = &mut alloc.extra.stacked_borrows {
536+
stacked_borrows.memory_written(ptr, size)
537+
} else {
538+
Ok(())
539+
}
540+
}
541+
542+
#[inline(always)]
543+
fn memory_deallocated(
544+
memory_extra: &mut Self::MemoryExtra,
545+
alloc: &mut Allocation<Tag, AllocExtra>,
546+
ptr: Pointer<Tag>,
547+
) -> InterpResult<'tcx> {
548+
let size = alloc.size();
549+
if Some(ptr.alloc_id) == memory_extra.tracked_alloc_id {
550+
register_diagnostic(NonHaltingDiagnostic::FreedAlloc(ptr.alloc_id));
551+
}
552+
if let Some(data_race) = &mut alloc.extra.data_race {
553+
data_race.deallocate(ptr, size)?;
554+
}
555+
if let Some(stacked_borrows) = &mut alloc.extra.stacked_borrows {
556+
stacked_borrows.memory_deallocated(ptr, size)
557+
} else {
558+
Ok(())
559+
}
518560
}
519561

520562
fn after_static_mem_initialized(
@@ -601,53 +643,3 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
601643
intptrcast::GlobalState::ptr_to_int(ptr, memory)
602644
}
603645
}
604-
605-
impl AllocationExtra<Tag> for AllocExtra {
606-
#[inline(always)]
607-
fn memory_read<'tcx>(
608-
alloc: &Allocation<Tag, AllocExtra>,
609-
ptr: Pointer<Tag>,
610-
size: Size,
611-
) -> InterpResult<'tcx> {
612-
if let Some(data_race) = &alloc.extra.data_race {
613-
data_race.read(ptr, size)?;
614-
}
615-
if let Some(stacked_borrows) = &alloc.extra.stacked_borrows {
616-
stacked_borrows.memory_read(ptr, size)
617-
} else {
618-
Ok(())
619-
}
620-
}
621-
622-
#[inline(always)]
623-
fn memory_written<'tcx>(
624-
alloc: &mut Allocation<Tag, AllocExtra>,
625-
ptr: Pointer<Tag>,
626-
size: Size,
627-
) -> InterpResult<'tcx> {
628-
if let Some(data_race) = &mut alloc.extra.data_race {
629-
data_race.write(ptr, size)?;
630-
}
631-
if let Some(stacked_borrows) = &mut alloc.extra.stacked_borrows {
632-
stacked_borrows.memory_written(ptr, size)
633-
} else {
634-
Ok(())
635-
}
636-
}
637-
638-
#[inline(always)]
639-
fn memory_deallocated<'tcx>(
640-
alloc: &mut Allocation<Tag, AllocExtra>,
641-
ptr: Pointer<Tag>,
642-
size: Size,
643-
) -> InterpResult<'tcx> {
644-
if let Some(data_race) = &mut alloc.extra.data_race {
645-
data_race.deallocate(ptr, size)?;
646-
}
647-
if let Some(stacked_borrows) = &mut alloc.extra.stacked_borrows {
648-
stacked_borrows.memory_deallocated(ptr, size)
649-
} else {
650-
Ok(())
651-
}
652-
}
653-
}

‎src/shims/intrinsics.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
574574
// even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must
575575
// be 8-aligned).
576576
let align = Align::from_bytes(place.layout.size.bytes()).unwrap();
577-
this.memory.check_ptr_access(place.ptr, place.layout.size, align)?;
577+
this.memory.check_ptr_access_align(place.ptr, place.layout.size, align, CheckInAllocMsg::MemoryAccessTest)?;
578+
// Perform regular access.
578579
this.write_scalar(val, dest)?;
579580
Ok(())
580581
}
@@ -594,7 +595,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
594595
// even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must
595596
// be 8-aligned).
596597
let align = Align::from_bytes(place.layout.size.bytes()).unwrap();
597-
this.memory.check_ptr_access(place.ptr, place.layout.size, align)?;
598+
this.memory.check_ptr_access_align(place.ptr, place.layout.size, align, CheckInAllocMsg::MemoryAccessTest)?;
598599

599600
// Perform atomic store
600601
this.write_scalar_atomic(val, &place, atomic)?;
@@ -644,7 +645,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
644645
// even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must
645646
// be 8-aligned).
646647
let align = Align::from_bytes(place.layout.size.bytes()).unwrap();
647-
this.memory.check_ptr_access(place.ptr, place.layout.size, align)?;
648+
this.memory.check_ptr_access_align(place.ptr, place.layout.size, align, CheckInAllocMsg::MemoryAccessTest)?;
648649

649650
match atomic_op {
650651
AtomicOp::Min => {
@@ -681,7 +682,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
681682
// even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must
682683
// be 8-aligned).
683684
let align = Align::from_bytes(place.layout.size.bytes()).unwrap();
684-
this.memory.check_ptr_access(place.ptr, place.layout.size, align)?;
685+
this.memory.check_ptr_access_align(place.ptr, place.layout.size, align, CheckInAllocMsg::MemoryAccessTest)?;
685686

686687
let old = this.atomic_exchange_scalar(&place, new, atomic)?;
687688
this.write_scalar(old, dest)?; // old value is returned
@@ -707,7 +708,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
707708
// even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must
708709
// be 8-aligned).
709710
let align = Align::from_bytes(place.layout.size.bytes()).unwrap();
710-
this.memory.check_ptr_access(place.ptr, place.layout.size, align)?;
711+
this.memory.check_ptr_access_align(place.ptr, place.layout.size, align, CheckInAllocMsg::MemoryAccessTest)?;
711712

712713
let old = this.atomic_compare_exchange_scalar(
713714
&place,

‎src/shims/os_str.rs

+7-10
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::os::unix::ffi::{OsStrExt, OsStringExt};
99
#[cfg(windows)]
1010
use std::os::windows::ffi::{OsStrExt, OsStringExt};
1111

12-
use rustc_target::abi::{LayoutOf, Size};
12+
use rustc_target::abi::{Align, LayoutOf, Size};
1313

1414
use crate::*;
1515

@@ -144,17 +144,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
144144
// Store the UTF-16 string.
145145
let size2 = Size::from_bytes(2);
146146
let this = self.eval_context_mut();
147-
let tcx = &*this.tcx;
148-
let ptr = this.force_ptr(sptr)?; // we need to write at least the 0 terminator
149-
let alloc = this.memory.get_raw_mut(ptr.alloc_id)?;
147+
let mut alloc = this
148+
.memory
149+
.get_mut(sptr, Size::from_bytes(string_length), Align::from_bytes(2).unwrap())?
150+
.unwrap(); // not a ZST, so we will get a result
150151
for (offset, wchar) in u16_vec.into_iter().chain(iter::once(0x0000)).enumerate() {
151152
let offset = u64::try_from(offset).unwrap();
152-
alloc.write_scalar(
153-
tcx,
154-
ptr.offset(size2 * offset, tcx)?,
155-
Scalar::from_u16(wchar).into(),
156-
size2,
157-
)?;
153+
alloc
154+
.write_scalar(alloc_range(size2 * offset, size2), Scalar::from_u16(wchar).into())?;
158155
}
159156
Ok((true, string_length - 1))
160157
}

‎src/shims/posix/fs.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -677,10 +677,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
677677
trace!("Reading from FD {}, size {}", fd, count);
678678

679679
// Check that the *entire* buffer is actually valid memory.
680-
this.memory.check_ptr_access(
680+
this.memory.check_ptr_access_align(
681681
buf,
682682
Size::from_bytes(count),
683-
Align::from_bytes(1).unwrap(),
683+
Align::ONE,
684+
CheckInAllocMsg::MemoryAccessTest,
684685
)?;
685686

686687
// We cap the number of read bytes to the largest value that we are able to fit in both the
@@ -722,10 +723,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
722723
// Isolation check is done via `FileDescriptor` trait.
723724

724725
// Check that the *entire* buffer is actually valid memory.
725-
this.memory.check_ptr_access(
726+
this.memory.check_ptr_access_align(
726727
buf,
727728
Size::from_bytes(count),
728-
Align::from_bytes(1).unwrap(),
729+
Align::ONE,
730+
CheckInAllocMsg::MemoryAccessTest,
729731
)?;
730732

731733
// We cap the number of written bytes to the largest value that we are able to fit in both the

‎src/shims/posix/linux/sync.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,11 @@ pub fn futex<'tcx>(
8282
// Check the pointer for alignment and validity.
8383
// The API requires `addr` to be a 4-byte aligned pointer, and will
8484
// use the 4 bytes at the given address as an (atomic) i32.
85-
this.memory.check_ptr_access(
85+
this.memory.check_ptr_access_align(
8686
addr.to_scalar()?,
8787
Size::from_bytes(4),
8888
Align::from_bytes(4).unwrap(),
89+
CheckInAllocMsg::MemoryAccessTest,
8990
)?;
9091
// Read an `i32` through the pointer, regardless of any wrapper types.
9192
// It's not uncommon for `addr` to be passed as another type than `*mut i32`, such as `*const AtomicI32`.

‎src/stacked_borrows.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
561561
);
562562

563563
// Get the allocation. It might not be mutable, so we cannot use `get_mut`.
564-
let extra = &this.memory.get_raw(ptr.alloc_id)?.extra;
564+
let extra = this.memory.get_alloc_extra(ptr.alloc_id)?;
565565
let stacked_borrows =
566566
extra.stacked_borrows.as_ref().expect("we should have Stacked Borrows data");
567567
// Update the stacks.

‎tests/compile-fail/data_race/alloc_read_race.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// ignore-windows: Concurrency on Windows is not supported yet.
2+
#![feature(new_uninit)]
23

34
use std::thread::spawn;
45
use std::ptr::null_mut;
@@ -29,7 +30,7 @@ pub fn main() {
2930
// Uses relaxed semantics to not generate
3031
// a release sequence.
3132
let pointer = &*ptr.0;
32-
pointer.store(Box::into_raw(Box::new(MaybeUninit::uninit())), Ordering::Relaxed);
33+
pointer.store(Box::into_raw(Box::new_uninit()), Ordering::Relaxed);
3334
});
3435

3536
let j2 = spawn(move || {

‎tests/compile-fail/data_race/alloc_write_race.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// ignore-windows: Concurrency on Windows is not supported yet.
2+
#![feature(new_uninit)]
23

34
use std::thread::spawn;
45
use std::ptr::null_mut;
@@ -10,11 +11,6 @@ struct EvilSend<T>(pub T);
1011
unsafe impl<T> Send for EvilSend<T> {}
1112
unsafe impl<T> Sync for EvilSend<T> {}
1213

13-
extern "C" {
14-
fn malloc(size: usize) -> *mut u8;
15-
fn free(ptr: *mut u8);
16-
}
17-
1814
pub fn main() {
1915
// Shared atomic pointer
2016
let pointer = AtomicPtr::new(null_mut::<usize>());
@@ -33,7 +29,7 @@ pub fn main() {
3329
// Uses relaxed semantics to not generate
3430
// a release sequence.
3531
let pointer = &*ptr.0;
36-
pointer.store(malloc(std::mem::size_of::<usize>()) as *mut usize, Ordering::Relaxed);
32+
pointer.store(Box::into_raw(Box::<usize>::new_uninit()) as *mut usize, Ordering::Relaxed);
3733
});
3834

3935
let j2 = spawn(move || {
@@ -45,6 +41,6 @@ pub fn main() {
4541
j2.join().unwrap();
4642

4743
// Clean up memory, will never be executed
48-
free(pointer.load(Ordering::Relaxed) as *mut _);
44+
drop(Box::from_raw(pointer.load(Ordering::Relaxed)));
4945
}
5046
}

0 commit comments

Comments
 (0)