Re: [PATCH v5 1/6] usdt: have copy_args() count args while parsing them

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

On Thu, Jul 10, 2025 at 10:59:25PM +0100, Alan Maguire wrote:
> stapsdt probes do not include an argument count, so the only
> way to count them is to parse the parameter string.  Adjust
> copy_args() to set upp->sargc while parsing upp->sargv.

I am looking at reworking this a bit.  The main challenge I see is that
it makes the overall copy_args() loop slower for *all* cases, not just
stapsdt.  At a minimum, replacing the strlen(p) with a check for *p != '\0'
might be better.

But there is also the issue that given how late copy_args() is executed,
setting sargc become a moot point.  It is not used after copy_args() is
called, so setting upp->sargc becomes pointless.

On the other hand, what we do want to know is how many arguments the probe
provides *before* we compile clauses, though in the adsence of real type
info, we could get away with not doing that.  Having the number of args
allows the use of args[n].

> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  libdtrace/dt_prov_uprobe.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index cf5cfd43..2cbd8910 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -1145,9 +1145,9 @@ static void enable_usdt(dtrace_hdl_t *dtp, dt_probe_t *prp)
>  }
>  
>  /*
> - * Generate code that populates the probe arguments.
> + * Generate code that populates, counts the probe arguments.
>   */
> -static void copy_args(dt_pcb_t *pcb, const dt_uprobe_t *upp)
> +static void copy_args(dt_pcb_t *pcb, dt_uprobe_t *upp)
>  {
>  	dtrace_hdl_t	*dtp = pcb->pcb_hdl;
>  	dt_irlist_t	*dlp = &pcb->pcb_ir;
> @@ -1158,7 +1158,7 @@ static void copy_args(dt_pcb_t *pcb, const dt_uprobe_t *upp)
>  
>  	assert(pvp != NULL);
>  
> -	for (i = 0; i < upp->sargc; i++) {
> +	for (i = 0; strlen(p) > 0; i++) {
>  		int	ssize, disp, len;
>  		char	*reg = NULL;
>  		int64_t	val = 0;
> @@ -1425,6 +1425,7 @@ static void copy_args(dt_pcb_t *pcb, const dt_uprobe_t *upp)
>  			usdt_error(pcb, "Unknown format in arg%d spec", i);
>  #endif
>  	}
> +	upp->sargc = i;
>  }
>  
>  /*
> @@ -1445,7 +1446,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  	dtrace_hdl_t		*dtp = pcb->pcb_hdl;
>  	dt_irlist_t		*dlp = &pcb->pcb_ir;
>  	const dt_probe_t	*uprp = pcb->pcb_probe;
> -	const dt_uprobe_t	*upp = uprp->prv_data;
> +	dt_uprobe_t		*upp = uprp->prv_data;
>  	const list_probe_t	*pop;
>  	uint_t			lbl_exit = pcb->pcb_exitlbl;
>  	dt_ident_t		*usdt_prids = dt_dlib_get_map(dtp, "usdt_prids");
> @@ -1519,7 +1520,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  	if (upp->flags & PP_IS_RETURN)
>  		goto out;
>  
> -	if (upp->sargc)
> +	if (upp->sargv)
>  		copy_args(pcb, upp);
>  	else
>  		dt_cg_tramp_copy_args_from_regs(pcb, 0);
> -- 
> 2.43.5
>