Skip to content
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

Extend Miri to correctly pass mutable pointers through FFI #133211

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/const_eval/dummy_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ impl<'tcx> interpret::Machine<'tcx> for DummyMachine {
})
}

fn expose_ptr(
_ecx: &mut InterpCx<'tcx, Self>,
_ptr: interpret::Pointer<Self::Provenance>,
fn expose_provenance(
_ecx: &InterpCx<'tcx, Self>,
_provenance: Self::Provenance,
) -> interpret::InterpResult<'tcx> {
unimplemented!()
}
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ use crate::errors::{LongRunning, LongRunningWarn};
use crate::fluent_generated as fluent;
use crate::interpret::{
self, AllocId, AllocRange, ConstAllocation, CtfeProvenance, FnArg, Frame, GlobalAlloc, ImmTy,
InterpCx, InterpResult, MPlaceTy, OpTy, Pointer, RangeSet, Scalar, compile_time_machine,
interp_ok, throw_exhaust, throw_inval, throw_ub, throw_ub_custom, throw_unsup,
throw_unsup_format,
InterpCx, InterpResult, MPlaceTy, OpTy, RangeSet, Scalar, compile_time_machine, interp_ok,
throw_exhaust, throw_inval, throw_ub, throw_ub_custom, throw_unsup, throw_unsup_format,
};

/// When hitting this many interpreted terminators we emit a deny by default lint
Expand Down Expand Up @@ -586,7 +585,10 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
}

#[inline(always)]
fn expose_ptr(_ecx: &mut InterpCx<'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx> {
fn expose_provenance(
_ecx: &InterpCx<'tcx, Self>,
_provenance: Self::Provenance,
) -> InterpResult<'tcx> {
// This is only reachable with -Zunleash-the-miri-inside-of-you.
throw_unsup_format!("exposing pointers is not possible at compile-time")
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let scalar = src.to_scalar();
let ptr = scalar.to_pointer(self)?;
match ptr.into_pointer_or_addr() {
Ok(ptr) => M::expose_ptr(self, ptr)?,
Ok(ptr) => M::expose_provenance(self, ptr.provenance)?,
Err(_) => {} // Do nothing, exposing an invalid pointer (`None` provenance) is a NOP.
};
interp_ok(ImmTy::from_scalar(
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,9 @@ pub trait Machine<'tcx>: Sized {

/// Marks a pointer as exposed, allowing it's provenance
/// to be recovered. "Pointer-to-int cast"
fn expose_ptr(
ecx: &mut InterpCx<'tcx, Self>,
ptr: Pointer<Self::Provenance>,
fn expose_provenance(
ecx: &InterpCx<'tcx, Self>,
provenance: Self::Provenance,
) -> InterpResult<'tcx>;

/// Convert a pointer with provenance into an allocation-offset pair and extra provenance info.
Expand Down
42 changes: 42 additions & 0 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,48 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
interp_ok(())
}

pub fn prepare_for_native_call(
&mut self,
id: AllocId,
initial_prov: M::Provenance,
) -> InterpResult<'tcx> {
// Expose provenance of the root allocation.
M::expose_provenance(self, initial_prov)?;

let mut done = rustc_data_structures::fx::FxHashSet::default();
let mut todo = vec![id];
while let Some(id) = todo.pop() {
if !done.insert(id) {
continue;
}
// This is a new allocation, add the allocations it points to to `todo`.
let info = self.get_alloc_info(id);

// If there is no data behind this pointer, skip this.
if !matches!(info.kind, AllocKind::LiveData) {
continue;
}

let alloc = self.get_alloc_raw(id)?;
for prov in alloc.provenance().provenances() {
M::expose_provenance(self, prov)?;
if let Some(id) = prov.get_alloc_id() {
todo.push(id);
}
}

// Prepare for possible write from native code if mutable.
if info.mutbl.is_mut() {
let tcx = self.tcx;
self.get_alloc_raw_mut(id)?
.0
.prepare_for_native_call(&tcx)
.map_err(|e| e.to_interp_error(id))?;
}
}
interp_ok(())
}

/// Create a lazy debug printer that prints the given allocation and all allocations it points
/// to, recursively.
#[must_use]
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_middle/src/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,28 @@ impl<Prov: Provenance, Extra, Bytes: AllocBytes> Allocation<Prov, Extra, Bytes>
Ok(())
}

/// Initialize all previously uninitialized bytes in the entire allocation, and set
/// provenance of everything to `Wildcard`. Before calling this, make sure all
/// provenance in this allocation is exposed!
pub fn prepare_for_native_call(&mut self, cx: &impl HasDataLayout) -> AllocResult {
let full_range = AllocRange { start: Size::ZERO, size: Size::from_bytes(self.len()) };
// Overwrite uninitialized bytes.
for chunk in self.init_mask.range_as_init_chunks(full_range) {
if !chunk.is_init() {
let uninit_bytes = &mut self.bytes
[chunk.range().start.bytes_usize()..chunk.range().end.bytes_usize()];
uninit_bytes.fill(0);
}
}
// Mark everything as initialized now.
self.mark_init(full_range, true);

// Set provenance of all bytes to wildcard.
self.provenance.write_wildcards(self.len(), cx);

Ok(())
}

/// Remove all provenance in the given memory range.
pub fn clear_provenance(&mut self, cx: &impl HasDataLayout, range: AllocRange) -> AllocResult {
self.provenance.clear(range, cx)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,28 @@

Ok(())
}

pub fn write_wildcards(&mut self, alloc_size: usize, cx: &impl HasDataLayout) {
// We can only write wildcards in Miri.
assert!(
Prov::OFFSET_IS_ADDR,
"writing wildcard provenance is not supported when `OFFSET_IS_ADDR` is false"
);
let wildcard = Prov::WILDCARD.unwrap();
let ptr_size = cx.data_layout().pointer_size;

// Write wildcards in intervals of pointer size.
let end = Size::from_bytes(alloc_size / ptr_size.bytes_usize());
for offset in Size::ZERO..end {
self.ptrs.insert(offset, wildcard);
}
// Write wildcards into the remaining bytes.
let last = Size::from_bytes(alloc_size);
let bytes = self.bytes.get_or_insert_with(Box::default);
for offset in end..last {
bytes.insert(offset, wildcard);
} // TODO: Is the above even remotely correct?

Check failure on line 218 in compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

TODO is used for tasks that should be done before merging a PR; If you want to leave a message in the codebase use FIXME
}
}

