Skip to content

Commit 8745d7a

Browse files
committed
add prepared_alloc_bytes to GlobalStateInner, fn take_prepared_alloc_bytes, adapt Allocation::adjust_from_tcx
1 parent 6006ef8 commit 8745d7a

File tree

7 files changed

+71
-25
lines changed

7 files changed

+71
-25
lines changed

compiler/rustc_const_eval/src/interpret/machine.rs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -357,17 +357,7 @@ pub trait Machine<'tcx>: Sized {
357357
ecx: &InterpCx<'tcx, Self>,
358358
id: AllocId,
359359
alloc: &'b Allocation,
360-
) -> InterpResult<'tcx, Cow<'b, Allocation<Self::Provenance, Self::AllocExtra, Self::Bytes>>>
361-
{
362-
// The default implementation does a copy; CTFE machines have a more efficient implementation
363-
// based on their particular choice for `Provenance`, `AllocExtra`, and `Bytes`.
364-
let kind = Self::GLOBAL_KIND
365-
.expect("if GLOBAL_KIND is None, adjust_global_allocation must be overwritten");
366-
let alloc = alloc.adjust_from_tcx(&ecx.tcx, |ptr| ecx.global_root_pointer(ptr))?;
367-
let extra =
368-
Self::init_alloc_extra(ecx, id, MemoryKind::Machine(kind), alloc.size(), alloc.align)?;
369-
Ok(Cow::Owned(alloc.with_extra(extra)))
370-
}
360+
) -> InterpResult<'tcx, Cow<'b, Allocation<Self::Provenance, Self::AllocExtra, Self::Bytes>>>;
371361

372362
/// Initialize the extra state of an allocation.
373363
///

