[DTrace-devel] [PATCH] cg: allow providers to specify a skip count for stack retrieval

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

I haven't played much with stack/fentry/fexit, so I hope this is right 
(e.g., stable number of skip frames).  Are our tests robust enough to 
assure us?

BUT!  What about stackdepth in bpf/get_bvar.c?

On 5/1/24 14:22, Kris Van Hees wrote:
> Some probes cause extra frames to be reported with bpf_get_stack() that
> need to be skipped to ensure we report the correct stack trace to the
> consumer.  FBT probes based on fentry/fexit probes need this.
>
> Signed-off-by: Kris Van Hees<kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_cg.c       | 6 ++++--
>   libdtrace/dt_prov_fbt.c | 1 +
>   libdtrace/dt_provider.h | 1 +
>   3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 27246a40..36975406 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2595,7 +2595,8 @@ dt_cg_act_stack_sub(dt_pcb_t *pcb, dt_node_t *dnp, int reg, int off, dtrace_actk
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
>   	dt_regset_t	*drp = pcb->pcb_regs;
>   	uint64_t	arg;
> -	int		nframes, stacksize, prefsz, align = sizeof(uint64_t);
> +	int		nframes, stacksize, prefsz, skip;
> +	int		align = sizeof(uint64_t);
>   	uint_t		lbl_valid = dt_irlist_label(dlp);
>   
>   	/* Get sizing information from dnp->dn_arg. */
> @@ -2603,6 +2604,7 @@ dt_cg_act_stack_sub(dt_pcb_t *pcb, dt_node_t *dnp, int reg, int off, dtrace_actk
>   	prefsz = kind == DTRACEACT_USTACK ? sizeof(uint64_t) : 0;
>   	nframes = DTRACE_USTACK_NFRAMES(arg);
>   	stacksize = nframes * sizeof(uint64_t);
> +	skip = yypcb->pcb_probe->prov->impl->skip_frames;
>   
>   	/* Handle alignment and reserve space in the output buffer. */
>   	if (reg >= 0) {
> @@ -2644,7 +2646,7 @@ dt_cg_act_stack_sub(dt_pcb_t *pcb, dt_node_t *dnp, int reg, int off, dtrace_actk
>   		emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, off + prefsz));
>   	}
>   	emit(dlp,  BPF_MOV_IMM(BPF_REG_3, stacksize));
> -	emit(dlp,  BPF_MOV_IMM(BPF_REG_4, (0 & BPF_F_SKIP_FIELD_MASK)
> +	emit(dlp,  BPF_MOV_IMM(BPF_REG_4, (skip & BPF_F_SKIP_FIELD_MASK)
>   					  | (kind == DTRACEACT_USTACK ? BPF_F_USER_STACK : 0)));
>   	dt_regset_xalloc(drp, BPF_REG_0);
>   	emit(dlp,  BPF_CALL_HELPER(BPF_FUNC_get_stack));
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 0ddffa20..dfeecfd1 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -431,6 +431,7 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp)
>   dt_provimpl_t	dt_fbt_fprobe = {
>   	.name		= prvname,
>   	.prog_type	= BPF_PROG_TYPE_TRACING,
> +	.skip_frames	= 4,
>   	.populate	= &populate,
>   	.load_prog	= &fprobe_prog_load,
>   	.trampoline	= &fprobe_trampoline,
> diff --git a/libdtrace/dt_provider.h b/libdtrace/dt_provider.h
> index a24b1d00..bf3af0be 100644
> --- a/libdtrace/dt_provider.h
> +++ b/libdtrace/dt_provider.h
> @@ -44,6 +44,7 @@ typedef struct dt_argdesc {
>   typedef struct dt_provimpl {
>   	const char *name;			/* provider generic name */
>   	int prog_type;				/* BPF program type */
> +	uint32_t skip_frames;			/* # of stack frames to skip */
>   	int (*populate)(dtrace_hdl_t *dtp);	/* register probes */
>   	int (*provide)(dtrace_hdl_t *dtp,	/* provide probes */
>   		       const dtrace_probedesc_t *pdp);
> -- 2.42.0