Skip to content

[WIP] Interpret tokio #603

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

Closed
wants to merge 6 commits into from
Closed
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
65 changes: 60 additions & 5 deletions src/fn_call.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustc::ty;
use rustc::ty::layout::{Align, LayoutOf, Size};
use rustc::hir::def_id::DefId;
use rustc::hir::def_id::{DefId, DefIndex, CrateNum};
use rustc::mir;
use syntax::attr;

Expand All @@ -15,7 +15,23 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
dest: Option<PlaceTy<'tcx, Borrow>>,
ret: Option<mir::BasicBlock>,
) -> EvalResult<'tcx, Option<&'mir mir::Mir<'tcx>>> {

// HACK: normal evaluation never sees `ReservedForIncrCompCache`, so we use it
// to signify a dlsym loaded thing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG WTF.^^

Okay so at the least could we wrap this awful hack into two functions dlsym_to_instance and instance_to_dlsym (or so)? Currently the other half of this hack is 200 lines away in the same file without any comment.

The better way, of course, would be to change create_fn_alloc such that it takes something like

enum MiriFunction {
  Instance(Instance),
  DlSym(String),
}

Then we'd need no hack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know... but the compiler and const eval have zero profit from supporting this but would still have to live with the complexity.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Complexity seems fairly minimal? Unless of course there is a perf issue, but I don't expect one -- this is just for fn ptrs, after all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a few months perspective on this: Indeed "OMG WTF what was I thinking?" I'll teach the engine about dlsyms

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am implementing this now. (Latest getrandom needs it on macOS.)

if let ty::InstanceDef::Item(did) = instance.def {
if let CrateNum::ReservedForIncrCompCache = did.krate {
let id = did.index.as_raw_u32();
self.emulate_dlsym_item(
id,
args,
dest.unwrap(),
ret.unwrap(),
)?;
return Ok(None);
}
}
let this = self.eval_context_mut();

trace!("eval_fn_call: {:#?}, {:?}", instance, dest.map(|place| *place));

// first run the common hooks also supported by CTFE
Expand Down Expand Up @@ -199,10 +215,23 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
let symbol_name = this.memory().get(symbol.alloc_id)?.read_c_str(tcx, symbol)?;
let err = format!("bad c unicode symbol: {:?}", symbol_name);
let symbol_name = ::std::str::from_utf8(symbol_name).unwrap_or(&err);
return err!(Unimplemented(format!(
"miri does not support dynamically loading libraries (requested symbol: {})",
symbol_name
)));
let id = match symbol_name {
"epoll_create1" => 0,
_ => return err!(Unimplemented(format!(
"miri does not support dynamically loading symbol `{}`",
symbol_name,
))),
};
let did = DefId {
krate: CrateNum::ReservedForIncrCompCache,
index: DefIndex::from_raw_u32(id),
};
let instance = ty::Instance {
def: ty::InstanceDef::Item(did),
substs: this.tcx.mk_substs([].iter()),
};
let ptr = this.memory_mut().create_fn_alloc(instance);
this.write_scalar(Scalar::from(ptr.with_default_tag()), dest)?;
}

"__rust_maybe_catch_panic" => {
Expand Down Expand Up @@ -677,6 +706,13 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
this.write_scalar(Scalar::Ptr(this.machine.cmd_line.unwrap()), dest)?;
}

"sched_getaffinity" => {
let _pid = this.read_scalar(args[0])?.to_i32()?;
let _cpusetsize = this.read_scalar(args[1])?.to_usize(this)?;
let _mask = this.read_scalar(args[2])?.to_ptr()?;
this.write_null(dest)?;
}

// We can't execute anything else
_ => {
return err!(Unimplemented(
Expand All @@ -693,4 +729,23 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
fn write_null(&mut self, dest: PlaceTy<'tcx, Borrow>) -> EvalResult<'tcx> {
self.eval_context_mut().write_scalar(Scalar::from_int(0, dest.layout.size), dest)
}

// FIXME: unify with emulate_foreign_item if we ever end up having overlap
fn emulate_dlsym_item(
&mut self,
id: u32,
args: &[OpTy<'tcx, Borrow>],
dest: PlaceTy<'tcx, Borrow>,
ret: mir::BasicBlock,
) -> EvalResult<'tcx> {
let this = self.eval_context_mut();
match id {
// epoll_create1
0 => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No magic constants please. Seems like a C-like enum would be appropriate here, or so?

this.write_null(dest)?;
},
_ => unreachable!(),
}
Ok(())
}
}
12 changes: 7 additions & 5 deletions src/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,12 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
assert_eq!(size as u64, self.pointer_size().bytes());
let bits = bits as u64;

// Case I: Comparing with NULL
if bits == 0 {
// Test if the ptr is in-bounds. Then it cannot be NULL.
// Even dangling pointers cannot be NULL.
// Case I: Comparing with small integers
if bits <= 1 {
// Test if the ptr is in-bounds. Then it cannot be a small integer.
// Even dangling pointers cannot be small integers.
// The only exception is wasm and some embedded devices, but that's a different
// story: https://github.com/rust-lang/rust/issues/57897
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this. Enabling such platform-specific assumptions should be somehow, at the very least, tied to the platform -- as in, depending on the target we should do this or not. I think we should also have a flag ("strict mode" or so) to not make such assumptions.

For platforms where we make this assumption, probably it applies at least to bits < 4096, doesn't it?

if self.memory().check_bounds_ptr(ptr, InboundsCheck::MaybeDead).is_ok() {
return Ok(false);
}
Expand All @@ -185,7 +187,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
// without wrapping around the address space.
{
// Compute the highest address at which this allocation could live.
// Substract one more, because it must be possible to add the size
// Subtract one more, because it must be possible to add the size
// to the base address without overflowing -- IOW, the very last address
// of the address space is never dereferencable (but it can be in-bounds, i.e.,
// one-past-the-end).
Expand Down
Empty file added tests/run-pass/tokio.rs
Empty file.
9 changes: 9 additions & 0 deletions tokio_test/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "tokio_test"
version = "0.1.0"
authors = ["Oliver Scherer <[email protected]>"]
edition = "2018"

[dependencies]
tokio = "0.1.14"
futures = "0.1.25"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have these in a subdir or so? We are getting too many root dirs for my taste.

3 changes: 3 additions & 0 deletions tokio_test/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
tokio::run(futures::future::ok::<_, ()>(()))
}