-
Notifications
You must be signed in to change notification settings - Fork 385
[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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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; | ||
|
||
|
@@ -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 | ||
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 | ||
|
@@ -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" => { | ||
|
@@ -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( | ||
|
@@ -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 => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(()) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
if self.memory().check_bounds_ptr(ptr, InboundsCheck::MaybeDead).is_ok() { | ||
return Ok(false); | ||
} | ||
|
@@ -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). | ||
|
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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
fn main() { | ||
tokio::run(futures::future::ok::<_, ()>(())) | ||
} |
There was a problem hiding this comment.
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
andinstance_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 likeThen we'd need no hack.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.)