Re: [PATCH] fbt: implement return value support for fexit-based FBT return probes

[Kris Van Hees <kris.van.hees@oracle.com>]

On Thu, Aug 01, 2024 at 01:49:35AM -0400, Eugene Loh wrote:
> Is there a test?

Yes, the testsuite already contains a test (tst.return1.d) whose failure
prompted the need for this patch.  And with the patch, it passes.

> 
> On 8/1/24 01:19, Kris Van Hees wrote:
> > Commit fe2101e5 ("fbt: implement based on fentry/fexit probes") failed
> > to provide a proper implementation to pass the function return value as
> > arg1 for FBT return probes based on fexit probes.
> > 
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > ---
> >   libdtrace/dt_prov_fbt.c | 20 +++++++++++++++++++-
> >   1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> > index fa888ed8..99b42586 100644
> > --- a/libdtrace/dt_prov_fbt.c
> > +++ b/libdtrace/dt_prov_fbt.c
> > @@ -172,6 +172,7 @@ static int populate(dtrace_hdl_t *dtp)
> >    */
> >   static int fprobe_trampoline(dt_pcb_t *pcb, uint_t exitlbl)
> >   {
> > +	dtrace_hdl_t	*dtp = pcb->pcb_hdl;
> >   	dt_irlist_t	*dlp = &pcb->pcb_ir;
> >   	dt_probe_t	*prp = pcb->pcb_probe;
> > @@ -185,6 +186,8 @@ static int fprobe_trampoline(dt_pcb_t *pcb, uint_t exitlbl)
> >   			emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(i), BPF_REG_0));
> >   		}
> >   	} else {
> > +		dt_module_t	*dmp;
> > +
> >   		/*
> >   		 * fbt:::return arg0 should be the function offset for the
> >   		 * return instruction.  The fexit prpbe fires at a point where
> > @@ -194,6 +197,21 @@ static int fprobe_trampoline(dt_pcb_t *pcb, uint_t exitlbl)
> >   		 */
> >   		dt_cg_xsetx(dlp, NULL, DT_LBL_NONE, BPF_REG_0, -1);
> >   		emit(dlp,  BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(0), BPF_REG_0));
> > +
> > +		/*
> > +		 * The return value is provided by the fexit probe as an
> > +		 * argument slot past the last function argument.  We can get
> > +		 * the number of function arguments using the BTF id that has
> > +		 * been stored as the tracepoint event id.
> > +		 */
> > +		dmp = dt_module_lookup_by_name(dtp, prp->desc->mod);
> > +		if (dmp != NULL) {
> > +			int32_t	btf_id = dt_tp_get_event_id(prp);
> > +			int	i = dt_btf_func_argc(dtp, dmp->dm_btf, btf_id);
> > +
> > +			emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, i * 8));
> > +			emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(1), BPF_REG_0));
> > +		}
> >   	}
> >   	dt_cg_tramp_epilogue(pcb);
> > @@ -274,7 +292,7 @@ static int fprobe_prog_load(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> >   	dt_module_t			*dmp;
> >   	atype = strcmp(desc->prb, "entry") == 0 ? BPF_TRACE_FENTRY
> > -						     : BPF_TRACE_FEXIT;
> > +						: BPF_TRACE_FEXIT;
> >   	dmp = dt_module_lookup_by_name(dtp, desc->mod);
> >   	if (dmp == NULL)