Skip to content

interpret: when passing an argument fails, point at that argument #154263

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.

Sign up for GitHub

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

Open
RalfJung wants to merge 1 commit into
base: main
Choose a base branch
from
Open

interpret: when passing an argument fails, point at that argument #154263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/eval_queries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>(
&ret.clone().into(),
ReturnContinuation::Stop { cleanup: false },
)?;
ecx.storage_live_for_always_live_locals()?;
ecx.push_stack_frame_done()?;

// The main interpreter loop.
while ecx.step()? {
Expand Down
286 changes: 139 additions & 147 deletions compiler/rustc_const_eval/src/interpret/call.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,166 +404,158 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {

// Push the "raw" frame -- this leaves locals uninitialized.
self.push_stack_frame_raw(instance, body, destination, cont)?;
let preamble_span = self.frame().loc.unwrap_right(); // the span used for preamble errors

trace!(
"caller ABI: {:#?}, args: {:#?}",
caller_fn_abi,
args.iter()
.map(|arg| (
arg.layout().ty,
match arg {
FnArg::Copy(op) => format!("copy({op:?})"),
FnArg::InPlace(mplace) => format!("in-place({mplace:?})"),
}
))
.collect::<Vec<_>>()
);
trace!(
"spread_arg: {:?}, locals: {:#?}",
body.spread_arg,
body.args_iter()
.map(|local| (local, self.layout_of_local(self.frame(), local, None).unwrap().ty,))
.collect::<Vec<_>>()
);

// If an error is raised here, pop the frame again to get an accurate backtrace.
// To this end, we wrap it all in a `try` block.
let res: InterpResult<'tcx> = try {
trace!(
"caller ABI: {:#?}, args: {:#?}",
caller_fn_abi,
args.iter()
.map(|arg| (
arg.layout().ty,
match arg {
FnArg::Copy(op) => format!("copy({op:?})"),
FnArg::InPlace(mplace) => format!("in-place({mplace:?})"),
}
))
.collect::<Vec<_>>()
);
trace!(
"spread_arg: {:?}, locals: {:#?}",
body.spread_arg,
body.args_iter()
.map(|local| (
local,
self.layout_of_local(self.frame(), local, None).unwrap().ty,
))
.collect::<Vec<_>>()
);

// In principle, we have two iterators: Where the arguments come from, and where
// they go to.

// The "where they come from" part is easy, we expect the caller to do any special handling
// that might be required here (e.g. for untupling).
// If `with_caller_location` is set we pretend there is an extra argument (that
// we will not pass; our `caller_location` intrinsic implementation walks the stack instead).
assert_eq!(
args.len() + if with_caller_location { 1 } else { 0 },
caller_fn_abi.args.len(),
"mismatch between caller ABI and caller arguments",
);
let mut caller_args = args
.iter()
.zip(caller_fn_abi.args.iter())
.filter(|arg_and_abi| !arg_and_abi.1.is_ignore());

// Now we have to spread them out across the callee's locals,
// taking into account the `spread_arg`. If we could write
// this is a single iterator (that handles `spread_arg`), then
// `pass_argument` would be the loop body. It takes care to
// not advance `caller_iter` for ignored arguments.
let mut callee_args_abis = callee_fn_abi.args.iter().enumerate();
// Determine whether there is a special VaList argument. This is always the
// last argument, and since arguments start at index 1 that's `arg_count`.
let va_list_arg =
callee_fn_abi.c_variadic.then(|| mir::Local::from_usize(body.arg_count));
for local in body.args_iter() {
// Construct the destination place for this argument. At this point all
// locals are still dead, so we cannot construct a `PlaceTy`.
let dest = mir::Place::from(local);
// `layout_of_local` does more than just the instantiation we need to get the
// type, but the result gets cached so this avoids calling the instantiation
// query *again* the next time this local is accessed.
let ty = self.layout_of_local(self.frame(), local, None)?.ty;
if Some(local) == va_list_arg {
// This is the last callee-side argument of a variadic function.
// This argument is a VaList holding the remaining caller-side arguments.
self.storage_live(local)?;

let place = self.eval_place(dest)?;
let mplace = self.force_allocation(&place)?;

// Consume the remaining arguments by putting them into the variable argument
// list.
let varargs = self.allocate_varargs(
&mut caller_args,
// "Ignored" arguments aren't actually passed, so the callee should also
// ignore them. (`pass_argument` does this for regular arguments.)
(&mut callee_args_abis).filter(|(_, abi)| !abi.is_ignore()),
)?;
// When the frame is dropped, these variable arguments are deallocated.
self.frame_mut().va_list = varargs.clone();
let key = self.va_list_ptr(varargs.into());

// Zero the VaList, so it is fully initialized.
self.write_bytes_ptr(
mplace.ptr(),
(0..mplace.layout.size.bytes()).map(|_| 0u8),
)?;
// In principle, we have two iterators: Where the arguments come from, and where
// they go to.

// Store the "key" pointer in the right field.
let key_mplace = self.va_list_key_field(&mplace)?;
self.write_pointer(key, &key_mplace)?;
} else if Some(local) == body.spread_arg {
// Make the local live once, then fill in the value field by field.
self.storage_live(local)?;
// Must be a tuple
let ty::Tuple(fields) = ty.kind() else {
span_bug!(self.cur_span(), "non-tuple type for `spread_arg`: {ty}")
};
for (i, field_ty) in fields.iter().enumerate() {
let dest = dest.project_deeper(
&[mir::ProjectionElem::Field(FieldIdx::from_usize(i), field_ty)],
*self.tcx,
);
let (idx, callee_abi) = callee_args_abis.next().unwrap();
self.pass_argument(
&mut caller_args,
callee_abi,
idx,
&dest,
field_ty,
/* already_live */ true,
)?;
}
} else {
// Normal argument. Cannot mark it as live yet, it might be unsized!
// The "where they come from" part is easy, we expect the caller to do any special handling
// that might be required here (e.g. for untupling).
// If `with_caller_location` is set we pretend there is an extra argument (that
// we will not pass; our `caller_location` intrinsic implementation walks the stack instead).
assert_eq!(
args.len() + if with_caller_location { 1 } else { 0 },
caller_fn_abi.args.len(),
"mismatch between caller ABI and caller arguments",
);
let mut caller_args = args
.iter()
.zip(caller_fn_abi.args.iter())
.filter(|arg_and_abi| !arg_and_abi.1.is_ignore());

// Now we have to spread them out across the callee's locals,
// taking into account the `spread_arg`. If we could write
// this is a single iterator (that handles `spread_arg`), then
// `pass_argument` would be the loop body. It takes care to
// not advance `caller_iter` for ignored arguments.
let mut callee_args_abis = callee_fn_abi.args.iter().enumerate();
// Determine whether there is a special VaList argument. This is always the
// last argument, and since arguments start at index 1 that's `arg_count`.
let va_list_arg = callee_fn_abi.c_variadic.then(|| mir::Local::from_usize(body.arg_count));
for local in body.args_iter() {
// Update the span that we show in case of an error to point to this argument.
self.frame_mut().loc = Right(body.local_decls[local].source_info.span);
// Construct the destination place for this argument. At this point all
// locals are still dead, so we cannot construct a `PlaceTy`.
let dest = mir::Place::from(local);
// `layout_of_local` does more than just the instantiation we need to get the
// type, but the result gets cached so this avoids calling the instantiation
// query *again* the next time this local is accessed.
let ty = self.layout_of_local(self.frame(), local, None)?.ty;
if Some(local) == va_list_arg {
// This is the last callee-side argument of a variadic function.
// This argument is a VaList holding the remaining caller-side arguments.
self.storage_live(local)?;

let place = self.eval_place(dest)?;
let mplace = self.force_allocation(&place)?;

// Consume the remaining arguments by putting them into the variable argument
// list.
let varargs = self.allocate_varargs(
&mut caller_args,
// "Ignored" arguments aren't actually passed, so the callee should also
// ignore them. (`pass_argument` does this for regular arguments.)
(&mut callee_args_abis).filter(|(_, abi)| !abi.is_ignore()),
)?;
// When the frame is dropped, these variable arguments are deallocated.
self.frame_mut().va_list = varargs.clone();
let key = self.va_list_ptr(varargs.into());

// Zero the VaList, so it is fully initialized.
self.write_bytes_ptr(mplace.ptr(), (0..mplace.layout.size.bytes()).map(|_| 0u8))?;

// Store the "key" pointer in the right field.
let key_mplace = self.va_list_key_field(&mplace)?;
self.write_pointer(key, &key_mplace)?;
} else if Some(local) == body.spread_arg {
// Make the local live once, then fill in the value field by field.
self.storage_live(local)?;
// Must be a tuple
let ty::Tuple(fields) = ty.kind() else {
span_bug!(self.cur_span(), "non-tuple type for `spread_arg`: {ty}")
};
for (i, field_ty) in fields.iter().enumerate() {
let dest = dest.project_deeper(
&[mir::ProjectionElem::Field(FieldIdx::from_usize(i), field_ty)],
*self.tcx,
);
let (idx, callee_abi) = callee_args_abis.next().unwrap();
self.pass_argument(
&mut caller_args,
callee_abi,
idx,
&dest,
ty,
/* already_live */ false,
field_ty,
/* already_live */ true,
)?;
}
} else {
// Normal argument. Cannot mark it as live yet, it might be unsized!
let (idx, callee_abi) = callee_args_abis.next().unwrap();
self.pass_argument(
&mut caller_args,
callee_abi,
idx,
&dest,
ty,
/* already_live */ false,
)?;
}
// If the callee needs a caller location, pretend we consume one more argument from the ABI.
if instance.def.requires_caller_location(*self.tcx) {
callee_args_abis.next().unwrap();
}
// Now we should have no more caller args or callee arg ABIs.
assert!(
callee_args_abis.next().is_none(),
"mismatch between callee ABI and callee body arguments"
);
if caller_args.next().is_some() {
throw_ub_format!("calling a function with more arguments than it expected");
}
// Don't forget to check the return type!
if !self.check_argument_compat(&caller_fn_abi.ret, &callee_fn_abi.ret)? {
throw_ub!(AbiMismatchReturn {
caller_ty: caller_fn_abi.ret.layout.ty,
callee_ty: callee_fn_abi.ret.layout.ty
});
}
}

// Protect return place for in-place return value passing.
// We only need to protect anything if this is actually an in-memory place.
if let Some(mplace) = destination_mplace {
M::protect_in_place_function_argument(self, &mplace)?;
}
// Don't forget to check the return type!
self.frame_mut().loc = Right(body.local_decls[mir::RETURN_PLACE].source_info.span);
if !self.check_argument_compat(&caller_fn_abi.ret, &callee_fn_abi.ret)? {
throw_ub!(AbiMismatchReturn {
caller_ty: caller_fn_abi.ret.layout.ty,
callee_ty: callee_fn_abi.ret.layout.ty
});
}
// Protect return place for in-place return value passing.
// We only need to protect anything if this is actually an in-memory place.
if let Some(mplace) = destination_mplace {
M::protect_in_place_function_argument(self, &mplace)?;
}

// Don't forget to mark "initially live" locals as live.
self.storage_live_for_always_live_locals()?;
};
res.inspect_err_kind(|_| {
// Don't show the incomplete stack frame in the error stacktrace.
self.stack_mut().pop();
})
// For the final checks, use same span as preamble since it is unclear what else to do.
self.frame_mut().loc = Right(preamble_span);
// If the callee needs a caller location, pretend we consume one more argument from the ABI.
if instance.def.requires_caller_location(*self.tcx) {
callee_args_abis.next().unwrap();
}
// Now we should have no more caller args or callee arg ABIs.
assert!(
callee_args_abis.next().is_none(),
"mismatch between callee ABI and callee body arguments"
);
if caller_args.next().is_some() {
throw_ub_format!("calling a function with more arguments than it expected");
}

// Done!
self.push_stack_frame_done()
}

/// Initiate a call to this function -- pushing the stack frame and initializing the arguments.
Expand Down
15 changes: 10 additions & 5 deletions compiler/rustc_const_eval/src/interpret/stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let locals = IndexVec::from_elem(dead_local, &body.local_decls);
let pre_frame = Frame {
body,
loc: Right(body.span), // Span used for errors caused during preamble.
loc: Right(self.tcx.def_span(body.source.def_id())), // Span used for errors caused during preamble.
return_cont,
return_place: return_place.clone(),
locals,
Expand All @@ -408,7 +408,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {

// Finish things up.
M::after_stack_push(self)?;
self.frame_mut().loc = Left(mir::Location::START);
// `tracing_separate_thread` is used to instruct the tracing_chrome [tracing::Layer] in Miri
// to put the "frame" span on a separate trace thread/line than other spans, to make the
// visualization in <https://ui.perfetto.dev> easier to interpret. It is set to a value of
Expand Down Expand Up @@ -466,9 +465,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}
}

/// In the current stack frame, mark all locals as live that are not arguments and don't have
/// `Storage*` annotations (this includes the return place).
pub(crate) fn storage_live_for_always_live_locals(&mut self) -> InterpResult<'tcx> {
/// Call this after `push_stack_frame_raw` and when all the other setup that needs to be done
/// is completed.
pub(crate) fn push_stack_frame_done(&mut self) -> InterpResult<'tcx> {
// Mark all locals as live that are not arguments and don't have `Storage*` annotations
// (this includes the return place, but not the arguments).
self.storage_live(mir::RETURN_PLACE)?;

let body = self.body();
Expand All @@ -478,6 +479,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
self.storage_live(local)?;
}
}

// Get ready to execute the first instruction in the stack frame.
self.frame_mut().loc = Left(mir::Location::START);

interp_ok(())
}

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_const_eval/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#![feature(never_type)]
#![feature(slice_ptr_get)]
#![feature(trait_alias)]
#![feature(try_blocks)]
#![feature(unqualified_local_imports)]
#![feature(yeet_expr)]
#![warn(unqualified_local_imports)]
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail-dep/concurrency/libc_pthread_create_too_few_args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use std::{mem, ptr};

extern "C" fn thread_start() -> *mut libc::c_void {
//~^ERROR: calling a function with more arguments than it expected
panic!()
}

Expand All @@ -16,7 +17,6 @@ fn main() {
mem::transmute(thread_start);
assert_eq!(
libc::pthread_create(&mut native, ptr::null(), thread_start, ptr::null_mut()),
//~^ERROR: calling a function with more arguments than it expected
0
);
assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
Expand Down
Loading
Loading