Skip to content
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

Handle the breakpoint intrinsic #34

Open
bjorn3 opened this issue Mar 4, 2021 · 11 comments
Open

Handle the breakpoint intrinsic #34

bjorn3 opened this issue Mar 4, 2021 · 11 comments

Comments

@bjorn3
Copy link
Collaborator

bjorn3 commented Mar 4, 2021

rust-lang/miri#1733 (comment)

@oli-obk
Copy link
Owner

oli-obk commented Mar 4, 2021

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.

@bjorn3
Copy link
Collaborator Author

bjorn3 commented Mar 4, 2021

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

@RalfJung
Copy link

RalfJung commented Mar 4, 2021

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.

@DJMcNab
Copy link
Contributor

DJMcNab commented Mar 4, 2021

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?

@DJMcNab
Copy link
Contributor

DJMcNab commented Mar 13, 2021

@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.

@RalfJung
Copy link

@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?

@oli-obk
Copy link
Owner

oli-obk commented Mar 15, 2021

I think the least intrusive way would be adding an Option<Box<dyn Fn(...)>> that priroda can fill in with a handler

@DJMcNab
Copy link
Contributor

DJMcNab commented Mar 15, 2021

Unfortunately for other changes we'll likely want a custom machine.

For example, a final form of #14, would likely require that we can cheaply clone InterpCxs, 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.

@DJMcNab
Copy link
Contributor

DJMcNab commented Mar 15, 2021

That being said, a hook literally for the breakpoint intrinsic would probably be 'good enough for now'

@RalfJung
Copy link

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. :/

@DJMcNab
Copy link
Contributor

DJMcNab commented Mar 15, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants