[DTrace-devel] [PATCH 3/4] ERROR probe implementation

[Eugene Loh]

On 1/23/21 11:31 AM, Kris Van Hees wrote:

> On Sat, Jan 23, 2021 at 03:16:01AM -0500, Eugene Loh wrote:
>
>> I'm confused by the comment block in front of dt_cg_probe_error().  It says,
>>       Generate code for a fault condition.  A call is made to dt_probe_error() to
>>       set the fault information, and then a non-zero return from the function is
>>       performed.
>> What does "non-zero return is performed" mean?  I assume this is
>> related: the (redacted) code has
>>       dt_regset_xalloc_args(drp);
>>       BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX)
>>       BPF_MOV_IMM(BPF_REG_2, off)
>>       BPF_MOV_IMM(BPF_REG_3, fault)
>>       BPF_MOV_IMM(BPF_REG_4, illval)
>>       dt_regset_xalloc(drp, BPF_REG_0);   // spill %r0???
>>       BPF_CALL_FUNC("dt_probe_error")     // %r0 gets return value
>>       dt_regset_free_args(drp);
>>       dt_regset_free(drp, BPF_REG_0);     //  fill %r0???
>>       BPF_RETURN()
>> That is, when we BPF_RETURN(), the return value %r0 is either that from
>> dt_probe_error() or else from before the call, depending on whether %r0
>> had to be spilled and filled.  Do I understand that correctly?  I'm
>> confused.  And the comment inside dt_cg_probe_error() says "return
>> dt_probe_error()" while in bpf/probe_error.*, dt_probe_error() is void.
>> Again, I'm confused.
> Ah yes.  That was a comment from a previous implementation of dt_probe_error
> that got changed later.  We no longer care about the return value so I will
> update the comment.
>
> By the way, yes, the dt_regset_free for %r0 should be after the RETURN.
This last sentence confuses me.  If the regset_free(%r0) comes after the 
return, then the code path that follows the return will not fill %r0 if 
it had been spilled.  Given that one does not care about the return 
value, regset_free(%r0) before the return would seem to be fine (more 
than fine: correct).