compiler/rustc_middle/src/mir/interpret/allocation.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,10 +358,11 @@ impl Allocation {
358358
pub fn adjust_from_tcx<Prov: Provenance, Bytes: AllocBytes, Err>(
359359
&self,
360360
cx: &impl HasDataLayout,
361+
mut alloc_bytes: impl FnMut(&[u8], Align) -> Bytes,
361362
mut adjust_ptr: impl FnMut(Pointer<CtfeProvenance>) -> Result<Pointer<Prov>, Err>,
362363
) -> Result<Allocation<Prov, (), Bytes>, Err> {
363364
// Copy the data.
364-
let mut bytes = Bytes::from_bytes(Cow::Borrowed(&*self.bytes), self.align);
365+
let mut bytes = alloc_bytes(&*self.bytes, self.align);
365366
// Adjust provenance of pointers stored in this allocation.
366367
let mut new_provenance = Vec::with_capacity(self.provenance.ptrs().len());
367368
let ptr_size = cx.data_layout().pointer_size.bytes_usize();

src/tools/miri/src/alloc_addresses/mod.rs

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ pub struct GlobalStateInner {
4242
/// they do not have an `AllocExtra`.
4343
/// This is the inverse of `int_to_ptr_map`.
4444
base_addr: FxHashMap<AllocId, u64>,
45+
// Temporarily store prepared memory space for global allocations the first time their memory address is required. This is used to ensure that the memory is allocated before Miri assigns it an internal address, which is important for matching the internal address to the machine address so FFI can read from pointers.
46+
prepared_alloc_bytes: FxHashMap<AllocId, MiriAllocBytes>,
4547
/// A pool of addresses we can reuse for future allocations.
4648
reuse: ReusePool,
4749
/// Whether an allocation has been exposed or not. This cannot be put
@@ -59,6 +61,7 @@ impl VisitProvenance for GlobalStateInner {
5961
let GlobalStateInner {
6062
int_to_ptr_map: _,
6163
base_addr: _,
64+
prepared_alloc_bytes: _,
6265
reuse: _,
6366
exposed: _,
6467
next_base_addr: _,
@@ -78,6 +81,7 @@ impl GlobalStateInner {
7881
GlobalStateInner {
7982
int_to_ptr_map: Vec::default(),
8083
base_addr: FxHashMap::default(),
84+
prepared_alloc_bytes: FxHashMap::default(),
8185
reuse: ReusePool::new(config),
8286
exposed: FxHashSet::default(),
8387
next_base_addr: stack_addr,
@@ -166,17 +170,22 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
166170
assert!(!matches!(kind, AllocKind::Dead));
167171

168172
// This allocation does not have a base address yet, pick or reuse one.
169-
let base_addr = if ecx.machine.native_lib.is_some() { // V1: memory.rs:212 InterpError(UseAfterFree...
170-
// let base_addr = if ecx.machine.native_lib.is_some() && kind != AllocKind::Function { // V2: /home/lwerner/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/rt.rs:159:9 UB???...
171-
// let base_addr = if ecx.machine.native_lib.is_some() && kind == AllocKind::LiveData { // V3: alloc_addresses/mod.rs:151 BorrowMutError...
172-
// Work with the real underlying machine address directly TODO https://doc.rust-lang.org/std/primitive.pointer.html
173+
let base_addr = if ecx.machine.native_lib.is_some() {
173174
match kind {
174175
AllocKind::LiveData => {
175-
let the = ecx.get_alloc_bytes_unchecked_raw(alloc_id);
176-
dbg!((1, &alloc_id, &kind, &the)); // TODO
177-
let the2 = the?.addr().try_into().unwrap();
178-
dbg!((2, &the2)); // TODO
179-
the2
176+
if ecx.tcx.try_get_global_alloc(alloc_id).is_some() {
177+
// For new global allocations, we always pre-allocate the memory to be able use the machine address directly.
178+
let prepared_bytes = MiriAllocBytes::zeroed(size, align)
179+
.unwrap_or_else(|| {
180+
panic!("Miri ran out of memory: cannot create allocation of {size:?} bytes")
181+
});
182+
let addr = prepared_bytes.as_ptr().addr().try_into().unwrap();
183+
// Store prepared allocation space to be picked up for use later.
184+
global_state.prepared_alloc_bytes.try_insert(alloc_id, prepared_bytes).unwrap();
185+
addr
186+
} else {
187+
ecx.get_alloc_bytes_unchecked_raw(alloc_id)?.addr().try_into().unwrap()
188+
}
180189
}
181190
AllocKind::Function | AllocKind::VTable => {
182191
let alloc_bytes = MiriAllocBytes::from_bytes(&[0u8; 1], Align::from_bytes(1).unwrap());
@@ -339,6 +348,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
339348
Ok(base_ptr.wrapping_offset(offset, ecx))
340349
}
341350

351+
// This returns some prepared `MiriAllocBytes`, either because `addr_from_alloc_id` was called in the past and reserved memory space, or by doing the pre-allocation right upon being called.
352+
fn take_prepared_alloc_bytes(&self, id: AllocId, kind: MemoryKind) -> InterpResult<'tcx, MiriAllocBytes> {
353+
let ecx = self.eval_context_ref();
354+
// This additional call ensures that some `MiriAllocBytes` are prepared.
355+
ecx.addr_from_alloc_id(id, kind)?;
356+
// TODO: Better debug message?
357+
let prepared_alloc_bytes = ecx.machine.alloc_addresses.borrow_mut().prepared_alloc_bytes.remove(&id).expect("alloc bytes for alloc_id could not prepared");
358+
Ok(prepared_alloc_bytes)
359+
}
360+
342361
/// When a pointer is used for a memory access, this computes where in which allocation the
343362
/// access is going.
344363
fn ptr_get_alloc(

src/tools/miri/src/concurrency/thread.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
887887
let alloc = this.ctfe_query(|tcx| tcx.eval_static_initializer(def_id))?;
888888
// We make a full copy of this allocation.
889889
let mut alloc =
890-
alloc.inner().adjust_from_tcx(&this.tcx, |ptr| this.global_root_pointer(ptr))?;
890+
alloc.inner().adjust_from_tcx(&this.tcx, |bytes, align| MiriAllocBytes::from_bytes(std::borrow::Cow::Borrowed(bytes), align), |ptr| this.global_root_pointer(ptr))?;
891891
// This allocation will be deallocated when the thread dies, so it is not in read-only memory.
892892
alloc.mutability = Mutability::Mut;
893893
// Create a fresh allocation with this content.

src/tools/miri/src/machine.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! Global machine state as well as implementation of the interpreter engine
22
//! `Machine` trait.
33
4+
use std::borrow::Cow;
45
use std::cell::RefCell;
56
use std::collections::hash_map::Entry;
67
use std::fmt;
@@ -1225,6 +1226,41 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
12251226
})
12261227
}
12271228

1229+
/// Called to adjust global allocations to the Provenance and AllocExtra of this machine.
1230+
///
1231+
/// If `alloc` contains pointers, then they are all pointing to globals.
1232+
///
1233+
/// This should avoid copying if no work has to be done! If this returns an owned
1234+
/// allocation (because a copy had to be done to adjust things), machine memory will
1235+
/// cache the result. (This relies on `AllocMap::get_or` being able to add the
1236+
/// owned allocation to the map even when the map is shared.)
1237+
fn adjust_global_allocation<'b>(
1238+
ecx: &InterpCx<'tcx, Self>,
1239+
id: AllocId,
1240+
alloc: &'b Allocation,
1241+
) -> InterpResult<'tcx, Cow<'b, Allocation<Self::Provenance, Self::AllocExtra, Self::Bytes>>>
1242+
{
1243+
// The default implementation does a copy; CTFE machines have a more efficient implementation
1244+
// based on their particular choice for `Provenance`, `AllocExtra`, and `Bytes`.
1245+
let kind = Self::GLOBAL_KIND
1246+
.expect("if GLOBAL_KIND is None, adjust_global_allocation must be overwritten");
1247+
// This closure is special in that it always relies on `take_prepared_alloc_bytes` to give it some reserved memory space.
1248+
let alloc = alloc.adjust_from_tcx(&ecx.tcx, |bytes, align| {
1249+
assert_eq!(alloc.align, align);
1250+
// TODO: is this unwrap warranted?
1251+
let mut alloc_bytes = ecx.take_prepared_alloc_bytes(id, MemoryKind::Machine(kind)).unwrap();
1252+
assert_eq!(alloc_bytes.len(), bytes.len());
1253+
// SAFETY: `alloc_bytes` and `bytes` span the same number of bytes.
1254+
unsafe { alloc_bytes.as_mut_ptr().copy_from(bytes.as_ptr(), bytes.len()) };
1255+
alloc_bytes
1256+
},
1257+
|ptr| ecx.global_root_pointer(ptr),
1258+
)?;
1259+
let extra =
1260+
Self::init_alloc_extra(ecx, id, MemoryKind::Machine(kind), alloc.size(), alloc.align)?;
1261+
Ok(Cow::Owned(alloc.with_extra(extra)))
1262+
}
1263+
12281264
#[inline(always)]
12291265
fn before_memory_read(
12301266
_tcx: TyCtxtAt<'tcx>,

src/tools/miri/tests/native-lib/pass/call_extern_c_fn.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ fn main() {
4747
// test void function that dereferences a pointer and prints from C
4848
let mut x = 42i32;
4949
let ptr = &mut x as *mut i32;
50-
//println!("pointer address from Rust: {:?}", ptr);
51-
//println!("printing dereference from Rust: {}", *ptr);
50+
println!("pointer address from Rust: {:?}", ptr);
51+
println!("printing dereference from Rust: {}", *ptr);
5252
ptr_printer(ptr);
5353

5454
}

src/tools/miri/tests/native-lib/test.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ void printer() {
1111
void ptr_printer(int *ptr) {
1212
printf("printing address from C: %p\n", ptr);
1313
// TODO: Get *this* to work.
14-
//printf("printing dereference from C: %d\n", *ptr);
14+
printf("printing dereference from C: %d\n", *ptr);
1515
}
1616

1717
// function with many arguments, to test functionality when some args are stored

0 commit comments

Comments
 (0)