/// A partial, owned list of provenance to transfer into another allocation.
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/mir/interpret/pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ pub trait Provenance: Copy + fmt::Debug + 'static {
/// pointer, and implement ptr-to-int transmutation by stripping provenance.
const OFFSET_IS_ADDR: bool;

/// If wildcard provenance is implemented, contains the unique, general wildcard provenance variant.
const WILDCARD: Option<Self>;

/// Determines how a pointer should be printed.
fn fmt(ptr: &Pointer<Self>, f: &mut fmt::Formatter<'_>) -> fmt::Result;

Expand Down Expand Up @@ -168,6 +171,9 @@ impl Provenance for CtfeProvenance {
// so ptr-to-int casts are not possible (since we do not know the global physical offset).
const OFFSET_IS_ADDR: bool = false;

// `CtfeProvenance` does not implement wildcard provenance.
const WILDCARD: Option<Self> = None;

fn fmt(ptr: &Pointer<Self>, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// Print AllocId.
fmt::Debug::fmt(&ptr.provenance.alloc_id(), f)?; // propagates `alternate` flag
Expand Down Expand Up @@ -197,6 +203,9 @@ impl Provenance for AllocId {
// so ptr-to-int casts are not possible (since we do not know the global physical offset).
const OFFSET_IS_ADDR: bool = false;

// `AllocId` does not implement wildcard provenance.
const WILDCARD: Option<Self> = None;

fn fmt(ptr: &Pointer<Self>, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// Forward `alternate` flag to `alloc_id` printing.
if f.alternate() {
Expand Down
7 changes: 3 additions & 4 deletions src/tools/miri/src/alloc_addresses/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {

impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
fn expose_ptr(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let global_state = this.machine.alloc_addresses.get_mut();
fn expose_ptr(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
let this = self.eval_context_ref();
let mut global_state = this.machine.alloc_addresses.borrow_mut();
// In strict mode, we don't need this, so we can save some cycles by not tracking it.
if global_state.provenance_mode == ProvenanceMode::Strict {
return interp_ok(());
Expand All @@ -299,7 +299,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
return interp_ok(());
}
trace!("Exposing allocation id {alloc_id:?}");
let global_state = this.machine.alloc_addresses.get_mut();
global_state.exposed.insert(alloc_id);
if this.machine.borrow_tracker.is_some() {
this.expose_tag(alloc_id, tag)?;
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/borrow_tracker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}
}

fn expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
fn expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
let this = self.eval_context_ref();
let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method;
match method {
BorrowTrackerMethod::StackedBorrows => this.sb_expose_tag(alloc_id, tag),
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1011,8 +1011,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}

/// Mark the given tag as exposed. It was found on a pointer with the given AllocId.
fn sb_expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
fn sb_expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
let this = self.eval_context_ref();

// Function pointers and dead objects don't have an alloc_extra so we ignore them.
// This is okay because accessing them is UB anyway, no need for any Stacked Borrows checks.
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}

/// Mark the given tag as exposed. It was found on a pointer with the given AllocId.
fn tb_expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
fn tb_expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
let this = self.eval_context_ref();

// Function pointers and dead objects don't have an alloc_extra so we ignore them.
// This is okay because accessing them is UB anyway, no need for any Tree Borrows checks.
Expand Down
7 changes: 5 additions & 2 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,9 @@ impl interpret::Provenance for Provenance {
/// We use absolute addresses in the `offset` of a `StrictPointer`.
const OFFSET_IS_ADDR: bool = true;

/// Miri implements wildcard provenance.
const WILDCARD: Option<Self> = Some(Provenance::Wildcard);

fn get_alloc_id(self) -> Option<AllocId> {
match self {
Provenance::Concrete { alloc_id, .. } => Some(alloc_id),
Expand Down Expand Up @@ -1241,8 +1244,8 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
/// Called on `ptr as usize` casts.
/// (Actually computing the resulting `usize` doesn't need machine help,
/// that's just `Scalar::try_to_int`.)
fn expose_ptr(ecx: &mut InterpCx<'tcx, Self>, ptr: StrictPointer) -> InterpResult<'tcx> {
match ptr.provenance {
fn expose_provenance(ecx: &InterpCx<'tcx, Self>, provenance: Self::Provenance) -> InterpResult<'tcx> {
match provenance {
Provenance::Concrete { alloc_id, tag } => ecx.expose_ptr(alloc_id, tag),
Provenance::Wildcard => {
// No need to do anything for wildcard pointers as
Expand Down
38 changes: 24 additions & 14 deletions src/tools/miri/src/shims/native_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,26 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
if !matches!(arg.layout.backend_repr, BackendRepr::Scalar(_)) {
throw_unsup_format!("only scalar argument types are support for native calls")
}
libffi_args.push(imm_to_carg(this.read_immediate(arg)?, this)?);
let imm = this.read_immediate(arg)?;
libffi_args.push(imm_to_carg(&imm, this)?);
// If we are passing a pointer, prepare the memory it points to.
if matches!(arg.layout.ty.kind(), ty::RawPtr(..)) {
Strophox marked this conversation as resolved.
Show resolved Hide resolved
let ptr = imm.to_scalar().to_pointer(this)?;
let Some(prov) = ptr.provenance else {
// Pointer without provenance may not access any memory.
continue;
};
// We use `get_alloc_id` for its best-effort behaviour with Wildcard provenance.
let Some(alloc_id) = prov.get_alloc_id() else {
// Wildcard pointer, whatever it points to must be already exposed.
continue;
};
this.prepare_for_native_call(alloc_id, prov)?;
}
}
Strophox marked this conversation as resolved.
Show resolved Hide resolved

// FIXME: In the future, we should also call `prepare_for_native_call` on all previously
// exposed allocations, since C may access any of them.

// Convert them to `libffi::high::Arg` type.
let libffi_args = libffi_args
Expand Down Expand Up @@ -220,7 +238,7 @@ impl<'a> CArg {

/// Extract the scalar value from the result of reading a scalar from the machine,
/// and convert it to a `CArg`.
fn imm_to_carg<'tcx>(v: ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'tcx, CArg> {
fn imm_to_carg<'tcx>(v: &ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'tcx, CArg> {
interp_ok(match v.layout.ty.kind() {
// If the primitive provided can be converted to a type matching the type pattern
// then create a `CArg` of this primitive value with the corresponding `CArg` constructor.
Expand All @@ -238,18 +256,10 @@ fn imm_to_carg<'tcx>(v: ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'t
ty::Uint(UintTy::U64) => CArg::UInt64(v.to_scalar().to_u64()?),
ty::Uint(UintTy::Usize) =>
CArg::USize(v.to_scalar().to_target_usize(cx)?.try_into().unwrap()),
ty::RawPtr(_, mutability) => {
// Arbitrary mutable pointer accesses are not currently supported in Miri.
if mutability.is_mut() {
throw_unsup_format!(
"unsupported mutable pointer type for native call: {}",
v.layout.ty
);
} else {
let s = v.to_scalar().to_pointer(cx)?.addr();
// This relies on the `expose_provenance` in `addr_from_alloc_id`.
CArg::RawPtr(std::ptr::with_exposed_provenance_mut(s.bytes_usize()))
}
ty::RawPtr(..) => {
let s = v.to_scalar().to_pointer(cx)?.addr();
// This relies on the `expose_provenance` in `addr_from_alloc_id`.
CArg::RawPtr(std::ptr::with_exposed_provenance_mut(s.bytes_usize()))
}
_ => throw_unsup_format!("unsupported argument type for native call: {}", v.layout.ty),
})
Expand Down
Loading
Loading