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

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 12/5/24 13:50, Julian Ganz wrote:
> Hi Pierrick,
> 
> December 5, 2024 at 6:56 PM, "Pierrick Bouvier" wrote:
>> On 12/5/24 04:40, Julian Ganz wrote:
>>
>>>
>>> Hi Pierrick,
>>>   December 4, 2024 at 11:41 PM, "Pierrick Bouvier" wrote:
>>>> Does it mean that information returned should be dependent of type of event, as we previously discussed on v1?
>>>>
>>>   Yes, and I don't like it.
>>>
>> I respect your personal preference, but our conversation should be based on arguments, and not only tastes.
>>
>> The important thing, from my point of view, is that the API stays easy to use and clear for the user. Having multiple callbacks is a headache, because you can't clearly group them somewhere, and force the user to implement all of them at once.
> 
> Having only one callback is not something I'm against.
> 
>> I was, and I am still ok with the current approach, of having from/to parameters and a "simple" callback type. But remove "from" because we can't get it right in some cases does not seem the best decision.
> 
> If you cannot rely on an input being a sensible value, doesn't that
> render the input useless?
> 

I agree. If for a specific event it's impossible to provide a value 
(i.e. the value has no meaning for a real cpu), it will just point that 
we need several types of data per event, and the compromise of having a 
single callback won't be possible.

We should differentiate "it's hard to find this value in QEMU" vs "this 
value does not exist in real life". The first can be solved if we put 
effort into it. And every time a cpu changes it's flow of execution, it 
makes sense to find where it was just before.

One of the end goals is to be able to build a full control flow graph, 
with edges labeled on transition type (exceptions, traps, interrupts, 
jump, fallback), which we can do with the triple {event,from,to}.

>> Let's try to move forward, and solve the problems we have with from_pc. The testing part can be solved already (as explained in a previous message). In which cases can't you identify from_pc?
> 
> I'll have to check, but problems that I discussed with a colleague
> included jumps to an unmapped page resulting in the appropriate
> exception. We ultimately agreed that in such a situation from_pc should
> point to the jump target inside the unmapped page, instead of, say, the
> jump. We assume that most targets should already behave this way without
> further changes. However, in order to compute the correct from_pc, we
> need to know the jump target before the exception is raised (i.e. right
> after the jump instruction is executed), and that's not necessarily
> straight-forward to do in a plugin.

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?

> 
> But as I wrote before in another message, I need to take another look at
> the cflow plugin.
> 
>> By the way, and if you are open to talk about naming.
> 
> I'm open to suggestions.
> 
>> I understand why you picked up discontinuity, which is coming from a mathematical background. However, I rarely see this term used in the literature for computer science, and people use "exceptional control flow" to qualify interrupts and traps. In more, when we'll integrate classic control flow (including fallback between tb), the term discontinuity will lose its meaning. For this reason, I think that {cflow,CFLOW} makes more sense.
> 
> Using the term "discontinuity" was, in fact, inspired by "uninferable PC
> discontinuities" defined in the RISC-V ETrace spec [1], arguably a
> technical document. We chose discontinuity over the notion of control
> flow because the PC is not the (only) thing affected by the event. In
> the case of host calls, we ideally don't even observe an effect on the
> PC. Thus control flow doesn't really fit the bill for those.
> 

I'm happy to read this spec reinvents a term clearly defined elsewhere 
(sigh...). Beyond the intellectual vocabulary creation pleasure, it's 
good to use term people can find on Google.

We can use this term though, PC discontinuity makes sense, and a 
fallback is a special case of discontinuity if we adopt this 
perspective. Thanks for explaining!

> Regards,
> Julian Ganz
> 
> [1] https://github.com/riscv-non-isa/riscv-trace-spec