Re: [DTrace-devel] [PATCH real-v2 1/4] cg, providers: refactor clearing probe arguments

Reviewed-by: Eugene Loh <[email protected]>

On 12/18/25 15:20, Kris Van Hees via DTrace-devel wrote:
> Signed-off-by: Kris Van Hees <[email protected]>
> ---
>   libdtrace/dt_cg.c           | 28 +++++++++++++++++++++++++---
>   libdtrace/dt_cg.h           |  1 +
>   libdtrace/dt_prov_cpc.c     | 17 -----------------
>   libdtrace/dt_prov_dtrace.c  |  8 ++------
>   libdtrace/dt_prov_profile.c | 23 +----------------------
>   libdtrace/dt_prov_sdt.c     |  7 ++-----
>   libdtrace/dt_prov_syscall.c | 10 ++--------
>   7 files changed, 33 insertions(+), 61 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index ec4e3123..a77c022c 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -454,6 +454,26 @@ dt_cg_tramp_copy_regs(dt_pcb_t *pcb)
>   	}
>   }
>   
> +/*
> + * Clear the content of the 'argv' member of the machine state, from the given
> + * index (idx).
> + *
> + * The caller must ensure that %r7 contains the value set by the
> + * dt_cg_tramp_prologue*() functions.
> + */
> +void
> +dt_cg_tramp_clear_argv(dt_pcb_t *pcb, int idx)
> +{
> +	dt_irlist_t	*dlp = &pcb->pcb_ir;
> +	int		i, argc = ARRAY_SIZE(((dt_mstate_t *)0)->argv);
> +
> +	if (idx >= argc)
> +		return;
> +
> +	for (i = idx; i < argc; i++)
> +		emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
> +}
> +
>   /*
>    * Copy arguments from a dt_pt_regs structure referenced by %r8.
>    * If 'called' is nonzero, the registers are laid out as when inside the
> @@ -626,6 +646,10 @@ dt_cg_tramp_copy_pc_from_regs(dt_pcb_t *pcb)
>   	/* done */
>   	emitl(dlp, Ldone,
>   	      BPF_NOP());
> +
> +	/* clear the rest of the arguments */
> +	dt_cg_tramp_clear_argv(pcb, 2);
> +
>   	dt_regset_free_args(drp);
>   }
>   
> @@ -640,15 +664,13 @@ void
>   dt_cg_tramp_copy_rval_from_regs(dt_pcb_t *pcb)
>   {
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
> -	int		i;
>   
>   	emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(0), 0));
>   
>   	emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, PT_REGS_RET));
>   	emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(1), BPF_REG_0));
>   
> -	for (i = 2; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
> -		emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
> +	dt_cg_tramp_clear_argv(pcb, 2);
>   }
>   
>   /*
> diff --git a/libdtrace/dt_cg.h b/libdtrace/dt_cg.h
> index 9493edbb..401ac0e0 100644
> --- a/libdtrace/dt_cg.h
> +++ b/libdtrace/dt_cg.h
> @@ -26,6 +26,7 @@ extern void dt_cg_tramp_prologue_cpu(dt_pcb_t *pcb);
>   extern void dt_cg_tramp_prologue(dt_pcb_t *pcb);
>   extern void dt_cg_tramp_clear_regs(dt_pcb_t *pcb);
>   extern void dt_cg_tramp_copy_regs(dt_pcb_t *pcb);
> +extern void dt_cg_tramp_clear_argv(dt_pcb_t *pcb, int i);
>   extern void dt_cg_tramp_copy_args_from_regs(dt_pcb_t *pcb, int called);
>   extern void dt_cg_tramp_copy_pc_from_regs(dt_pcb_t *pcb);
>   extern void dt_cg_tramp_copy_rval_from_regs(dt_pcb_t *pcb);
> diff --git a/libdtrace/dt_prov_cpc.c b/libdtrace/dt_prov_cpc.c
> index 57b11b13..fd33990f 100644
> --- a/libdtrace/dt_prov_cpc.c
> +++ b/libdtrace/dt_prov_cpc.c
> @@ -393,26 +393,9 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
>    */
>   static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>   {
> -	int		i;
> -	dt_irlist_t	*dlp = &pcb->pcb_ir;
> -
>   	dt_cg_tramp_prologue_cpu(pcb);
> -
> -	/*
> -	 * After the dt_cg_tramp_prologue_cpu() call, we have:
> -	 *				//     (%r7 = dctx->mst)
> -	 *				//     (%r8 = dctx->ctx)
> -	 */
> -
>   	dt_cg_tramp_copy_regs(pcb);
> -
> -	/*
> -	 * Use the PC to set arg0 and arg1, then clear the other args.
> -	 */
>   	dt_cg_tramp_copy_pc_from_regs(pcb);
> -	for (i = 2; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
> -		emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
> -
>   	dt_cg_tramp_epilogue(pcb);
>   
>   	return 0;
> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> index 34b5d8e2..453d3e58 100644
> --- a/libdtrace/dt_prov_dtrace.c
> +++ b/libdtrace/dt_prov_dtrace.c
> @@ -83,7 +83,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>   {
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
>   	dt_activity_t	act = DT_ACTIVITY_ACTIVE;
> -	uint32_t	i, key = 0;
> +	uint32_t	key = 0;
>   
>   	/*
>   	 * The ERROR probe isn't really a trace event that a BPF program is
> @@ -174,11 +174,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>   	emit(dlp, BPF_CALL_HELPER(BPF_FUNC_map_update_elem));
>   
>   	dt_cg_tramp_copy_regs(pcb);
> -
> -	/* zero the probe args */
> -	for (i = 0; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
> -		emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
> -
> +	dt_cg_tramp_clear_argv(pcb, 0);
>   	dt_cg_tramp_epilogue_advance(pcb, act);
>   
>   	return 0;
> diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
> index e1369ca9..cb65ca43 100644
> --- a/libdtrace/dt_prov_profile.c
> +++ b/libdtrace/dt_prov_profile.c
> @@ -216,24 +216,9 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
>    */
>   static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>   {
> -	int		i;
> -	dt_irlist_t	*dlp = &pcb->pcb_ir;
> -
>   	dt_cg_tramp_prologue_cpu(pcb);
> -
> -	/*
> -	 * After the dt_cg_tramp_prologue_cpu() call, we have:
> -	 *				//     (%r7 = dctx->mst)
> -	 *				//     (%r8 = dctx->ctx)
> -	 */
> -
>   	dt_cg_tramp_copy_regs(pcb);
> -
> -	/*
> -	 * dctx->mst->argv[0] = kernel PC
> -	 * dctx->mst->argv[1] = userspace PC
> -	 */
> -        dt_cg_tramp_copy_pc_from_regs(pcb);
> +	dt_cg_tramp_copy_pc_from_regs(pcb);
>   
>   	/*
>   	 * TODO: For profile-n probes:
> @@ -246,12 +231,6 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>   	 * and update expected+=interval.
>   	 */
>   
> -	/*
> -	 *     (we clear dctx->mst->argv[2] and on)
> -	 */
> -	for (i = 2; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
> -		emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
> -
>   	dt_cg_tramp_epilogue(pcb);
>   
>   	return 0;
> diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c
> index 74555f5b..5be43ebc 100644
> --- a/libdtrace/dt_prov_sdt.c
> +++ b/libdtrace/dt_prov_sdt.c
> @@ -174,11 +174,8 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>   		off += size;
>   	}
>   
> -	/*
> -	 * Clear the remainder of the cached arguments.
> -	 */
> -	for (; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
> -		emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
> +	/* Clear the remainder of the cached arguments.  */
> +	dt_cg_tramp_clear_argv(pcb, i);
>   
>   	dt_cg_tramp_epilogue(pcb);
>   
> diff --git a/libdtrace/dt_prov_syscall.c b/libdtrace/dt_prov_syscall.c
> index 20843c6f..237f2728 100644
> --- a/libdtrace/dt_prov_syscall.c
> +++ b/libdtrace/dt_prov_syscall.c
> @@ -159,14 +159,8 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>   		emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(i), BPF_REG_0));
>   	}
>   
> -	/*
> -	 * Zero the remaining probe args.
> -	 *	for ( ; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
> -	 *		dctx->mst->argv[i] = 0;
> -	 *				// stdw [%r7 + DMST_ARG(i)], 0
> -	 */
> -	for ( ; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
> -		emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
> +	/* Zero the remaining probe args. */
> +	dt_cg_tramp_clear_argv(pcb, i);
>   
>   	/*
>   	 * For return probes, store the errno.  That is, examine arg0.