Skip to content

Commit 216499c

Browse files
committed
Auto merge of #133950 - matthiaskrgr:rollup-b7g5p73, r=matthiaskrgr
Rollup of 5 pull requests Successful merges: - #130777 (rust_for_linux: -Zreg-struct-return commandline flag for X86 (#116973)) - #133211 (Extend Miri to correctly pass mutable pointers through FFI) - #133790 (Improve documentation for Vec::extend_from_within) - #133930 (rustbook: update to use new mdbook-trpl package from The Book) - #133931 (Only allow PassMode::Direct for aggregates on wasm when using the C ABI) r? `@ghost` `@rustbot` modify labels: rollup
2 parents fa65b86 + f411e7c commit 216499c

File tree

11 files changed

+366
-47
lines changed

11 files changed

+366
-47
lines changed

src/alloc_addresses/mod.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
286286

287287
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
288288
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
289-
fn expose_ptr(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
290-
let this = self.eval_context_mut();
291-
let global_state = this.machine.alloc_addresses.get_mut();
289+
fn expose_ptr(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
290+
let this = self.eval_context_ref();
291+
let mut global_state = this.machine.alloc_addresses.borrow_mut();
292292
// In strict mode, we don't need this, so we can save some cycles by not tracking it.
293293
if global_state.provenance_mode == ProvenanceMode::Strict {
294294
return interp_ok(());
@@ -299,8 +299,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
299299
return interp_ok(());
300300
}
301301
trace!("Exposing allocation id {alloc_id:?}");
302-
let global_state = this.machine.alloc_addresses.get_mut();
303302
global_state.exposed.insert(alloc_id);
303+
// Release the global state before we call `expose_tag`, which may call `get_alloc_info_extra`,
304+
// which may need access to the global state.
305+
drop(global_state);
304306
if this.machine.borrow_tracker.is_some() {
305307
this.expose_tag(alloc_id, tag)?;
306308
}

src/borrow_tracker/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
302302
}
303303
}
304304

305-
fn expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
306-
let this = self.eval_context_mut();
305+
fn expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> {
306+
let this = self.eval_context_ref();
307307
let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method;
308308
match method {
309309
BorrowTrackerMethod::StackedBorrows => this.sb_expose_tag(alloc_id, tag),

src/borrow_tracker/stacked_borrows/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,8 +1011,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
10111011
}
10121012

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

10171017
// Function pointers and dead objects don't have an alloc_extra so we ignore them.
10181018
// This is okay because accessing them is UB anyway, no need for any Stacked Borrows checks.

src/borrow_tracker/tree_borrows/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -532,8 +532,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
532532
}
533533

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

538538
// Function pointers and dead objects don't have an alloc_extra so we ignore them.
539539
// This is okay because accessing them is UB anyway, no need for any Tree Borrows checks.

