[DTrace-devel] [PATCH] Add support for array/struct/union to trace()

[Eugene Loh]

On 1/26/22 11:26 AM, Kris Van Hees via DTrace-devel wrote:

> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> @@ -1002,6 +1002,32 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
>   		return 0;
>   	}
>   
> +	/* Handle tracing of by-ref values (arrays, struct, union). */
> +	if (kind == DTRACEACT_DIFEXPR && (arg & DT_NF_REF)) {
> +		off = dt_rec_add(dtp, dt_cg_fill_gap, kind,
> +				 size, size, pfp, arg);
> +
> +		TRACE_REGSET("store_val(): Begin ");
> +		dt_cg_check_notnull(dlp, drp, dnp->dn_reg);
> +
> +		if (dt_regset_xalloc_args(drp) == -1)
> +			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> +
> +		emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_9));
> +		emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, off));
> +		emit(dlp, BPF_MOV_IMM(BPF_REG_2, size));
> +		emit(dlp, BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
> +		dt_regset_free(drp, dnp->dn_reg);
> +		dt_regset_xalloc(drp, BPF_REG_0);
> +		emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
> +		dt_regset_free_args(drp);
> +
> +		dt_regset_free(drp, BPF_REG_0);
> +		TRACE_REGSET("store_val(): End   ");
> +
> +		return 0;
> +	}
> +
>   	return -1;
>   }

For dt_rec_add(), why is the alignment "size" rather than 1?

dt_cg_store_val() handles different cases (USYM, scalar, string, and now 
by-ref), in each case ending with "return 0" -- or if no such case then 
"return -1".  There are two styles for expressing this:

     if (...) {
         [...]
         return 0;
     }
     if (...) {
         [...]
         return 0;
     }
     if (...) {
         [...]
         return 0;
     }
     return -1;

or

     if (...) {
         [...]
         return 0;
     }
     else if (...) {
         [...]
         return 0;
     }
     else if (...) {
         [...]
         return 0;
     }
     return -1;

Right now, we use both styles.  I think it makes sense to use one, 
consistent style (I prefer the former), and I think it is reasonable in 
this patch to fix the pre-existing inconsistency.

Regarding the tests...

We need to check the output of the new tests.  For struct and union, 
this means .r files.  For array, we do not care about differences past 
the first NUL char...  so this may need a tst.array.r.p file as well.

The output of tst.union.d looks wrong.  If I try
     # dtrace -qs test/unittest/actions/trace/tst.union.d
with Solaris and DTv1 I get:
       xV4
while with DTv2 I get:
                  0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f 
0123456789abcdef
              0: 78 56 34                                         xV4

> diff --git a/test/unittest/actions/trace/tst.array.d b/test/unittest/actions/trace/tst.array.d
> new file mode 100644
> index 00000000..104d42e1
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array.d
> @@ -0,0 +1,18 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * ASSERTION: The trace() action prints an array correctly.
> + *
> + * SECTION: Actions and Subroutines/trace()
> + */
> +
> +BEGIN
> +{
> +	trace(curthread->comm);
> +	exit(0);
> +}
> diff --git a/test/unittest/actions/trace/tst.struct.d b/test/unittest/actions/trace/tst.struct.d
> new file mode 100644
> index 00000000..ec60ea11
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.struct.d
> @@ -0,0 +1,27 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * ASSERTION: The trace() action prints a struct correctly.
> + *
> + * SECTION: Actions and Subroutines/trace()
> + */
> +
> +struct {
> +	int	a;
> +	char	b;
> +	int	c;
> +} st;
> +
> +BEGIN
> +{
> +	st.a = 0x1234;
> +	st.b = 0;
> +	st.c = 0x9876;
> +	trace(st);
> +	exit(0);
> +}
> diff --git a/test/unittest/actions/trace/tst.union.d b/test/unittest/actions/trace/tst.union.d
> new file mode 100644
> index 00000000..b9d107d3
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.union.d
> @@ -0,0 +1,25 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * ASSERTION: The trace() action prints a union correctly.
> + *
> + * SECTION: Actions and Subroutines/trace()
> + */
> +
> +union {
> +	int	a;
> +	char	b[3];
> +	short	c;
> +} u;
> +
> +BEGIN
> +{
> +	u.a = 0x12345678;
> +	trace(u.b);
> +	exit(0);
> +}