Linux DTrace development list
 help / color / mirror / Atom feed
* [PATCH] trace: fix char-array handling
@ 2025-02-06 16:46 Kris Van Hees
  2025-02-07  6:04 ` Eugene Loh
  0 siblings, 1 reply; 3+ messages in thread
From: Kris Van Hees @ 2025-02-06 16:46 UTC (permalink / raw)
  To: dtrace, dtrace-devel

The special handling of strings in the consumer's trace implementation
did not correctly account for char-arrays that do not necessarily are
intended to hold string data.  Thr trace action is documented to print
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.

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@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; }
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] trace: fix char-array handling
  2025-02-06 16:46 [PATCH] trace: fix char-array handling Kris Van Hees
@ 2025-02-07  6:04 ` Eugene Loh
  2025-02-07  6:08   ` Kris Van Hees
  0 siblings, 1 reply; 3+ messages in thread
From: Eugene Loh @ 2025-02-07  6:04 UTC (permalink / raw)
  To: Kris Van Hees, dtrace, dtrace-devel

I don't understand the relevant code very well, but
Reviewed-by: Eugene Loh <eugene.loh@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@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; }

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] trace: fix char-array handling
  2025-02-07  6:04 ` Eugene Loh
@ 2025-02-07  6:08   ` Kris Van Hees
  0 siblings, 0 replies; 3+ messages in thread
From: Kris Van Hees @ 2025-02-07  6:08 UTC (permalink / raw)
  To: Eugene Loh; +Cc: Kris Van Hees, dtrace, dtrace-devel

On Fri, Feb 07, 2025 at 01:04:53AM -0500, Eugene Loh wrote:
> I don't understand the relevant code very well, but
> Reviewed-by: Eugene Loh <eugene.loh@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.

I'll have a look.  Copyright years may need to be updated.

> 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.

'a' 'b' 0 'c' 0 will indeed by bumped as raw bytes because it is not a string
followed by one or more 0-bytes, but rather a potential string that has a
non-printable character (byte) embedded in it, which causes it to not be seen
as a string at all.

> 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@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; }

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-02-07  6:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-06 16:46 [PATCH] trace: fix char-array handling Kris Van Hees
2025-02-07  6:04 ` Eugene Loh
2025-02-07  6:08   ` Kris Van Hees

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox