Re: [DTrace-devel] [PATCH 1/2] Clarify how the usdt_prids key is stored on the BPF stack

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

On Wed, Feb 19, 2025 at 11:43:49PM -0500, eugene.loh@oracle.com wrote:
> 
> While one can access the BPF stack relative to %r9, the whole
> point of DT_TRAMP_SP_SLOT(0) is to make trampoline code more
> readable.  So use it.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  libdtrace/dt_prov_uprobe.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index 5d9f74244..f1323cc31 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -1015,22 +1015,15 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  	emit(dlp,  BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32));
>  
>  	/*
> -	 * Look up in the BPF 'usdt_prids' map.  Space for the look-up key
> -	 * will be used on the BPF stack:
> -	 *
> -	 *     offset                                       value
> -	 *
> -	 *     -sizeof(usdt_prids_map_key_t)                pid (in %r0)
> -	 *
> -	 *     -sizeof(usdt_prids_map_key_t) + sizeof(pid_t)
> -	 *     ==
> -	 *     -sizeof(dtrace_id_t)                         underlying-probe prid
> +	 * Look up in the BPF 'usdt_prids' map.  The key should fit into
> +	 * trampoline stack slot 0.
>  	 */
> -	emit(dlp,  BPF_STORE(BPF_W, BPF_REG_9, (int)(-sizeof(usdt_prids_map_key_t)), BPF_REG_0));
> -	emit(dlp,  BPF_STORE_IMM(BPF_W, BPF_REG_9, (int)(-sizeof(dtrace_id_t)), uprp->desc->id));
> +	assert(sizeof(usdt_prids_map_key_t) <= DT_STK_SLOT_SZ);
> +	emit(dlp,  BPF_STORE(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0), BPF_REG_0));
> +	emit(dlp,  BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0) + sizeof(pid_t), uprp->desc->id));
>  	dt_cg_xsetx(dlp, usdt_prids, DT_LBL_NONE, BPF_REG_1, usdt_prids->di_id);
> -	emit(dlp,  BPF_MOV_REG(BPF_REG_2, BPF_REG_9));
> -	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, (int)(-sizeof(usdt_prids_map_key_t))));
> +	emit(dlp,  BPF_MOV_REG(BPF_REG_2, BPF_REG_FP));
> +	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, DT_TRAMP_SP_SLOT(0)));
>  	emit(dlp,  BPF_CALL_HELPER(BPF_FUNC_map_lookup_elem));
>  	emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_exit));
>  
> -- 
> 2.43.5
> 
>