[DTrace-devel] [PATCH 52/61] Allow dt_cg_arglist() to handle the NULL case

[Kris Van Hees]

On Fri, Jul 08, 2022 at 10:45:36AM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> Even if args==NULL, we might want to call dt_cg_arglist() to
> construct a pointer to a tuple.  So add another mechanism for
> retrieving the register with that pointer and protect against
> dereferencing a NULL pointer.

I agree that dt_cg_arglist() needs to support an empty args list in view of how
you are making non-indexed aggregations a variant of indexed aggregations.

But I disagree with how you are implementing it.

> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_cg.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 4a22c1db..5a74f5ad 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2639,7 +2639,7 @@ dt_cg_push_stack(dt_irlist_t *dlp, dt_regset_t *drp, int reg)
>   * Note that we leave space at the beginning of the tuple for a uint32_t value,
>   * and at the end space for a uint64_t value.
>   */
> -static void
> +static int

This is the core of my objection - dt_cg_arglist suddenly returning a register
is (in my opinion) is changing things beyond what is needed.  All that should
be needed is making sure that it can accept empty argument lists.

So, instead (I will post an alternative patch in a moment) I think we should
actually support having a void-typed node that is passed as args (can be
created on the fly in dt_cg_agg()) and some logic added to dt_xg_arglist() to 
recognize a void-types args node as an empty argument list.

>  dt_cg_arglist(dt_ident_t *idp, dt_node_t *args, dt_irlist_t *dlp,
>  	      dt_regset_t *drp)
>  {
> @@ -2814,9 +2814,12 @@ dt_cg_arglist(dt_ident_t *idp, dt_node_t *args, dt_irlist_t *dlp,
>  	emit(dlp, BPF_LOAD(BPF_DW, treg, treg, DCTX_MEM));
>  	emit(dlp, BPF_ALU64_IMM(BPF_ADD, treg, DMEM_TUPLE(dtp)));
>  
> -	args->dn_reg = treg;
> +	if (args != NULL)
> +		args->dn_reg = treg;
>  
>  	TRACE_REGSET("      arglist: End  ");
> +
> +	return treg;
>  }
>  
>  /*
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel