Skip to content

Adjust Miri to also require return places everywhere #2138

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

Merged
merged 2 commits into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
32c8c5df06c025441ad04791d7982d65c79a60e4
b2eba058e6e1c698723e47074561a30b50b5fa7a
2 changes: 1 addition & 1 deletion src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ pub fn report_error<'tcx, 'mir>(
for (i, frame) in ecx.active_thread_stack().iter().enumerate() {
trace!("-------------------");
trace!("Frame {}", i);
trace!(" return: {:?}", frame.return_place.map(|p| *p));
trace!(" return: {:?}", *frame.return_place);
for (i, local) in frame.locals.iter().enumerate() {
trace!(" local {}: {:?}", i, local.value);
}
Expand Down
4 changes: 2 additions & 2 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
start_instance,
Abi::Rust,
&[Scalar::from_pointer(main_ptr, &ecx).into(), argc.into(), argv],
Some(&ret_place.into()),
&ret_place.into(),
StackPopCleanup::Root { cleanup: true },
)?;
}
Expand All @@ -286,7 +286,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
entry_instance,
Abi::Rust,
&[argc.into(), argv],
Some(&ret_place.into()),
&ret_place.into(),
StackPopCleanup::Root { cleanup: true },
)?;
}
Expand Down
2 changes: 1 addition & 1 deletion src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
f: ty::Instance<'tcx>,
caller_abi: Abi,
args: &[Immediate<Tag>],
dest: Option<&PlaceTy<'tcx, Tag>>,
dest: &PlaceTy<'tcx, Tag>,
stack_pop: StackPopCleanup,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
Expand Down
15 changes: 9 additions & 6 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,10 +512,11 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
instance: ty::Instance<'tcx>,
abi: Abi,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
ecx.find_mir_or_eval_fn(instance, abi, args, ret, unwind)
ecx.find_mir_or_eval_fn(instance, abi, args, dest, ret, unwind)
}

#[inline(always)]
Expand All @@ -524,21 +525,23 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
fn_val: Dlsym,
abi: Abi,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
_unwind: StackPopUnwind,
) -> InterpResult<'tcx> {
ecx.call_dlsym(fn_val, abi, args, ret)
ecx.call_dlsym(fn_val, abi, args, dest, ret)
}

#[inline(always)]
fn call_intrinsic(
ecx: &mut MiriEvalContext<'mir, 'tcx>,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx> {
ecx.call_intrinsic(instance, args, ret, unwind)
ecx.call_intrinsic(instance, args, dest, ret, unwind)
}

#[inline(always)]
Expand Down
8 changes: 5 additions & 3 deletions src/shims/dlsym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
dlsym: Dlsym,
abi: Abi,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
match dlsym {
Dlsym::Posix(dlsym) => posix::EvalContextExt::call_dlsym(this, dlsym, abi, args, ret),
Dlsym::Posix(dlsym) =>
posix::EvalContextExt::call_dlsym(this, dlsym, abi, args, dest, ret),
Dlsym::Windows(dlsym) =>
windows::EvalContextExt::call_dlsym(this, dlsym, abi, args, ret),
windows::EvalContextExt::call_dlsym(this, dlsym, abi, args, dest, ret),
}
}
}
5 changes: 3 additions & 2 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
def_id: DefId,
abi: Abi,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
let this = self.eval_context_mut();
let link_name = this.item_link_name(def_id);
let tcx = this.tcx.tcx;

// First: functions that diverge.
let (dest, ret) = match ret {
let ret = match ret {
None =>
match &*link_name.as_str() {
"miri_start_panic" => {
Expand Down
7 changes: 4 additions & 3 deletions src/shims/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
&mut self,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
_unwind: StackPopUnwind,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();

if this.emulate_intrinsic(instance, args, ret)? {
if this.emulate_intrinsic(instance, args, dest, ret)? {
return Ok(());
}

// All supported intrinsics have a return place.
let intrinsic_name = this.tcx.item_name(instance.def_id());
let intrinsic_name = intrinsic_name.as_str();
let (dest, ret) = match ret {
let ret = match ret {
None => throw_unsup_format!("unimplemented (diverging) intrinsic: {}", intrinsic_name),
Some(p) => p,
};
Expand Down
14 changes: 8 additions & 6 deletions src/shims/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
instance: ty::Instance<'tcx>,
abi: Abi,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
let this = self.eval_context_mut();
trace!("eval_fn_call: {:#?}, {:?}", instance, ret.map(|p| p.0));
trace!("eval_fn_call: {:#?}, {:?}", instance, dest);

// There are some more lang items we want to hook that CTFE does not hook (yet).
if this.tcx.lang_items().align_offset_fn() == Some(instance.def.def_id()) {
let [ptr, align] = check_arg_count(args)?;
if this.align_offset(ptr, align, ret, unwind)? {
if this.align_offset(ptr, align, dest, ret, unwind)? {
return Ok(None);
}
}
Expand All @@ -50,7 +51,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// to run extra MIR), and Ok(Some(body)) if we found MIR to run for the
// foreign function
// Any needed call to `goto_block` will be performed by `emulate_foreign_item`.
return this.emulate_foreign_item(instance.def_id(), abi, args, ret, unwind);
return this.emulate_foreign_item(instance.def_id(), abi, args, dest, ret, unwind);
}

// Otherwise, load the MIR.
Expand All @@ -63,11 +64,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
&mut self,
ptr_op: &OpTy<'tcx, Tag>,
align_op: &OpTy<'tcx, Tag>,
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx, bool> {
let this = self.eval_context_mut();
let (dest, ret) = ret.unwrap();
let ret = ret.unwrap();

if this.machine.check_alignment != AlignmentCheck::Symbolic {
// Just use actual implementation.
Expand Down
8 changes: 4 additions & 4 deletions src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
f_instance,
Abi::Rust,
&[data.into()],
Some(&ret_place),
&ret_place,
// Directly return to caller.
StackPopCleanup::Goto { ret: Some(ret), unwind: StackPopUnwind::Skip },
)?;
Expand Down Expand Up @@ -153,7 +153,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
f_instance,
Abi::Rust,
&[catch_unwind.data.into(), payload.into()],
Some(&ret_place),
&ret_place,
// Directly return to caller of `try`.
StackPopCleanup::Goto { ret: Some(catch_unwind.ret), unwind: StackPopUnwind::Skip },
)?;
Expand All @@ -179,7 +179,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
panic,
Abi::Rust,
&[msg.to_ref(this)],
None,
&MPlaceTy::dangling(this.machine.layouts.unit).into(),
StackPopCleanup::Goto { ret: None, unwind },
)
}
Expand Down Expand Up @@ -208,7 +208,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
panic_bounds_check,
Abi::Rust,
&[index.into(), len.into()],
None,
&MPlaceTy::dangling(this.machine.layouts.unit).into(),
StackPopCleanup::Goto {
ret: None,
unwind: match unwind {
Expand Down
7 changes: 4 additions & 3 deletions src/shims/posix/dlsym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
dlsym: Dlsym,
abi: Abi,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();

this.check_abi(abi, Abi::C { unwind: false })?;

match dlsym {
Dlsym::Linux(dlsym) => linux::EvalContextExt::call_dlsym(this, dlsym, args, ret),
Dlsym::MacOs(dlsym) => macos::EvalContextExt::call_dlsym(this, dlsym, args, ret),
Dlsym::Linux(dlsym) => linux::EvalContextExt::call_dlsym(this, dlsym, args, dest, ret),
Dlsym::MacOs(dlsym) => macos::EvalContextExt::call_dlsym(this, dlsym, args, dest, ret),
}
}
}
5 changes: 3 additions & 2 deletions src/shims/posix/linux/dlsym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
&mut self,
dlsym: Dlsym,
_args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
_dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let (_dest, _ret) = ret.expect("we don't support any diverging dlsym");
let _ret = ret.expect("we don't support any diverging dlsym");
assert!(this.tcx.sess.target.os == "linux");

match dlsym {}
Expand Down
5 changes: 3 additions & 2 deletions src/shims/posix/macos/dlsym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
&mut self,
dlsym: Dlsym,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let (dest, ret) = ret.expect("we don't support any diverging dlsym");
let ret = ret.expect("we don't support any diverging dlsym");
assert!(this.tcx.sess.target.os == "macos");

match dlsym {
Expand Down
2 changes: 1 addition & 1 deletion src/shims/posix/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
instance,
Abi::C { unwind: false },
&[*func_arg],
Some(&ret_place.into()),
&ret_place.into(),
StackPopCleanup::Root { cleanup: true },
)?;

Expand Down
6 changes: 3 additions & 3 deletions src/shims/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
thread_callback,
Abi::System { unwind: false },
&[Scalar::null_ptr(this).into(), reason.into(), Scalar::null_ptr(this).into()],
Some(&ret_place),
&ret_place,
StackPopCleanup::Root { cleanup: true },
)?;

Expand All @@ -281,7 +281,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
instance,
Abi::C { unwind: false },
&[data.into()],
Some(&ret_place),
&ret_place,
StackPopCleanup::Root { cleanup: true },
)?;

Expand Down Expand Up @@ -324,7 +324,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
instance,
Abi::C { unwind: false },
&[ptr.into()],
Some(&ret_place),
&ret_place,
StackPopCleanup::Root { cleanup: true },
)?;

Expand Down
5 changes: 3 additions & 2 deletions src/shims/windows/dlsym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
dlsym: Dlsym,
abi: Abi,
args: &[OpTy<'tcx, Tag>],
ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
dest: &PlaceTy<'tcx, Tag>,
ret: Option<mir::BasicBlock>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let (dest, ret) = ret.expect("we don't support any diverging dlsym");
let ret = ret.expect("we don't support any diverging dlsym");
assert!(this.tcx.sess.target.os == "windows");

this.check_abi(abi, Abi::System { unwind: false })?;
Expand Down
9 changes: 2 additions & 7 deletions src/stacked_borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -930,12 +930,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
/// explicit. Also see https://github.com/rust-lang/rust/issues/71117.
fn retag_return_place(&mut self) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let return_place = if let Some(return_place) = this.frame_mut().return_place {
return_place
} else {
// No return place, nothing to do.
return Ok(());
};
let return_place = this.frame_mut().return_place;
if return_place.layout.is_zst() {
// There may not be any memory here, nothing to do.
return Ok(());
Expand All @@ -955,7 +950,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
)?;
// And use reborrowed pointer for return place.
let return_place = this.ref_to_mplace(&val)?;
this.frame_mut().return_place = Some(return_place.into());
this.frame_mut().return_place = return_place.into();

Ok(())
}
Expand Down