Re: [RFC PATCH v3 01/11] plugins: add types for callbacks related to certain discontinuities

[Julian Ganz]

Hi Pierrick,

December 6, 2024 at 7:59 PM, "Pierrick Bouvier" wrote:
> On 12/6/24 00:58, Julian Ganz wrote:
> >  December 6, 2024 at 12:03 AM, "Pierrick Bouvier" wrote:
> > > It's an interesting conversation. For the scope of this series, I agree 
> > > you should use the jump target, which triggered the trap.
> > > 
> > >  In fine, transitions should simply follow what the cpu does.
> > > 
> > >  - orig_insn: jump to A
> > >  - jump_target: execute A traps
> > >  - page_fault: load page
> > >  - jump_target: come back to A
> > > 
> > >  event(JUMP, orig_insn, jump_target) // not covered by this series
> > >  event(EXCEPTION, jump_target, page_fault)
> > >  ... execute page_fault (with potential other transitions)
> > >  event(JUMP, end_page_fault, jump_target)
> > > 
> > >  In the case of a double trap, we could follow the same logic, and 
> > > represent the original transition that lead to the trap, and the two 
> > > consecutive traps.
> > > 
> > >  Does it make sense?
> > > 
> >  Yes, those transitions are correct imo. And if a jump event should be
> >  introduced at some point, the call sequence would look like that. My
> >  issue is that testing this (in a plugin) will not be straight forward
> >  or even impossible. And overly complex tests don't exactly provoke
> >  confidence.
> > 
> Instruction instrumentation is done before executing the instruction itself, 
> as you can see by running:
> ./build/qemu-x86_64 -plugin build/tests/tcg/plugins/libinsn.so -d in_asm,op 
> /usr/bin/true
> 
> I'm not entirely sure about the sequence when there is an exception while 
> fetching the instruction though. You can give it a try, track the PC using 
> insn instrumentation, and we can identify which cases are not working.
> 
> The test plugin itself is not complicated.
> You'll need:
> - one callback per instruction to set the expected pc (possibly optimized 
> with inline operation), used to compare to from_pc, and we check if 
> (optional) to_pc matches the current instruction.

What I'm saying ist that this exactly is not feasible for quite a few
relevant instructions. As someone who isn't all too intimate with TCG
itself, it's not even clear if we can rely on jump and branch
instructions only occuring at the end of a tb. At least a superficial
glance at the documentation tells me we can, but if this should in fact
not be the case, or if someone introduces something like zero overhead
loops inside a tb, all bets are off.

> - when the callback for discontinuity is called, we check if from_pc is 
> matching, and register the next expected with to_pc.
> 
> We can then add tests targeting supported architectures using the plugin, and 
> ensuring it never fails.
> It's hard to know we don't miss events though. Except if we write manual 
> assembly system mode tests, that trigger the expected behaviour. But it would 
> be tedious, and I'm really not sure there is a real value with reduced 
> examples like this.

So currently my thinking goes in the direction of having the plugin
print a warning every time we don't have an expected from_pc to check
against. Probably also have this be a feature you can deactivate.

Regards,
Julian Ganz