[DTrace-devel] [PATCH v2 3/5] Include aggregates in the DIFO vartab

[Eugene Loh]

I'm curious about two things:


On 11/24/2020 01:50 PM, Kris Van Hees wrote:
> Aggregates used to be handled as special identifiers.  Under the new
> design they will be handled more like variables and it is therefore
> warranted to include them in the variable table.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   include/dtrace/dif_defines.h |  1 +
>   libdtrace/dt_as.c            | 24 +++++++++++++++++++-----
>   libdtrace/dt_dis.c           |  3 +++
>   3 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/include/dtrace/dif_defines.h b/include/dtrace/dif_defines.h
> index b983c45c..216bce42 100644
> --- a/include/dtrace/dif_defines.h
> +++ b/include/dtrace/dif_defines.h
> @@ -275,6 +275,7 @@ typedef uint32_t	dif_instr_t;
>   
>   #define DIFV_KIND_ARRAY		0
>   #define DIFV_KIND_SCALAR	1
> +#define DIFV_KIND_AGGREGATE	2
>   
>   #define DIFV_SCOPE_GLOBAL	0
>   #define DIFV_SCOPE_THREAD	1
> diff --git a/libdtrace/dt_as.c b/libdtrace/dt_as.c
> index 3169cf66..437a2d86 100644
> --- a/libdtrace/dt_as.c
> +++ b/libdtrace/dt_as.c
> @@ -61,7 +61,9 @@ dt_countvar(dt_idhash_t *dhp, dt_ident_t *idp, void *data)
>   	size_t *np = data;
>   
>   	if (idp->di_flags & (DT_IDFLG_DIFR | DT_IDFLG_DIFW))
> -		(*np)++; /* include variable in vartab */
> +		(*np)++;		/* include variable in vartab */
> +	else if (idp->di_kind == DT_IDENT_AGG)
> +		(*np)++;		/* include variable in vartab */

How about collapsing those to be:
         if (idp->di_flags & (DT_IDFLG_DIFR | DT_IDFLG_DIFW))||
             idp->di_kind == DT_IDENT_AGG)
                 (*np)++;        /* include variable in vartab */

That would be more compact and would also mimic the expression in 
dt_copyvar(), seen immediately following.

>   	return (0);
>   }
> @@ -76,8 +78,9 @@ dt_copyvar(dt_idhash_t *dhp, dt_ident_t *idp, dtrace_hdl_t *dtp)
>   	ssize_t		stroff;
>   	dt_node_t	dn;
>   
> -	if (!(idp->di_flags & (DT_IDFLG_DIFR | DT_IDFLG_DIFW)))
> -		return (0); /* omit variable from vartab */
> +	if (!(idp->di_flags & (DT_IDFLG_DIFR | DT_IDFLG_DIFW)) &&
> +	    idp->di_kind != DT_IDENT_AGG)
> +		return 0;		/* omit variable from vartab */
>   
>   	dvp = &pcb->pcb_difo->dtdo_vartab[pcb->pcb_asvidx++];
>   	stroff = dt_strtab_insert(dtp->dt_ccstab, idp->di_name);
> @@ -91,8 +94,17 @@ dt_copyvar(dt_idhash_t *dhp, dt_ident_t *idp, dtrace_hdl_t *dtp)
>   	dvp->dtdv_id = idp->di_id;
>   	dvp->dtdv_flags = 0;
>   
> -	dvp->dtdv_kind = (idp->di_kind == DT_IDENT_ARRAY) ?
> -	    DIFV_KIND_ARRAY : DIFV_KIND_SCALAR;
> +	switch (idp->di_kind) {
> +	case DT_IDENT_AGG:
> +		dvp->dtdv_kind = DIFV_KIND_AGGREGATE;
> +		break;
> +	case DT_IDENT_ARRAY:
> +		dvp->dtdv_kind = DIFV_KIND_ARRAY;
> +		break;
> +	default:
> +		dvp->dtdv_kind = DIFV_KIND_SCALAR;
> +	}
> +
>   
>   	if (idp->di_flags & DT_IDFLG_LOCAL)
>   		dvp->dtdv_scope = DIFV_SCOPE_LOCAL;
> @@ -358,6 +370,7 @@ fail:
>   	 * table we insert the corresponding variable names into the strtab.
>   	 */
>   	dt_idhash_iter(dtp->dt_tls, dt_countvar, &n);
> +	dt_idhash_iter(dtp->dt_aggs, dt_countvar, &n);
>   	dt_idhash_iter(dtp->dt_globals, dt_countvar, &n);
>   	dt_idhash_iter(pcb->pcb_locals, dt_countvar, &n);
>   
> @@ -369,6 +382,7 @@ fail:
>   			longjmp(pcb->pcb_jmpbuf, EDT_NOMEM);
>   
>   		dt_idhash_iter(dtp->dt_tls, (dt_idhash_f *)dt_copyvar, dtp);
> +		dt_idhash_iter(dtp->dt_aggs, (dt_idhash_f *)dt_copyvar, dtp);
>   		dt_idhash_iter(dtp->dt_globals, (dt_idhash_f *)dt_copyvar, dtp);
>   		dt_idhash_iter(pcb->pcb_locals, (dt_idhash_f *)dt_copyvar, dtp);
>   	}
> diff --git a/libdtrace/dt_dis.c b/libdtrace/dt_dis.c
> index 342db62b..b10c3867 100644
> --- a/libdtrace/dt_dis.c
> +++ b/libdtrace/dt_dis.c
> @@ -663,6 +663,9 @@ dt_dis_difo(const dtrace_difo_t *dp, FILE *fp)
>   		char kind[4], scope[4], range[12], flags[16] = { 0 };
>   
>   		switch (v->dtdv_kind) {
> +		case DIFV_KIND_AGGREGATE:
> +			strcpy(kind, "agg");
> +			break;
>   		case DIFV_KIND_ARRAY:
>   			strcpy(kind, "arr");
>   			break;

I realize that this is not yet sufficient to generate disassembly for 
aggregations, but I'm just wondering what tests we have for this.  
Actually, hang on, do I not know where to look, or is it the case that 
we have absolutely no tests of any kind for disassembly? That would seem 
to be a big omission.