[DTrace-devel] [PATCH v3] cg: allow providers to specify a skip count for stack retrieval
Eugene Loh
eugene.loh at oracle.com
Thu May 2 00:52:17 UTC 2024
Looks good to me (famous last words), though I confess I do not know:
what tests show the problem/fix?
Assuming there is test coverage of the problem/fix... ideally
stack/ustack, fbt/non-fbt, caller, stackdepth:
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
On 5/1/24 16:52, 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>
> ---
> bpf/get_bvar.c | 13 +++++++++----
> libdtrace/dt_bpf.h | 1 +
> libdtrace/dt_cc.c | 3 +++
> libdtrace/dt_cg.c | 11 +++++++++--
> libdtrace/dt_dlibs.c | 1 +
> libdtrace/dt_prov_fbt.c | 1 +
> libdtrace/dt_provider.h | 1 +
> 7 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
> index 71eef277..ea5dc6b1 100644
> --- a/bpf/get_bvar.c
> +++ b/bpf/get_bvar.c
> @@ -28,6 +28,7 @@ extern uint64_t STBSZ;
> extern uint64_t STKSIZ;
> extern uint64_t BOOTTM;
> extern uint64_t STACK_OFF;
> +extern uint64_t STACK_SKIP;
>
> #define error(dctx, fault, illval) \
> ({ \
> @@ -64,12 +65,14 @@ noinline uint64_t dt_get_bvar(const dt_dctx_t *dctx, uint32_t id, uint32_t idx)
> case DIF_VAR_STACKDEPTH:
> case DIF_VAR_USTACKDEPTH: {
> uint32_t bufsiz = (uint32_t) (uint64_t) (&STKSIZ);
> - uint64_t flags = 0 & BPF_F_SKIP_FIELD_MASK;
> + uint64_t flags;
> char *buf = dctx->mem + (uint64_t)(&STACK_OFF);
> uint64_t stacksize;
>
> if (id == DIF_VAR_USTACKDEPTH)
> - flags |= BPF_F_USER_STACK;
> + flags = BPF_F_USER_STACK;
> + else
> + flags = (uint64_t)(&STACK_SKIP) & BPF_F_SKIP_FIELD_MASK;
>
> stacksize = bpf_get_stack(dctx->ctx, buf, bufsiz, flags);
> if (stacksize < 0)
> @@ -89,11 +92,13 @@ noinline uint64_t dt_get_bvar(const dt_dctx_t *dctx, uint32_t id, uint32_t idx)
> }
> case DIF_VAR_CALLER:
> case DIF_VAR_UCALLER: {
> - uint64_t flags = 0 & BPF_F_SKIP_FIELD_MASK;
> + uint64_t flags;
> uint64_t buf[2] = { 0, };
>
> if (id == DIF_VAR_UCALLER)
> - flags |= BPF_F_USER_STACK;
> + flags = BPF_F_USER_STACK;
> + else
> + flags = (uint64_t)(&STACK_SKIP) & BPF_F_SKIP_FIELD_MASK;
>
> if (bpf_get_stack(dctx->ctx, buf, sizeof(buf), flags) < 0)
> return 0;
> diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h
> index 3b9fc633..5b2df264 100644
> --- a/libdtrace/dt_bpf.h
> +++ b/libdtrace/dt_bpf.h
> @@ -55,6 +55,7 @@ extern "C" {
> #define DT_CONST_RODATA_SIZE 21
> #define DT_CONST_ZERO_OFF 22
> #define DT_CONST_STACK_OFF 23
> +#define DT_CONST_STACK_SKIP 24
>
> #define DT_BPF_LOG_SIZE_DEFAULT (UINT32_MAX >> 8)
> #define DT_BPF_LOG_SIZE_SMALL 4096
> diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> index cb07b8d6..d1ee3843 100644
> --- a/libdtrace/dt_cc.c
> +++ b/libdtrace/dt_cc.c
> @@ -1193,6 +1193,9 @@ dt_link_construct(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> case DT_CONST_STACK_OFF:
> nrp->dofr_data = DMEM_STACK(dtp);
> continue;
> + case DT_CONST_STACK_SKIP:
> + nrp->dofr_data = prp->prov->impl->stack_skip;
> + continue;
> default:
> /* probe name -> value is probe id */
> if (strchr(idp->di_name, ':') != NULL)
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 27246a40..a1c24e37 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2597,6 +2597,9 @@ dt_cg_act_stack_sub(dt_pcb_t *pcb, dt_node_t *dnp, int reg, int off, dtrace_actk
> uint64_t arg;
> int nframes, stacksize, prefsz, align = sizeof(uint64_t);
> uint_t lbl_valid = dt_irlist_label(dlp);
> + dt_ident_t *skip = dt_dlib_get_var(dtp, "STACK_SKIP");
> +
> + assert(skip != NULL);
>
> /* Get sizing information from dnp->dn_arg. */
> arg = dt_cg_stack_arg(dtp, dnp, kind);
> @@ -2644,8 +2647,12 @@ 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)
> - | (kind == DTRACEACT_USTACK ? BPF_F_USER_STACK : 0)));
> + if (kind == DTRACEACT_USTACK)
> + emit(dlp, BPF_MOV_IMM(BPF_REG_4, BPF_F_USER_STACK));
> + else {
> + emite(dlp, BPF_MOV_IMM(BPF_REG_4, -1), skip);
> + emit(dlp, BPF_ALU64_IMM(BPF_AND, BPF_REG_4, BPF_F_SKIP_FIELD_MASK));
> + }
> dt_regset_xalloc(drp, BPF_REG_0);
> emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_stack));
> dt_regset_free_args(drp);
> diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
> index fe22616a..bc883e11 100644
> --- a/libdtrace/dt_dlibs.c
> +++ b/libdtrace/dt_dlibs.c
> @@ -96,6 +96,7 @@ static const dt_ident_t dt_bpf_symbols[] = {
> DT_BPF_SYMBOL_ID(RODATA_SIZE, DT_IDENT_SCALAR, DT_CONST_RODATA_SIZE),
> DT_BPF_SYMBOL_ID(ZERO_OFF, DT_IDENT_SCALAR, DT_CONST_ZERO_OFF),
> DT_BPF_SYMBOL_ID(STACK_OFF, DT_IDENT_SCALAR, DT_CONST_STACK_OFF),
> + DT_BPF_SYMBOL_ID(STACK_SKIP, DT_IDENT_SCALAR, DT_CONST_STACK_SKIP),
>
> /* End-of-list marker */
> { NULL, }
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 0ddffa20..fa888ed8 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,
> + .stack_skip = 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..17b1844c 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 stack_skip; /* # 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);
More information about the DTrace-devel
mailing list