[DTrace-devel] [PATCH 3/5] Fix size for typecast tuple (and aggregation key) components

[Eugene Loh]

I think of this fix slightly differently.  The point is not that "size" 
needs to be updated;  rather, it was never computed correctly in the 
first place.  With this patch, "size" is computed once (incorrectly, but 
never really used) and then overwritten with the correct value.  It 
would be cleaner just to compute size correctly in the first place.  
Instead of inserting the new correction, just go up a dozen lines and:

-               size = t.dtdt_size;
+               size = dt_node_sizeof(&isp->dis_args[i]);

Also, the patch should have a test.  Here is a suggestion:

BEGIN {
   @a[11111111, 22222222,        0x7fff7fff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +1);
   @a[11111111, 22222222, (int  )0x7fff7fff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +2);
   @a[11111111, 22222222, (short)0x7fff7fff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +4);
   @a[11111111, 22222222, (long )0x7fff7fff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +8);
   @a[11111111, 22222222,        0x7fffffff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +16);
   @a[11111111, 22222222, (int  )0x7fffffff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +32);
   @a[11111111, 22222222, (short)0x7fffffff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +64);
   @a[11111111, 22222222, (long )0x7fffffff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +128);
   @a[11111111, 22222222,        0xffff7fff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +256);
   @a[11111111, 22222222, (int  )0xffff7fff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +512);
   @a[11111111, 22222222, (short)0xffff7fff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +1024);
   @a[11111111, 22222222, (long )0xffff7fff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +2048);
   @a[11111111, 22222222,        0xffffffff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +4096);
   @a[11111111, 22222222, (int  )0xffffffff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +8192);
   @a[11111111, 22222222, (short)0xffffffff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +16384);
   @a[11111111, 22222222, (long )0xffffffff, 44444444, 55555555] = 
lquantize(3, 0, 10, 1, +32768);
   exit(0);
}

On 9/6/22 21:36, Kris Van Hees via DTrace-devel wrote:
> Components of a tuple (or aggregation key) were being typecast where
> needed but if narrowing or widening took place, the storage size to store
> the value was never updated.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_cg.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index ffab2443..fae32516 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2765,6 +2765,9 @@ empty_args:
>   		dt_cg_typecast(dnp, &isp->dis_args[i], dlp, drp);
>   		isp->dis_args[i].dn_reg = -1;
>   
> +		/* The typecast may have changed the size. */
> +		size = dt_node_sizeof(&isp->dis_args[i]);
> +
>   		if (dt_node_is_scalar(dnp) || dt_node_is_float(dnp)) {
>   			assert(size > 0 && size <= 8 &&
>   			       (size & (size - 1)) == 0);