Skip to content

Commit 65b1432

Browse files
RalfJungRalfJung
committed
interpret: refactor function call handling to be better-abstracted
1 parent 67e7de3 commit 65b1432

13 files changed

+72
-83
lines changed

‎src/concurrency/thread.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ pub struct Thread<'tcx> {
256256
/// which then forwards it to 'Resume'. However this argument is implicit in MIR,
257257
/// so we have to store it out-of-band. When there are multiple active unwinds,
258258
/// the innermost one is always caught first, so we can store them as a stack.
259-
pub(crate) panic_payloads: Vec<Scalar>,
259+
pub(crate) panic_payloads: Vec<ImmTy<'tcx>>,
260260

261261
/// Last OS error location in memory. It is a 32-bit integer.
262262
pub(crate) last_error: Option<MPlaceTy<'tcx>>,
@@ -952,7 +952,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
952952
this.call_function(
953953
instance,
954954
start_abi,
955-
&[*func_arg],
955+
&[func_arg],
956956
Some(&ret_place),
957957
StackPopCleanup::Root { cleanup: true },
958958
)?;

‎src/eval.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,8 @@ pub fn create_ecx<'tcx>(
307307
// First argument is constructed later, because it's skipped if the entry function uses #[start].
308308

309309
// Second argument (argc): length of `config.args`.
310-
let argc = Scalar::from_target_usize(u64::try_from(config.args.len()).unwrap(), &ecx);
310+
let argc =
311+
ImmTy::from_int(i64::try_from(config.args.len()).unwrap(), ecx.machine.layouts.isize);
311312
// Third argument (`argv`): created from `config.args`.
312313
let argv = {
313314
// Put each argument in memory, collect pointers.
@@ -334,21 +335,19 @@ pub fn create_ecx<'tcx>(
334335
ecx.write_immediate(arg, &place)?;
335336
}
336337
ecx.mark_immutable(&argvs_place);
337-
// A pointer to that place is the 3rd argument for main.
338-
let argv = argvs_place.to_ref(&ecx);
339338
// Store `argc` and `argv` for macOS `_NSGetArg{c,v}`.
340339
{
341340
let argc_place =
342341
ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
343-
ecx.write_scalar(argc, &argc_place)?;
342+
ecx.write_immediate(*argc, &argc_place)?;
344343
ecx.mark_immutable(&argc_place);
345344
ecx.machine.argc = Some(argc_place.ptr());
346345

347346
let argv_place = ecx.allocate(
348347
ecx.layout_of(Ty::new_imm_ptr(tcx, tcx.types.unit))?,
349348
MiriMemoryKind::Machine.into(),
350349
)?;
351-
ecx.write_immediate(argv, &argv_place)?;
350+
ecx.write_pointer(argvs_place.ptr(), &argv_place)?;
352351
ecx.mark_immutable(&argv_place);
353352
ecx.machine.argv = Some(argv_place.ptr());
354353
}
@@ -369,7 +368,7 @@ pub fn create_ecx<'tcx>(
369368
}
370369
ecx.mark_immutable(&cmd_place);
371370
}
372-
argv
371+
ecx.mplace_to_ref(&argvs_place)?
373372
};
374373

375374
// Return place (in static memory so that it does not count as leak).
@@ -405,10 +404,14 @@ pub fn create_ecx<'tcx>(
405404
start_instance,
406405
Abi::Rust,
407406
&[
408-
Scalar::from_pointer(main_ptr, &ecx).into(),
409-
argc.into(),
407+
ImmTy::from_scalar(
408+
Scalar::from_pointer(main_ptr, &ecx),
409+
// FIXME use a proper fn ptr type
410+
ecx.machine.layouts.const_raw_ptr,
411+
),
412+
argc,
410413
argv,
411-
Scalar::from_u8(sigpipe).into(),
414+
ImmTy::from_uint(sigpipe, ecx.machine.layouts.u8),
412415
],
413416
Some(&ret_place),
414417
StackPopCleanup::Root { cleanup: true },
@@ -418,7 +421,7 @@ pub fn create_ecx<'tcx>(
418421
ecx.call_function(
419422
entry_instance,
420423
Abi::Rust,
421-
&[argc.into(), argv],
424+
&[argc, argv],
422425
Some(&ret_place),
423426
StackPopCleanup::Root { cleanup: true },
424427
)?;

‎src/helpers.rs

Lines changed: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,14 @@ use rustc_apfloat::Float;
1212
use rustc_hir::{
1313
def::{DefKind, Namespace},
1414
def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE},
15+
Safety,
1516
};
1617
use rustc_index::IndexVec;
1718
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
1819
use rustc_middle::middle::dependency_format::Linkage;
1920
use rustc_middle::middle::exported_symbols::ExportedSymbol;
2021
use rustc_middle::mir;
21-
use rustc_middle::ty::layout::MaybeResult;
22+
use rustc_middle::ty::layout::{FnAbiOf, MaybeResult};
2223
use rustc_middle::ty::{
2324
self,
2425
layout::{LayoutOf, TyAndLayout},
@@ -492,48 +493,38 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
492493
&mut self,
493494
f: ty::Instance<'tcx>,
494495
caller_abi: Abi,
495-
args: &[Immediate<Provenance>],
496+
args: &[ImmTy<'tcx>],
496497
dest: Option<&MPlaceTy<'tcx>>,
497498
stack_pop: StackPopCleanup,
498499
) -> InterpResult<'tcx> {
499500
let this = self.eval_context_mut();
500-
let param_env = ty::ParamEnv::reveal_all(); // in Miri this is always the param_env we use... and this.param_env is private.
501-
let callee_abi = f.ty(*this.tcx, param_env).fn_sig(*this.tcx).abi();
502-
if callee_abi != caller_abi {
503-
throw_ub_format!(
504-
"calling a function with ABI {} using caller ABI {}",
505-
callee_abi.name(),
506-
caller_abi.name()
507-
)
508-
}
509501

510-
// Push frame.
502+
// Get MIR.
511503
let mir = this.load_mir(f.def, None)?;
512504
let dest = match dest {
513505
Some(dest) => dest.clone(),
514506
None => MPlaceTy::fake_alloc_zst(this.layout_of(mir.return_ty())?),
515507
};
516-
this.push_stack_frame(f, mir, &dest, stack_pop)?;
517-
518-
// Initialize arguments.
519-
let mut callee_args = this.frame().body.args_iter();
520-
for arg in args {
521-
let local = callee_args
522-
.next()
523-
.ok_or_else(|| err_ub_format!("callee has fewer arguments than expected"))?;
524-
// Make the local live, and insert the initial value.
525-
this.storage_live(local)?;
526-
let callee_arg = this.local_to_place(local)?;
527-
this.write_immediate(*arg, &callee_arg)?;
528-
}
529-
if callee_args.next().is_some() {
530-
throw_ub_format!("callee has more arguments than expected");
531-
}
532-
533-
// Initialize remaining locals.
534-
this.storage_live_for_always_live_locals()?;
535508

536-
Ok(())
509+
// Construct a function pointer type representing the caller perspective.
510+
let sig = this.tcx.mk_fn_sig(
511+
args.iter().map(|a| a.layout.ty),
512+
dest.layout.ty,
513+
/*c_variadic*/ false,
514+
Safety::Safe,
515+
caller_abi,
516+
);
517+
let caller_fn_abi = this.fn_abi_of_fn_ptr(ty::Binder::dummy(sig), ty::List::empty())?;
518+
519+
this.init_stack_frame(
520+
f,
521+
mir,
522+
caller_fn_abi,
523+
&args.iter().map(|a| FnArg::Copy(a.clone().into())).collect::<Vec<_>>(),
524+
/*with_caller_location*/ false,
525+
&dest,
526+
stack_pop,
527+
)
537528
}
538529

539530
/// Visits the memory covered by `place`, sensitive to freezing: the 2nd parameter

‎src/shims/panic.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub struct CatchUnwindData<'tcx> {
2525
/// The `catch_fn` callback to call in case of a panic.
2626
catch_fn: Pointer,
2727
/// The `data` argument for that callback.
28-
data: Scalar,
28+
data: ImmTy<'tcx>,
2929
/// The return place from the original call to `try`.
3030
dest: MPlaceTy<'tcx>,
3131
/// The return block from the original call to `try`.
@@ -50,7 +50,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
5050

5151
trace!("miri_start_unwind: {:?}", this.frame().instance);
5252

53-
let payload = this.read_scalar(payload)?;
53+
let payload = this.read_immediate(payload)?;
5454
let thread = this.active_thread_mut();
5555
thread.panic_payloads.push(payload);
5656

@@ -80,7 +80,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
8080
// Get all the arguments.
8181
let [try_fn, data, catch_fn] = check_arg_count(args)?;
8282
let try_fn = this.read_pointer(try_fn)?;
83-
let data = this.read_scalar(data)?;
83+
let data = this.read_immediate(data)?;
8484
let catch_fn = this.read_pointer(catch_fn)?;
8585

8686
// Now we make a function call, and pass `data` as first and only argument.
@@ -89,7 +89,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
8989
this.call_function(
9090
f_instance,
9191
Abi::Rust,
92-
&[data.into()],
92+
&[data.clone()],
9393
None,
9494
// Directly return to caller.
9595
StackPopCleanup::Goto { ret, unwind: mir::UnwindAction::Continue },
@@ -140,7 +140,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
140140
this.call_function(
141141
f_instance,
142142
Abi::Rust,
143-
&[catch_unwind.data.into(), payload.into()],
143+
&[catch_unwind.data, payload],
144144
None,
145145
// Directly return to caller of `try`.
146146
StackPopCleanup::Goto {
@@ -169,7 +169,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
169169
this.call_function(
170170
panic,
171171
Abi::Rust,
172-
&[msg.to_ref(this)],
172+
&[this.mplace_to_ref(&msg)?],
173173
None,
174174
StackPopCleanup::Goto { ret: None, unwind },
175175
)
@@ -188,7 +188,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
188188
this.call_function(
189189
panic,
190190
Abi::Rust,
191-
&[msg.to_ref(this)],
191+
&[this.mplace_to_ref(&msg)?],
192192
None,
193193
StackPopCleanup::Goto { ret: None, unwind: mir::UnwindAction::Unreachable },
194194
)
@@ -207,17 +207,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
207207
// Forward to `panic_bounds_check` lang item.
208208

209209
// First arg: index.
210-
let index = this.read_scalar(&this.eval_operand(index, None)?)?;
210+
let index = this.read_immediate(&this.eval_operand(index, None)?)?;
211211
// Second arg: len.
212-
let len = this.read_scalar(&this.eval_operand(len, None)?)?;
212+
let len = this.read_immediate(&this.eval_operand(len, None)?)?;
213213

214214
// Call the lang item.
215215
let panic_bounds_check = this.tcx.lang_items().panic_bounds_check_fn().unwrap();
216216
let panic_bounds_check = ty::Instance::mono(this.tcx.tcx, panic_bounds_check);
217217
this.call_function(
218218
panic_bounds_check,
219219
Abi::Rust,
220-
&[index.into(), len.into()],
220+
&[index, len],
221221
None,
222222
StackPopCleanup::Goto { ret: None, unwind },
223223
)?;
@@ -226,9 +226,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
226226
// Forward to `panic_misaligned_pointer_dereference` lang item.
227227

228228
// First arg: required.
229-
let required = this.read_scalar(&this.eval_operand(required, None)?)?;
229+
let required = this.read_immediate(&this.eval_operand(required, None)?)?;
230230
// Second arg: found.
231-
let found = this.read_scalar(&this.eval_operand(found, None)?)?;
231+
let found = this.read_immediate(&this.eval_operand(found, None)?)?;
232232

233233
// Call the lang item.
234234
let panic_misaligned_pointer_dereference =
@@ -238,7 +238,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
238238
this.call_function(
239239
panic_misaligned_pointer_dereference,
240240
Abi::Rust,
241-
&[required.into(), found.into()],
241+
&[required, found],
242242
None,
243243
StackPopCleanup::Goto { ret: None, unwind },
244244
)?;

‎src/shims/tls.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,14 +315,16 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
315315
// FIXME: Technically, the reason should be `DLL_PROCESS_DETACH` when the main thread exits
316316
// but std treats both the same.
317317
let reason = this.eval_windows("c", "DLL_THREAD_DETACH");
318+
let null_ptr =
319+
ImmTy::from_scalar(Scalar::null_ptr(this), this.machine.layouts.const_raw_ptr);
318320

319321
// The signature of this function is `unsafe extern "system" fn(h: c::LPVOID, dwReason: c::DWORD, pv: c::LPVOID)`.
320322
// FIXME: `h` should be a handle to the current module and what `pv` should be is unknown
321323
// but both are ignored by std.
322324
this.call_function(
323325
thread_callback,
324326
Abi::System { unwind: false },
325-
&[Scalar::null_ptr(this).into(), reason.into(), Scalar::null_ptr(this).into()],
327+
&[null_ptr.clone(), ImmTy::from_scalar(reason, this.machine.layouts.u32), null_ptr],
326328
None,
327329
StackPopCleanup::Root { cleanup: true },
328330
)?;
@@ -343,7 +345,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
343345
this.call_function(
344346
instance,
345347
Abi::C { unwind: false },
346-
&[data.into()],
348+
&[ImmTy::from_scalar(data, this.machine.layouts.mut_raw_ptr)],
347349
None,
348350
StackPopCleanup::Root { cleanup: true },
349351
)?;
@@ -380,7 +382,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
380382
this.call_function(
381383
instance,
382384
Abi::C { unwind: false },
383-
&[ptr.into()],
385+
&[ImmTy::from_scalar(ptr, this.machine.layouts.mut_raw_ptr)],
384386
None,
385387
StackPopCleanup::Root { cleanup: true },
386388
)?;

‎src/shims/unix/thread.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use crate::*;
2-
use rustc_middle::ty::layout::LayoutOf;
32
use rustc_target::spec::abi::Abi;
43

54
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
@@ -24,7 +23,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
2423
start_routine,
2524
Abi::C { unwind: false },
2625
func_arg,
27-
this.layout_of(this.tcx.types.usize)?,
26+
this.machine.layouts.mut_raw_ptr,
2827
)?;
2928

3029
Ok(())

‎tests/fail-dep/concurrency/libc_pthread_create_too_few_args.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
//@ignore-target-windows: No pthreads on Windows
2+
//~^ERROR: calling a function with more arguments than it expected
23

34
//! The thread function must have exactly one argument.
45
56
use std::{mem, ptr};
67

78
extern "C" fn thread_start() -> *mut libc::c_void {
8-
panic!() //~ ERROR: callee has fewer arguments than expected
9+
panic!()
910
}
1011

1112
fn main() {

‎tests/fail-dep/concurrency/libc_pthread_create_too_few_args.stderr

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
1-
error: Undefined Behavior: callee has fewer arguments than expected
2-
--> $DIR/libc_pthread_create_too_few_args.rs:LL:CC
3-
|
4-
LL | panic!()
5-
| ^^^^^^^^ callee has fewer arguments than expected
1+
error: Undefined Behavior: calling a function with more arguments than it expected
62
|
3+
= note: calling a function with more arguments than it expected
4+
= note: (no span available)
75
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
86
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
97
= note: BACKTRACE on thread `unnamed-ID`:
10-
= note: inside `thread_start` at RUSTLIB/core/src/panic.rs:LL:CC
11-
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
128

139
error: aborting due to 1 previous error
1410

‎tests/fail-dep/concurrency/libc_pthread_create_too_many_args.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
//@ignore-target-windows: No pthreads on Windows
2+
//~^ERROR: calling a function with fewer arguments than it requires
23

34
//! The thread function must have exactly one argument.
45
56
use std::{mem, ptr};
67

78
extern "C" fn thread_start(_null: *mut libc::c_void, _x: i32) -> *mut libc::c_void {
8-
panic!() //~ ERROR: callee has more arguments than expected
9+
panic!()
910
}
1011

1112
fn main() {

‎tests/fail-dep/concurrency/libc_pthread_create_too_many_args.stderr

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
1-
error: Undefined Behavior: callee has more arguments than expected
2-
--> $DIR/libc_pthread_create_too_many_args.rs:LL:CC
3-
|
4-
LL | panic!()
5-
| ^^^^^^^^ callee has more arguments than expected
1+
error: Undefined Behavior: calling a function with fewer arguments than it requires
62
|
3+
= note: calling a function with fewer arguments than it requires
4+
= note: (no span available)
75
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
86
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
97
= note: BACKTRACE on thread `unnamed-ID`:
10-
= note: inside `thread_start` at RUSTLIB/core/src/panic.rs:LL:CC
11-
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
128

139
error: aborting due to 1 previous error
1410

0 commit comments

Comments
 (0)