[DTrace-devel] [PATCH] Preface usym/umod/uaddr with pid

[Kris Van Hees]

On Tue, Nov 19, 2024 at 06:51:25PM -0500, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> Translation of user space addresses requires that we know which process
> to use for the translation.  We have been prefacing the address to
> translate like this:
>         /* Preface the value with the user process tgid. */
>         emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_current_pid_tgid));
>         emit(dlp, BPF_ALU64_IMM(BPF_AND, BPF_REG_0, 0xffffffff));
> 
> One problem is that the IMM "and" operation is a no-op.  The 0xffffffff
> is promoted to a 64-bit -1, leading to no %r0 bits changing.  Another
> problem is that we are pulling out the tgid.  This means that the
> translation will fail.  Further, even if the consumer knew which part
> of the pid_tgid combination to use for address translation, keys for
> aggregations could fail spuriously due to different tgids for the same
> process.
> 
> Convert to
>         /* Preface the value with the user process pid. */
>         emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_current_pid_tgid));
>         emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32));
> 
> Also, in the umod test (which uncovered this problem), add ld-*.so to
> the list of shared objects we're going to skip.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>

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

> ---
>  libdtrace/dt_cg.c                   | 8 ++++----
>  test/unittest/profile-n/tst.umod.sh | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 461a68270..b07aaea3d 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1647,13 +1647,13 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
>  	    kind == DTRACEACT_UADDR) {
>  		off = dt_rec_add(dtp, dt_cg_fill_gap, kind, 16, 8, NULL, arg);
>  
> -		/* preface the value with the user process tgid */
> +		/* preface the value with the user process pid */
>  		if (dt_regset_xalloc_args(drp) == -1)
>  			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  		dt_regset_xalloc(drp, BPF_REG_0);
>  		emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_current_pid_tgid));
>  		dt_regset_free_args(drp);
> -		emit(dlp, BPF_ALU64_IMM(BPF_AND, BPF_REG_0, 0xffffffff));
> +		emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32));
>  		emit(dlp, BPF_STORE(BPF_DW, BPF_REG_9, off, BPF_REG_0));
>  		dt_regset_free(drp, BPF_REG_0);
>  
> @@ -3823,13 +3823,13 @@ empty_args:
>  				if (tuplesize < nextoff)
>  					emit(dlp,  BPF_ALU64_IMM(BPF_ADD, treg, nextoff - tuplesize));
>  
> -				/* Preface the value with the user process tgid. */
> +				/* Preface the value with the user process pid. */
>  				if (dt_regset_xalloc_args(drp) == -1)
>  					longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  				dt_regset_xalloc(drp, BPF_REG_0);
>  				emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_current_pid_tgid));
>  				dt_regset_free_args(drp);
> -				emit(dlp, BPF_ALU64_IMM(BPF_AND, BPF_REG_0, 0xffffffff));
> +				emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32));
>  				emit(dlp, BPF_STORE(BPF_DW, treg, 0, BPF_REG_0));
>  				dt_regset_free(drp, BPF_REG_0);
>  
> diff --git a/test/unittest/profile-n/tst.umod.sh b/test/unittest/profile-n/tst.umod.sh
> index a5b1127ff..45d2b1e9b 100755
> --- a/test/unittest/profile-n/tst.umod.sh
> +++ b/test/unittest/profile-n/tst.umod.sh
> @@ -53,7 +53,7 @@ if ! grep -wq 'bash' $tmpfile; then
>  fi
>  
>  # Check that modules are unique.  (Exclude shared libraries and unresolved addresses.)
> -if gawk '!/^ *lib/ && !/^ *0x/ {print $1}' $tmpfile | sort | uniq -c | grep -qv " 1 "; then
> +if gawk '!/^ *lib/ && !/^ *ld-.*\.so / && !/^ *0x/ {print $1}' $tmpfile | sort | uniq -c | grep -qv " 1 "; then
>  	echo ERROR: duplicate umod
>  	status=1
>  fi
> -- 
> 2.43.5
>