Open bjorn3 opened 3 years ago
Could we add a way for priroda to skip the current instruction instead of stepping on it? Then we could just interpret until the breakpoint instruction is hit (which errors), manually skip it, and then continue interpreting.
I believe @RalfJung objects to changing the current location outside of regular steps. Or did I misinterpret https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/Frame.3A.3Aloc.20no.20longer.20public/near/228074161
Couldn't you just have another implementation of the intrinsic that doesn't do anything / signals something to priroda? That seems cleaner than messing with the program counter.
Yeah, I think that seems correct. Currently I don't priroda
has any infrastructure to support that, but it sounds like it wouldn't be too bad?
@RalfJung I don't think there's any feasible way for priroda to overwrite miri's intrinsics at the moment.
The problem is that we cannot create a InterpCx<Evaluator>
from a InterpCx<PrirodaMachine>
, which prevents us from using most of the methods of the Machine
implementation for Evaluator
(because they have arguments of ecx: &InterpCx<Evaluator>
which we cannot construct).
Additionally, simply copy-pasting the definitions from miri
also doesn't work, because e.g. many of the methods call the methods on the extension traits implemented for InterpCx<Evaluator>
. Additionally, the intptrcast
module is currently also private.
@DJMcNab hm... yeah that seems tricky indeed.
What is the least-overhead way for Miri to provide "hooks" that downstream creates could fill in with their own code?
I think the least intrusive way would be adding an Option<Box<dyn Fn(...)>>
that priroda can fill in with a handler
Unfortunately for other changes we'll likely want a custom machine.
For example, a final form of https://github.com/oli-obk/priroda/issues/14, would likely require that we can cheaply clone InterpCx
s, for some definition of cheap. This would likely require a custom allocation map data structure (maybe https://crates.io/crates/evmap?)
It's possible that #14 is infeasible anyway because it would break aliasing guarantees. Although if we bail as unprintable on UB (or even some subset of UB), we should be fine.
That being said, a hook literally for the breakpoint intrinsic would probably be 'good enough for now'
I'd prefer something slightly less ad-hoc than a hook for a specific intrinsic.
Unfortunately for other changes we'll likely want a custom machine.
The only way I see to allow that would be to make much of Miri generic in the machine, which sounds like it'll add a lot of boilerplate on the Miri side. :/
Yeah, at first glance there aren't many good solutions. A possible alternative would be change upstream InterpCx
to delegate all of its fields to the machine, so then we could store the miri machine as a field. Neither of those are especially fun though.
That being said, for the moment we don't need to handle the breakpoint intrinsic, since in practise it isn't likely to be in heavy use.
https://github.com/rust-lang/miri/pull/1733#issuecomment-790528244