[WIP] Interpret tokio #603
Conversation
| @@ -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 | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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.
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.
I am implementing this now. (Latest getrandom needs it on macOS.)
| let this = self.eval_context_mut(); | ||
| match id { | ||
| // epoll_create1 | ||
| 0 => { |
There was a problem hiding this comment.
No magic constants please. Seems like a C-like enum would be appropriate here, or so?
| // 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.
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?
|
|
||
| [dependencies] | ||
| tokio = "0.1.14" | ||
| futures = "0.1.25" |
There was a problem hiding this comment.
Can we have these in a subdir or so? We are getting too many root dirs for my taste.
RalfJung
added
the
S-waiting-on-author
|
@oli-obk Is there any point in keeping this open? I'd rather keep the PR list clean, having only things in there that are actually being worked on, or that are blocked on things that will likely soon be resolved. |
fixes #602