[DTrace-devel] [PATCH 2/3] Allow dt_cg_subr_arg_to_tstring() to take any subroutine arg index

[Kris Van Hees]

On Thu, Dec 14, 2023 at 01:40:00PM -0500, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> The function dt_cg_subr_arg_to_tstring() is used to generate BPF
> code for a variety of subroutines that output temporary strings.
> It sets up a BPF function call where one arg is a key arg from
> the subroutine being implemented.  Typically the first subroutine
> arg is used.
> 
> Allow any subroutine arg to be used, both for calculating the
> arg value and checking pointers.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_cg.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 7a84be5d..0a9a09b3 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -4938,7 +4938,7 @@ dt_cg_array_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  static void
>  dt_cg_subr_arg_to_tstring(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>  			  const char *fname, int isreg4, uint32_t val4,
> -			  int isreg5, uint32_t val5)
> +			  int isreg5, uint32_t val5, int argnum)

I would suggest placing the argument in slightly different order (mostly making
sure the optional arguments are at the end):

  dt_cg_subr_arg_to_tstring(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 			    const char *fname, int argnum, int isreg4,
			    uint32_t val4, int isreg5, uint32_t val5)

>  {
>  	dt_ident_t	*idp;
>  	dt_node_t	*arg = dnp->dn_args;
> @@ -4950,10 +4950,12 @@ dt_cg_subr_arg_to_tstring(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>  	assert(dnp->dn_ident);
>  	isp = dnp->dn_ident->di_data;
>  	assert(isp && isp->dis_args);
> -	argtype = &isp->dis_args[0];
> +	argtype = &isp->dis_args[argnum];

I think you should add an assert that argnum is not outside the valid range
for the prototype (number of arguments).  Just to be safe.

>  
>  	/* handle the one "input value" */
>  	/* (its type matters only as to whether we check it is null */
> +	while (argnum-- > 0)
> +		arg = arg->dn_list;

I think you should add a check for arg != NULL or an assert to ensure that we
do not accidentally deref a NULL pointer.  That would be an internal compiler
error but still - just to be safe.

>  	dt_cg_node(arg, dlp, drp);
>  	if (dt_node_is_pointer(argtype) || dt_node_is_string(argtype))
>  		dt_cg_check_ptr_arg(dlp, drp, arg, NULL);
> @@ -5003,21 +5005,21 @@ static void
>  dt_cg_subr_basename(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  {
>  	dt_cg_subr_arg_to_tstring(dnp, dlp, drp, "dt_basename", DT_IGNOR, 0,
> -				  DT_IGNOR, 0);
> +				  DT_IGNOR, 0, 0);
>  }
>  
>  static void
>  dt_cg_subr_cleanpath(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  {
>  	dt_cg_subr_arg_to_tstring(dnp, dlp, drp, "dt_cleanpath", DT_IGNOR, 0,
> -				  DT_IGNOR, 0);
> +				  DT_IGNOR, 0, 0);
>  }
>  
>  static void
>  dt_cg_subr_dirname(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  {
>  	dt_cg_subr_arg_to_tstring(dnp, dlp, drp, "dt_dirname", DT_IGNOR, 0,
> -				  DT_IGNOR, 0);
> +				  DT_IGNOR, 0, 0);
>  }
>  
>  static void
> @@ -5083,7 +5085,7 @@ static void
>  dt_cg_subr_lltostr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  {
>  	dt_cg_subr_arg_to_tstring(dnp, dlp, drp, "dt_lltostr", DT_IGNOR, 0,
> -				  DT_IGNOR, 0);
> +				  DT_IGNOR, 0, 0);
>  }
>  
>  static void
> @@ -6248,7 +6250,7 @@ static void
>  dt_cg_subr_inet_ntoa(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  {
>  	dt_cg_subr_arg_to_tstring(dnp, dlp, drp, "dt_inet_ntoa", DT_IGNOR, 0,
> -				  DT_IGNOR, 0);
> +				  DT_IGNOR, 0, 0);
>  }
>  
>  static void
> @@ -6314,7 +6316,7 @@ dt_cg_subr_inet_ntoa6(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  
>  	dt_cg_subr_arg_to_tstring(dnp, dlp, drp, "dt_inet_ntoa6",
>  				  DT_ISIMM, tbloff,
> -				  DT_ISIMM, strict && strict->dn_value ? 1 : 0);
> +				  DT_ISIMM, strict && strict->dn_value ? 1 : 0, 0);
>  }
>  
>  static void
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel