[DTrace-devel] [PATCH v3] Add support for inet_ntoa6() subroutine

[Eugene Loh]

There should be a test with strsize < 72.  There should also be a test 
with strsize so short that it clips the output (e.g., strsize=4).  
Specifically....

On 8/5/23 10:48, Kris Van Hees via DTrace-devel wrote:
> diff --git a/bpf/inet_ntoa6.S b/bpf/inet_ntoa6.S
> [...]
> +	/* Set the upper bound for %r7. */
> +	and	%r7, 0xff		/* Needed for BPF verifier */
> [...]
> +
> +	ldxdw	%r6, [%r6 + DCTX_RODATA]
> +	ldxw	%r1, [%fp + -4]		/* restore tbloff */
> +	lddw	%r0, RODATA_SIZE
> +	jge	%r1, %r0, .Ldone
> +	add	%r6, %r1		/* %r6 = dctx->rodata + tbloff */
> +	add	%r6, %r7
> +	ldxb	%r7, [%r6 + 0]		/* %r7 = tbl[%r7] */

Here is a good reason to have such tests.  What happens with shorter 
strsize?

I think we have the nonzero mask in r7.  We make sure it's at most 255.  
We have the rodata pointer in r6.  We have the tbloff in r1, and we make 
sure it doesn't exceed RODATA_SIZE.  Then we look a byte up at r6+r1+r7, 
which could be at r6+RODATA_SIZE+255.

I think the thing to do is to skip the r7&=0xff.  Just do a "r1+=r7" 
before the r1>=r0 check.  Then skip the r6+=r7.

> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> @@ -5878,6 +5878,65 @@ dt_cg_subr_inet_ntoa(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> +static void
> +dt_cg_subr_inet_ntoa6(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> +{
> +	dtrace_hdl_t	*dtp = yypcb->pcb_hdl;
> +	ssize_t		tbloff;
> +	/*
> +	 * This table encodes the start and length of the longest run of 0 bits
> +	 * in the binary representation of the table index, if any such run of
> +	 * length >= 2 exists.  Each value pair (start, length) is stored as a
> +	 * single byte: (start << 4) | length.  If no rush run exists, the

s/rush/such/?

> +	 * value 0x70 is used.
> +	 *
> +	 * See bpf/inet_ntoa6.S for details on how this table is used.
> +	 */
> diff --git a/libdtrace/dt_dctx.h b/libdtrace/dt_dctx.h
>   #define DMEM_TSTR_SZ(dtp) \
> -		(DT_TSTRING_SLOTS * \
> -		 P2ROUNDUP((dtp)->dt_options[DTRACEOPT_STRSIZE] + 1, 8))
> +		(DT_TSTRING_SLOTS * DT_TSTRING_SIZE(dtp))

The resulting line is short enough that it can be combined with the 
#define line.

> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> + *
> + * Each tstring needs to be large enough to hold the largest possible string
> + * and accomodate the largest known need for tstring space in subroutines.
>    */
>   #define DT_TSTRING_SLOTS	4
> +#define DT_TSTRING_SIZE(dtp)	\
> +		MAX(P2ROUNDUP((dtp)->dt_options[DTRACEOPT_STRSIZE] + 1, 8), \
> +		    72)

The reader of this patch might understand the "72" but the reader of 
this code would be left wondering where the 72 comes from.