src/machine.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,9 @@ impl interpret::Provenance for Provenance {
270270
/// We use absolute addresses in the `offset` of a `StrictPointer`.
271271
const OFFSET_IS_ADDR: bool = true;
272272

273+
/// Miri implements wildcard provenance.
274+
const WILDCARD: Option<Self> = Some(Provenance::Wildcard);
275+
273276
fn get_alloc_id(self) -> Option<AllocId> {
274277
match self {
275278
Provenance::Concrete { alloc_id, .. } => Some(alloc_id),
@@ -1242,8 +1245,8 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
12421245
/// Called on `ptr as usize` casts.
12431246
/// (Actually computing the resulting `usize` doesn't need machine help,
12441247
/// that's just `Scalar::try_to_int`.)
1245-
fn expose_ptr(ecx: &mut InterpCx<'tcx, Self>, ptr: StrictPointer) -> InterpResult<'tcx> {
1246-
match ptr.provenance {
1248+
fn expose_provenance(ecx: &InterpCx<'tcx, Self>, provenance: Self::Provenance) -> InterpResult<'tcx> {
1249+
match provenance {
12471250
Provenance::Concrete { alloc_id, tag } => ecx.expose_ptr(alloc_id, tag),
12481251
Provenance::Wildcard => {
12491252
// No need to do anything for wildcard pointers as

src/shims/native_lib.rs

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@ use std::ops::Deref;
33

44
use libffi::high::call as ffi;
55
use libffi::low::CodePtr;
6-
use rustc_abi::{BackendRepr, HasDataLayout};
7-
use rustc_middle::ty::{self as ty, IntTy, UintTy};
6+
use rustc_abi::{BackendRepr, HasDataLayout, Size};
7+
use rustc_middle::{
8+
mir::interpret::Pointer,
9+
ty::{self as ty, IntTy, UintTy},
10+
};
811
use rustc_span::Symbol;
912

1013
use crate::*;
@@ -75,6 +78,11 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
7578
unsafe { ffi::call::<()>(ptr, libffi_args.as_slice()) };
7679
return interp_ok(ImmTy::uninit(dest.layout));
7780
}
81+
ty::RawPtr(..) => {
82+
let x = unsafe { ffi::call::<*const ()>(ptr, libffi_args.as_slice()) };
83+
let ptr = Pointer::new(Provenance::Wildcard, Size::from_bytes(x.addr()));
84+
Scalar::from_pointer(ptr, this)
85+
}
7886
_ => throw_unsup_format!("unsupported return type for native call: {:?}", link_name),
7987
};
8088
interp_ok(ImmTy::from_scalar(scalar, dest.layout))
@@ -152,8 +160,26 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
152160
if !matches!(arg.layout.backend_repr, BackendRepr::Scalar(_)) {
153161
throw_unsup_format!("only scalar argument types are support for native calls")
154162
}
155-
libffi_args.push(imm_to_carg(this.read_immediate(arg)?, this)?);
163+
let imm = this.read_immediate(arg)?;
164+
libffi_args.push(imm_to_carg(&imm, this)?);
165+
// If we are passing a pointer, prepare the memory it points to.
166+
if matches!(arg.layout.ty.kind(), ty::RawPtr(..)) {
167+
let ptr = imm.to_scalar().to_pointer(this)?;
168+
let Some(prov) = ptr.provenance else {
169+
// Pointer without provenance may not access any memory.
170+
continue;
171+
};
172+
// We use `get_alloc_id` for its best-effort behaviour with Wildcard provenance.
173+
let Some(alloc_id) = prov.get_alloc_id() else {
174+
// Wildcard pointer, whatever it points to must be already exposed.
175+
continue;
176+
};
177+
this.prepare_for_native_call(alloc_id, prov)?;
178+
}
156179
}
180+
181+
// FIXME: In the future, we should also call `prepare_for_native_call` on all previously
182+
// exposed allocations, since C may access any of them.
157183

158184
// Convert them to `libffi::high::Arg` type.
159185
let libffi_args = libffi_args
@@ -220,7 +246,7 @@ impl<'a> CArg {
220246

221247
/// Extract the scalar value from the result of reading a scalar from the machine,
222248
/// and convert it to a `CArg`.
223-
fn imm_to_carg<'tcx>(v: ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'tcx, CArg> {
249+
fn imm_to_carg<'tcx>(v: &ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'tcx, CArg> {
224250
interp_ok(match v.layout.ty.kind() {
225251
// If the primitive provided can be converted to a type matching the type pattern
226252
// then create a `CArg` of this primitive value with the corresponding `CArg` constructor.
@@ -238,18 +264,10 @@ fn imm_to_carg<'tcx>(v: ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'t
238264
ty::Uint(UintTy::U64) => CArg::UInt64(v.to_scalar().to_u64()?),
239265
ty::Uint(UintTy::Usize) =>
240266
CArg::USize(v.to_scalar().to_target_usize(cx)?.try_into().unwrap()),
241-
ty::RawPtr(_, mutability) => {
242-
// Arbitrary mutable pointer accesses are not currently supported in Miri.
243-
if mutability.is_mut() {
244-
throw_unsup_format!(
245-
"unsupported mutable pointer type for native call: {}",
246-
v.layout.ty
247-
);
248-
} else {
249-
let s = v.to_scalar().to_pointer(cx)?.addr();
250-
// This relies on the `expose_provenance` in `addr_from_alloc_id`.
251-
CArg::RawPtr(std::ptr::with_exposed_provenance_mut(s.bytes_usize()))
252-
}
267+
ty::RawPtr(..) => {
268+
let s = v.to_scalar().to_pointer(cx)?.addr();
269+
// This relies on the `expose_provenance` in `addr_from_alloc_id`.
270+
CArg::RawPtr(std::ptr::with_exposed_provenance_mut(s.bytes_usize()))
253271
}
254272
_ => throw_unsup_format!("unsupported argument type for native call: {}", v.layout.ty),
255273
})

tests/native-lib/pass/ptr_read_access.rs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,14 @@
33
//@only-on-host
44

55
fn main() {
6-
test_pointer();
7-
8-
test_simple();
9-
10-
test_nested();
11-
12-
test_static();
6+
test_access_pointer();
7+
test_access_simple();
8+
test_access_nested();
9+
test_access_static();
1310
}
1411

15-
// Test void function that dereferences a pointer and prints its contents from C.
16-
fn test_pointer() {
12+
/// Test function that dereferences an int pointer and prints its contents from C.
13+
fn test_access_pointer() {
1714
extern "C" {
1815
fn print_pointer(ptr: *const i32);
1916
}
@@ -23,8 +20,8 @@ fn test_pointer() {
2320
unsafe { print_pointer(&x) };
2421
}
2522

26-
// Test function that dereferences a simple struct pointer and accesses a field.
27-
fn test_simple() {
23+
/// Test function that dereferences a simple struct pointer and accesses a field.
24+
fn test_access_simple() {
2825
#[repr(C)]
2926
struct Simple {
3027
field: i32,
@@ -39,8 +36,8 @@ fn test_simple() {
3936
assert_eq!(unsafe { access_simple(&simple) }, -42);
4037
}
4138

42-
// Test function that dereferences nested struct pointers and accesses fields.
43-
fn test_nested() {
39+
/// Test function that dereferences nested struct pointers and accesses fields.
40+
fn test_access_nested() {
4441
use std::ptr::NonNull;
4542

4643
#[derive(Debug, PartialEq, Eq)]
@@ -61,8 +58,8 @@ fn test_nested() {
6158
assert_eq!(unsafe { access_nested(&nested_2) }, 97);
6259
}
6360

64-
// Test function that dereferences static struct pointers and accesses fields.
65-
fn test_static() {
61+
/// Test function that dereferences a static struct pointer and accesses fields.
62+
fn test_access_static() {
6663
#[repr(C)]
6764
struct Static {
6865
value: i32,

0 commit comments

Comments
 (0)