[DTrace-devel] [PATCH] trace: fix char-array handling
Eugene Loh
eugene.loh at oracle.com
Fri Feb 7 06:04:53 UTC 2025
I don't understand the relevant code very well, but
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
Incidentally, it looks funny to have so many "new" files have 2022
copyrights on them, but I suppose they're inheriting a lot from
pre-existing tests... so, okay.
Also...
On 2/6/25 11:46, Kris Van Hees wrote:
> The special handling of strings in the consumer's trace implementation
> did not correctly account for char-arrays that do not necessarily are
s/do not necessarily are/are not necessarily/
> intended to hold string data. Thr trace action is documented to print
s/Thr/The/
> data as an ASCII string if it can be represented as such, i.e. if it
> comprises only printable characters, optionally terminated by one or
> more 0-bytes.
That doesn't read quite right to me. E.g., 'a' 'b' 0 'c' 0 looks to me
like "only printable chars, terminated by one or more 0 bytes" yet I
think it's supposed to be dumped (not printed as a string). I'm not sure
and maybe it doesn't matter.
Another possible test is all bytes are 0, but I'm not worried about
whether such a test is included.
> The implementation for trace on both producer and consumer side was
> using the data alignment as a marker to determine whether data should
> be printed as a string (alignment == 1) or not. The real alignment
> of the data should be used, reverting code back to the necessary
> heuristics in dt_print_bytes().
>
> The dtrace_diftype struct has its dtdt_pad member renamed as dtdt_align.
>
> Added several new test cases to cover various possibilities.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> include/dtrace/dif.h | 2 +-
> libdtrace/dt_cg.c | 16 ++++++----
> libdtrace/dt_consume.c | 20 ++-----------
> libdtrace/dt_parser.c | 2 +-
> .../actions/trace/tst.array-char-multi-nul.d | 30 +++++++++++++++++++
> .../actions/trace/tst.array-char-multi-nul.r | 8 +++++
> .../trace/tst.array-char-str-multi-nul.d | 30 +++++++++++++++++++
> .../trace/tst.array-char-str-multi-nul.r | 5 ++++
> .../actions/trace/tst.array-char-str-no-nul.d | 30 +++++++++++++++++++
> .../actions/trace/tst.array-char-str-no-nul.r | 5 ++++
> .../actions/trace/tst.array-char-str.d | 30 +++++++++++++++++++
> .../actions/trace/tst.array-char-str.r | 5 ++++
> .../trace/tst.array-char-unprintable.d | 30 +++++++++++++++++++
> .../trace/tst.array-char-unprintable.r | 8 +++++
> test/unittest/actions/trace/tst.array.d | 11 +++++--
> test/unittest/actions/trace/tst.array.r | 2 +-
> test/unittest/actions/trace/tst.array.r.p | 6 ----
> 17 files changed, 206 insertions(+), 34 deletions(-)
> create mode 100644 test/unittest/actions/trace/tst.array-char-multi-nul.d
> create mode 100644 test/unittest/actions/trace/tst.array-char-multi-nul.r
> create mode 100644 test/unittest/actions/trace/tst.array-char-str-multi-nul.d
> create mode 100644 test/unittest/actions/trace/tst.array-char-str-multi-nul.r
> create mode 100644 test/unittest/actions/trace/tst.array-char-str-no-nul.d
> create mode 100644 test/unittest/actions/trace/tst.array-char-str-no-nul.r
> create mode 100644 test/unittest/actions/trace/tst.array-char-str.d
> create mode 100644 test/unittest/actions/trace/tst.array-char-str.r
> create mode 100644 test/unittest/actions/trace/tst.array-char-unprintable.d
> create mode 100644 test/unittest/actions/trace/tst.array-char-unprintable.r
> delete mode 100755 test/unittest/actions/trace/tst.array.r.p
>
> diff --git a/include/dtrace/dif.h b/include/dtrace/dif.h
> index 576bda2c..70257b2f 100644
> --- a/include/dtrace/dif.h
> +++ b/include/dtrace/dif.h
> @@ -33,7 +33,7 @@ typedef struct dtrace_diftype {
> uint8_t dtdt_kind;
> uint8_t dtdt_ckind;
> uint8_t dtdt_flags;
> - uint8_t dtdt_pad;
> + uint8_t dtdt_align;
> uint32_t dtdt_size;
> } dtrace_diftype_t;
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 6e74b4b0..a5c9aa09 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1601,12 +1601,11 @@ static int
> dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
> dt_pfargv_t *pfp, int arg)
> {
> - dtrace_diftype_t vtype;
> dtrace_hdl_t *dtp = pcb->pcb_hdl;
> dt_irlist_t *dlp = &pcb->pcb_ir;
> dt_regset_t *drp = pcb->pcb_regs;
> uint_t off;
> - size_t size;
> + size_t size, align;
> int not_null = 0;
> int cflags = pcb->pcb_stmt->dtsd_clauseflags;
>
> @@ -1619,10 +1618,14 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
> longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> emit(dlp, BPF_MOV_IMM(dnp->dn_reg, dnp->dn_ident->di_id));
> size = sizeof(dnp->dn_ident->di_id);
> + align = size;
> } else {
> + dtrace_diftype_t vtype;
> +
> dt_cg_node(dnp, dlp, drp);
> dt_node_diftype(dtp, dnp, &vtype);
> size = vtype.dtdt_size;
> + align = vtype.dtdt_align;
>
> /*
> * A DEREF of a REF node does not get resolved in dt_cg_node()
> @@ -1664,7 +1667,7 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
>
> if (dt_node_is_scalar(dnp) || dt_node_is_float(dnp) ||
> dnp->dn_kind == DT_NODE_AGG) {
> - off = dt_rec_add(dtp, dt_cg_fill_gap, kind, size, size, pfp,
> + off = dt_rec_add(dtp, dt_cg_fill_gap, kind, size, align, pfp,
> arg);
>
> emit(dlp, BPF_STOREX(size, BPF_REG_9, off, dnp->dn_reg));
> @@ -1678,8 +1681,8 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
> dt_cg_check_ptr_arg(dlp, drp, dnp, NULL);
>
> TRACE_REGSET("store_val(): Begin ");
> - off = dt_rec_add(dtp, dt_cg_fill_gap, kind, size + 1,
> - 1, pfp, arg);
> + off = dt_rec_add(dtp, dt_cg_fill_gap, kind, size + 1, 1, pfp,
> + arg);
>
> /*
> * Copy the string data (no more than STRSIZE + 1 bytes) to the
> @@ -1706,7 +1709,8 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
>
> /* Handle tracing of by-ref values (arrays, struct, union). */
> if ((dnp->dn_flags & DT_NF_REF) || (arg & DT_NF_REF)) {
> - off = dt_rec_add(dtp, dt_cg_fill_gap, kind, size, 2, pfp, arg);
> + off = dt_rec_add(dtp, dt_cg_fill_gap, kind, size, align, pfp,
> + arg);
>
> TRACE_REGSET("store_val(): Begin ");
> if (!not_null)
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index 74004c69..58a2ead9 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -1921,23 +1921,9 @@ dt_print_trace(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
> if (dtp->dt_options[DTRACEOPT_RAWBYTES] != DTRACEOPT_UNSET)
> return dt_print_rawbytes(dtp, fp, data, rec->dtrd_size);
>
> - /*
> - * String data can be recognized as a non-scalar data item with
> - * alignment == 1.
> - * Any other non-scalar data items are printed as a byte stream.
> - */
> - if (rec->dtrd_arg == DT_NF_REF) {
> - char *s = (char *)data;
> -
> - if (rec->dtrd_alignment > 1)
> - return dt_print_rawbytes(dtp, fp, data, rec->dtrd_size);
> -
> - /* We have a string. Print it. */
> - if (quiet)
> - return dt_printf(dtp, fp, "%s", s);
> - else
> - return dt_printf(dtp, fp, " %-33s", s);
> - }
> + /* Handle non-scalar data. */
> + if (rec->dtrd_arg == DT_NF_REF)
> + return dt_print_bytes(dtp, fp, data, rec->dtrd_size, 33, quiet);
>
> /*
> * Differentiate between signed and unsigned numeric values.
> diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
> index 325ba881..eefe8341 100644
> --- a/libdtrace/dt_parser.c
> +++ b/libdtrace/dt_parser.c
> @@ -4914,7 +4914,7 @@ dt_node_diftype(dtrace_hdl_t *dtp, const dt_node_t *dnp, dtrace_diftype_t *tp)
> }
>
> tp->dtdt_flags = (dnp->dn_flags & DT_NF_REF) ? DIF_TF_BYREF : 0;
> - tp->dtdt_pad = 0;
> + tp->dtdt_align = ctf_type_align(dnp->dn_ctfp, dnp->dn_type);
> tp->dtdt_size = ctf_type_size(dnp->dn_ctfp, dnp->dn_type);
> }
>
> diff --git a/test/unittest/actions/trace/tst.array-char-multi-nul.d b/test/unittest/actions/trace/tst.array-char-multi-nul.d
> new file mode 100644
> index 00000000..187fb711
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-multi-nul.d
> @@ -0,0 +1,30 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, 2025, 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 char-array of printable characters
> + * with multiple 0-bytes in its content.
> + *
> + * SECTION: Actions and Subroutines/trace()
> + */
> +
> +char n[9];
> +
> +BEGIN
> +{
> + n[0] = 'a';
> + n[1] = 'A';
> + n[2] = 0;
> + n[3] = 'B';
> + n[4] = 0;
> + n[5] = 'C';
> + n[6] = 0;
> + n[7] = 'D';
> + n[8] = 'e';
> + trace(n);
> + exit(0);
> +}
> diff --git a/test/unittest/actions/trace/tst.array-char-multi-nul.r b/test/unittest/actions/trace/tst.array-char-multi-nul.r
> new file mode 100644
> index 00000000..c1361e75
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-multi-nul.r
> @@ -0,0 +1,8 @@
> + FUNCTION:NAME
> + :BEGIN
> + 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
> + 0: 61 41 00 42 00 43 00 44 65 aA.B.C.De
> +
> +
> +-- @@stderr --
> +dtrace: script 'test/unittest/actions/trace/tst.array-char-multi-nul.d' matched 1 probe
> diff --git a/test/unittest/actions/trace/tst.array-char-str-multi-nul.d b/test/unittest/actions/trace/tst.array-char-str-multi-nul.d
> new file mode 100644
> index 00000000..75164cd3
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-str-multi-nul.d
> @@ -0,0 +1,30 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, 2025, 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 char-array of printable characters
> + * followed y by multiple 0-bytes correctly.
> + *
> + * SECTION: Actions and Subroutines/trace()
> + */
> +
> +char n[9];
> +
> +BEGIN
> +{
> + n[0] = 'a';
> + n[1] = 'A';
> + n[2] = 'b';
> + n[3] = 'B';
> + n[4] = 'c';
> + n[5] = 0;
> + n[6] = 0;
> + n[7] = 0;
> + n[8] = 0;
> + trace(n);
> + exit(0);
> +}
> diff --git a/test/unittest/actions/trace/tst.array-char-str-multi-nul.r b/test/unittest/actions/trace/tst.array-char-str-multi-nul.r
> new file mode 100644
> index 00000000..1951ae69
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-str-multi-nul.r
> @@ -0,0 +1,5 @@
> + FUNCTION:NAME
> + :BEGIN aAbBc
> +
> +-- @@stderr --
> +dtrace: script 'test/unittest/actions/trace/tst.array-char-str-multi-nul.d' matched 1 probe
> diff --git a/test/unittest/actions/trace/tst.array-char-str-no-nul.d b/test/unittest/actions/trace/tst.array-char-str-no-nul.d
> new file mode 100644
> index 00000000..532c3519
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-str-no-nul.d
> @@ -0,0 +1,30 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, 2025, 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 char-aarray of printable characters
> + * (not terminated) correctly.
> + *
> + * SECTION: Actions and Subroutines/trace()
> + */
> +
> +char n[9];
> +
> +BEGIN
> +{
> + n[0] = 'a';
> + n[1] = 'A';
> + n[2] = 'b';
> + n[3] = 'B';
> + n[4] = 'c';
> + n[5] = 'C';
> + n[6] = 'd';
> + n[7] = 'D';
> + n[8] = 'e';
> + trace(n);
> + exit(0);
> +}
> diff --git a/test/unittest/actions/trace/tst.array-char-str-no-nul.r b/test/unittest/actions/trace/tst.array-char-str-no-nul.r
> new file mode 100644
> index 00000000..3d56e520
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-str-no-nul.r
> @@ -0,0 +1,5 @@
> + FUNCTION:NAME
> + :BEGIN aAbBcCdDe
> +
> +-- @@stderr --
> +dtrace: script 'test/unittest/actions/trace/tst.array-char-str-no-nul.d' matched 1 probe
> diff --git a/test/unittest/actions/trace/tst.array-char-str.d b/test/unittest/actions/trace/tst.array-char-str.d
> new file mode 100644
> index 00000000..70f6371f
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-str.d
> @@ -0,0 +1,30 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, 2025, 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 char-array of printable characters
> + * (terminated) correctly.
> + *
> + * SECTION: Actions and Subroutines/trace()
> + */
> +
> +char n[9];
> +
> +BEGIN
> +{
> + n[0] = 'a';
> + n[1] = 'A';
> + n[2] = 'b';
> + n[3] = 'B';
> + n[4] = 'c';
> + n[5] = 'C';
> + n[6] = 'd';
> + n[7] = 'D';
> + n[8] = '\0';
> + trace(n);
> + exit(0);
> +}
> diff --git a/test/unittest/actions/trace/tst.array-char-str.r b/test/unittest/actions/trace/tst.array-char-str.r
> new file mode 100644
> index 00000000..9778ddd2
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-str.r
> @@ -0,0 +1,5 @@
> + FUNCTION:NAME
> + :BEGIN aAbBcCdD
> +
> +-- @@stderr --
> +dtrace: script 'test/unittest/actions/trace/tst.array-char-str.d' matched 1 probe
> diff --git a/test/unittest/actions/trace/tst.array-char-unprintable.d b/test/unittest/actions/trace/tst.array-char-unprintable.d
> new file mode 100644
> index 00000000..d88df369
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-unprintable.d
> @@ -0,0 +1,30 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, 2025, 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 char-array with a non-printable
> + * character correctly.
> + *
> + * SECTION: Actions and Subroutines/trace()
> + */
> +
> +char n[9];
> +
> +BEGIN
> +{
> + n[0] = 'a';
> + n[1] = 'A';
> + n[2] = 'b';
> + n[3] = 'B';
> + n[4] = 'c';
> + n[5] = 5;
> + n[6] = 'd';
> + n[7] = 'D';
> + n[8] = '\0';
> + trace(n);
> + exit(0);
> +}
> diff --git a/test/unittest/actions/trace/tst.array-char-unprintable.r b/test/unittest/actions/trace/tst.array-char-unprintable.r
> new file mode 100644
> index 00000000..1f84fcfa
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.array-char-unprintable.r
> @@ -0,0 +1,8 @@
> + FUNCTION:NAME
> + :BEGIN
> + 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
> + 0: 61 41 62 42 63 05 64 44 00 aAbBc.dD.
> +
> +
> +-- @@stderr --
> +dtrace: script 'test/unittest/actions/trace/tst.array-char-unprintable.d' matched 1 probe
> diff --git a/test/unittest/actions/trace/tst.array.d b/test/unittest/actions/trace/tst.array.d
> index 104d42e1..0779d90f 100644
> --- a/test/unittest/actions/trace/tst.array.d
> +++ b/test/unittest/actions/trace/tst.array.d
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2022, 2025, 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.
> */
> @@ -11,8 +11,15 @@
> * SECTION: Actions and Subroutines/trace()
> */
>
> +short n[5];
> +
> BEGIN
> {
> - trace(curthread->comm);
> + n[0] = 0x1234;
> + n[1] = 0x5678;
> + n[2] = 0x0000;
> + n[3] = 0x8765;
> + n[4] = 0x4321;
> + trace(n);
> exit(0);
> }
> diff --git a/test/unittest/actions/trace/tst.array.r b/test/unittest/actions/trace/tst.array.r
> index fbb674b6..52ff28ec 100644
> --- a/test/unittest/actions/trace/tst.array.r
> +++ b/test/unittest/actions/trace/tst.array.r
> @@ -1,7 +1,7 @@
> FUNCTION:NAME
> :BEGIN
> 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
> - 0: 64 74 72 61 63 65 00 00 00 00 00 00 00 00 00 00 dtrace..........
> + 0: 34 12 78 56 00 00 65 87 21 43 4.xV..e.!C
>
>
> -- @@stderr --
> diff --git a/test/unittest/actions/trace/tst.array.r.p b/test/unittest/actions/trace/tst.array.r.p
> deleted file mode 100755
> index b8cc8daf..00000000
> --- a/test/unittest/actions/trace/tst.array.r.p
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -#!/usr/bin/gawk -f
> -
> -# Some Linux kernel versions leave garbage at the end of the string.
> -{ sub(/( [0-9A-F]{2}){9} /, " 00 00 00 00 00 00 00 00 00 "); }
> -{ sub(/ dtrace\..{9}/, " dtrace.........."); }
> -{ print; }
More information about the DTrace-devel
mailing list