[PATCH v5 1/2] print() action: identify ctf object used for print

[PATCH v5 1/2] print() action: identify ctf object used for print

[Alan Maguire]

when generating code for print() action we need to identify source
of CTF; is it shared CTF, cdefs or ddefs or another module?
Rework the print() action to be a printf()like function under the
hood using the format specifier as module name.  We can then
do away with dt_print_print(), calling dt_print_type() directly
from consume.

Suggested-by: Kris Van Hees <kris.van.hees@oracle.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 libdtrace/dt_cg.c      | 10 ++++++++--
 libdtrace/dt_consume.c | 28 +++-------------------------
 libdtrace/dt_impl.h    |  1 +
 libdtrace/dt_printf.c  | 28 ++++++++++++++++++++++++----
 libdtrace/dt_printf.h  |  6 ++++--
 5 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index a1c24e37..164f9e5f 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -20,6 +20,7 @@
 #include <dt_dctx.h>
 #include <dt_cg.h>
 #include <dt_grammar.h>
+#include <dt_module.h>
 #include <dt_parser.h>
 #include <dt_printf.h>
 #include <dt_provider.h>
@@ -2838,8 +2839,11 @@ dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
 	ctf_file_t	*fp = addr->dn_ctfp;
 	ctf_id_t	type = addr->dn_type;
 	char		n[DT_TYPE_NAMELEN];
+	dt_module_t	*dmp;
+	dt_pfargv_t	*pfp;
 	size_t		size;
 
+	dmp = dt_module_lookup_by_ctf(dtp, fp);
 	type = ctf_type_reference(fp, type);
 	if (type == CTF_ERR)
 		longjmp(yypcb->pcb_jmpbuf, EDT_CTF);
@@ -2849,11 +2853,13 @@ dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
 			"print( ) argument #1 reference has type '%s' with size 0; cannot print( ) it.\n",
 			ctf_type_name(fp, type, n, sizeof(n)));
 
+	pfp = dt_printf_create(dtp, dmp->dm_name);
+
 	/* reserve space for addr/type, data/size */
 	addr_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
-			      sizeof(uint64_t), 8, NULL, type);
+			      sizeof(uint64_t), 8, pfp, type);
 	data_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
-			      size, 8, NULL, size);
+			      size, 8, NULL, 0);
 
 	dt_cg_node(addr, &pcb->pcb_ir, drp);
 	dt_cg_check_ptr_arg(dlp, drp, addr, NULL);
diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
index e9b33cfc..07856746 100644
--- a/libdtrace/dt_consume.c
+++ b/libdtrace/dt_consume.c
@@ -507,7 +507,7 @@ dt_nullrec()
 	return DTRACE_CONSUME_NEXT;
 }
 
-static int
+int
 dt_read_scalar(caddr_t addr, const dtrace_recdesc_t *rec, uint64_t *valp)
 {
 	addr += rec->dtrd_offset;
@@ -1983,25 +1983,6 @@ dt_print_trace(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
 	return dt_print_rawbytes(dtp, fp, data, rec->dtrd_size);
 }
 
-static int
-dt_print_print(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
-	       const caddr_t buf)
-{
-	dtrace_recdesc_t *data_rec = rec + 1;
-	size_t max_size = dtp->dt_options[DTRACEOPT_PRINTSIZE];
-	size_t size = (size_t)data_rec->dtrd_arg;
-	uint64_t printaddr;
-
-	if (size > max_size)
-		size = max_size;
-
-	if (dt_read_scalar(buf, rec, &printaddr) < 0)
-		return dt_set_errno(dtp, EDT_PRINT);
-
-	return dt_print_type(dtp, fp, printaddr, (ctf_id_t)rec->dtrd_arg,
-			     (caddr_t)buf + data_rec->dtrd_offset, size);
-}
-
 /*
  * The lifecycle of speculation buffers is as follows:
  *
@@ -2562,11 +2543,8 @@ dt_consume_one_probe(dtrace_hdl_t *dtp, FILE *fp, char *data, uint32_t size,
 			func = dtrace_freopen;
 			break;
 		case DTRACEACT_PRINT:
-			n = dt_print_print(dtp, fp, rec, data);
-			if (n < 0)
-				return -1;
-			i += n - 1;
-			continue;
+			func = dt_print_type;
+			break;
 		default:
 			break;
 		}
diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
index 01313ff3..88b31ce0 100644
--- a/libdtrace/dt_impl.h
+++ b/libdtrace/dt_impl.h
@@ -789,6 +789,7 @@ extern int dt_buffered_flush(dtrace_hdl_t *, dtrace_probedata_t *,
     const dtrace_recdesc_t *, const dtrace_aggdata_t *, uint32_t flags);
 extern void dt_buffered_disable(dtrace_hdl_t *);
 extern void dt_buffered_destroy(dtrace_hdl_t *);
+extern int dt_read_scalar(caddr_t, const dtrace_recdesc_t *, uint64_t *);
 
 extern uint64_t dt_stddev(uint64_t *, uint64_t);
 
diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
index 50842216..e999498c 100644
--- a/libdtrace/dt_printf.c
+++ b/libdtrace/dt_printf.c
@@ -14,6 +14,7 @@
 #include <limits.h>
 #include <port.h>
 
+#include <dt_module.h>
 #include <dt_printf.h>
 #include <dt_string.h>
 #include <dt_impl.h>
@@ -2235,15 +2236,34 @@ err:
 }
 
 int
-dt_print_type(dtrace_hdl_t *dtp, FILE *fp, uint64_t printaddr,
-	      ctf_id_t type, caddr_t data, size_t size)
+dt_print_type(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
+	      const dtrace_probedata_t *data, const dtrace_recdesc_t *recs,
+	      uint_t nrecs, const void *buf, size_t len)
 {
+	const char *modname = ((dt_pfargv_t *)fmtdata)->pfv_format;
+	size_t max_size = dtp->dt_options[DTRACEOPT_PRINTSIZE];
+	ctf_id_t type = (ctf_id_t)recs->dtrd_arg;
+	const dtrace_recdesc_t *data_rec = recs + 1;
+	size_t size = data_rec->dtrd_size;
+	caddr_t addr_data = (caddr_t)buf;
 	struct dt_visit_arg dva;
+	uint64_t printaddr;
+	dt_module_t *dmp;
 
+	if (size > max_size)
+		size = max_size;
+
+	if (dt_read_scalar(addr_data, recs, &printaddr) < 0)
+		return dt_set_errno(dtp, EDT_PRINT);
+
+	dmp = dt_module_lookup_by_name(dtp, modname);
+	if (!dmp) {
+		return dt_set_errno(dtp, EDT_PRINT);
+	}
 	dva.dv_dtp = dtp;
 	dva.dv_fp = fp;
-	dva.dv_ctfp = dtp->dt_ddefs->dm_ctfp;
-	dva.dv_data = data;
+	dva.dv_ctfp = dmp->dm_ctfp;
+	dva.dv_data = addr_data + data_rec->dtrd_offset;
 	dva.dv_size = size;
 	dva.dv_last_depth = 0;
 	dva.dv_startindent = DT_PRINT_STARTINDENT;
diff --git a/libdtrace/dt_printf.h b/libdtrace/dt_printf.h
index 9771c4a8..4f4386a5 100644
--- a/libdtrace/dt_printf.h
+++ b/libdtrace/dt_printf.h
@@ -108,8 +108,10 @@ extern int dt_print_ustack(dtrace_hdl_t *, FILE *,
     const char *, caddr_t, uint64_t);
 extern int dt_print_mod(dtrace_hdl_t *, FILE *, const char *, caddr_t);
 extern int dt_print_umod(dtrace_hdl_t *, FILE *, const char *, caddr_t);
-extern int dt_print_type(dtrace_hdl_t *, FILE *, uint64_t, ctf_id_t,
-			 caddr_t, size_t);
+extern int dt_print_type(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
+			 const dtrace_probedata_t *data,
+			 const dtrace_recdesc_t *recs,
+			 uint_t nrecs, const void *buf, size_t len);
 
 #ifdef	__cplusplus
 }
-- 
2.43.5

[PATCH v5 2/2] unittest/print: add test covering module-defined type

[Alan Maguire]

print() action tests do not cover module-defined types.  Add a test
using a module-defined type that has not changed recently.

struct tun_page was chosen since it is identical in UEK6U3 and upstream
and declared in tun.c which is a module on most distros.

Added tun to test/modules as suggested by Eugene.

Suggested-by: Nick Alcock <nick.alcock@oracle.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
---
 test/modules                            |  1 +
 test/unittest/print/tst.print.modtype.d | 18 ++++++++++++++++++
 test/unittest/print/tst.print.modtype.r |  6 ++++++
 3 files changed, 25 insertions(+)
 create mode 100644 test/unittest/print/tst.print.modtype.d
 create mode 100644 test/unittest/print/tst.print.modtype.r

diff --git a/test/modules b/test/modules
index 53dc5544..0f01d6e0 100644
--- a/test/modules
+++ b/test/modules
@@ -2,3 +2,4 @@ ext4
 isofs
 nfs
 rds
+tun
diff --git a/test/unittest/print/tst.print.modtype.d b/test/unittest/print/tst.print.modtype.d
new file mode 100644
index 00000000..ed68b492
--- /dev/null
+++ b/test/unittest/print/tst.print.modtype.d
@@ -0,0 +1,18 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2024, 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.
+ */
+/* @@nosort */
+
+#pragma D option quiet
+
+BEGIN
+{
+	tun_page = (struct tun_page *)alloca(sizeof (struct tun_page));
+	tun_page->page = (struct page *)0xfeedfacefeedface;
+	tun_page->count = 123;
+	print(tun_page);
+	exit(0);
+}
diff --git a/test/unittest/print/tst.print.modtype.r b/test/unittest/print/tst.print.modtype.r
new file mode 100644
index 00000000..b9736607
--- /dev/null
+++ b/test/unittest/print/tst.print.modtype.r
@@ -0,0 +1,6 @@
+{ptr} = *
+                                            (struct tun_page) {
+                                             .page = (struct page *){ptr},
+                                             .count = (int)123,
+                                            }
+
-- 
2.43.5

Re: [PATCH v5 1/2] print() action: identify ctf object used for print

[Kris Van Hees]

On Thu, Aug 01, 2024 at 10:29:11PM +0100, Alan Maguire wrote:
> when generating code for print() action we need to identify source
> of CTF; is it shared CTF, cdefs or ddefs or another module?
> Rework the print() action to be a printf()like function under the
> hood using the format specifier as module name.  We can then
> do away with dt_print_print(), calling dt_print_type() directly
> from consume.
> 
> Suggested-by: Kris Van Hees <kris.van.hees@oracle.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  libdtrace/dt_cg.c      | 10 ++++++++--
>  libdtrace/dt_consume.c | 28 +++-------------------------
>  libdtrace/dt_impl.h    |  1 +
>  libdtrace/dt_printf.c  | 28 ++++++++++++++++++++++++----
>  libdtrace/dt_printf.h  |  6 ++++--
>  5 files changed, 40 insertions(+), 33 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index a1c24e37..164f9e5f 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -20,6 +20,7 @@
>  #include <dt_dctx.h>
>  #include <dt_cg.h>
>  #include <dt_grammar.h>
> +#include <dt_module.h>
>  #include <dt_parser.h>
>  #include <dt_printf.h>
>  #include <dt_provider.h>
> @@ -2838,8 +2839,11 @@ dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>  	ctf_file_t	*fp = addr->dn_ctfp;
>  	ctf_id_t	type = addr->dn_type;
>  	char		n[DT_TYPE_NAMELEN];
> +	dt_module_t	*dmp;
> +	dt_pfargv_t	*pfp;
>  	size_t		size;
>  
> +	dmp = dt_module_lookup_by_ctf(dtp, fp);
>  	type = ctf_type_reference(fp, type);
>  	if (type == CTF_ERR)
>  		longjmp(yypcb->pcb_jmpbuf, EDT_CTF);
> @@ -2849,11 +2853,13 @@ dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>  			"print( ) argument #1 reference has type '%s' with size 0; cannot print( ) it.\n",
>  			ctf_type_name(fp, type, n, sizeof(n)));
>  
> +	pfp = dt_printf_create(dtp, dmp->dm_name);
> +
>  	/* reserve space for addr/type, data/size */
>  	addr_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> -			      sizeof(uint64_t), 8, NULL, type);
> +			      sizeof(uint64_t), 8, pfp, type);
>  	data_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> -			      size, 8, NULL, size);
> +			      size, 8, NULL, 0);
>  
>  	dt_cg_node(addr, &pcb->pcb_ir, drp);
>  	dt_cg_check_ptr_arg(dlp, drp, addr, NULL);
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index e9b33cfc..07856746 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -507,7 +507,7 @@ dt_nullrec()
>  	return DTRACE_CONSUME_NEXT;
>  }
>  
> -static int
> +int
>  dt_read_scalar(caddr_t addr, const dtrace_recdesc_t *rec, uint64_t *valp)
>  {
>  	addr += rec->dtrd_offset;
> @@ -1983,25 +1983,6 @@ dt_print_trace(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
>  	return dt_print_rawbytes(dtp, fp, data, rec->dtrd_size);
>  }
>  
> -static int
> -dt_print_print(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
> -	       const caddr_t buf)
> -{
> -	dtrace_recdesc_t *data_rec = rec + 1;
> -	size_t max_size = dtp->dt_options[DTRACEOPT_PRINTSIZE];
> -	size_t size = (size_t)data_rec->dtrd_arg;
> -	uint64_t printaddr;
> -
> -	if (size > max_size)
> -		size = max_size;
> -
> -	if (dt_read_scalar(buf, rec, &printaddr) < 0)
> -		return dt_set_errno(dtp, EDT_PRINT);
> -
> -	return dt_print_type(dtp, fp, printaddr, (ctf_id_t)rec->dtrd_arg,
> -			     (caddr_t)buf + data_rec->dtrd_offset, size);
> -}
> -
>  /*
>   * The lifecycle of speculation buffers is as follows:
>   *
> @@ -2562,11 +2543,8 @@ dt_consume_one_probe(dtrace_hdl_t *dtp, FILE *fp, char *data, uint32_t size,
>  			func = dtrace_freopen;
>  			break;
>  		case DTRACEACT_PRINT:
> -			n = dt_print_print(dtp, fp, rec, data);
> -			if (n < 0)
> -				return -1;
> -			i += n - 1;
> -			continue;
> +			func = dt_print_type;
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> index 01313ff3..88b31ce0 100644
> --- a/libdtrace/dt_impl.h
> +++ b/libdtrace/dt_impl.h
> @@ -789,6 +789,7 @@ extern int dt_buffered_flush(dtrace_hdl_t *, dtrace_probedata_t *,
>      const dtrace_recdesc_t *, const dtrace_aggdata_t *, uint32_t flags);
>  extern void dt_buffered_disable(dtrace_hdl_t *);
>  extern void dt_buffered_destroy(dtrace_hdl_t *);
> +extern int dt_read_scalar(caddr_t, const dtrace_recdesc_t *, uint64_t *);
>  
>  extern uint64_t dt_stddev(uint64_t *, uint64_t);
>  
> diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
> index 50842216..e999498c 100644
> --- a/libdtrace/dt_printf.c
> +++ b/libdtrace/dt_printf.c
> @@ -14,6 +14,7 @@
>  #include <limits.h>
>  #include <port.h>
>  
> +#include <dt_module.h>
>  #include <dt_printf.h>
>  #include <dt_string.h>
>  #include <dt_impl.h>
> @@ -2235,15 +2236,34 @@ err:
>  }
>  
>  int
> -dt_print_type(dtrace_hdl_t *dtp, FILE *fp, uint64_t printaddr,
> -	      ctf_id_t type, caddr_t data, size_t size)
> +dt_print_type(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
> +	      const dtrace_probedata_t *data, const dtrace_recdesc_t *recs,
> +	      uint_t nrecs, const void *buf, size_t len)
>  {
> +	const char *modname = ((dt_pfargv_t *)fmtdata)->pfv_format;
> +	size_t max_size = dtp->dt_options[DTRACEOPT_PRINTSIZE];
> +	ctf_id_t type = (ctf_id_t)recs->dtrd_arg;
> +	const dtrace_recdesc_t *data_rec = recs + 1;
> +	size_t size = data_rec->dtrd_size;
> +	caddr_t addr_data = (caddr_t)buf;
>  	struct dt_visit_arg dva;
> +	uint64_t printaddr;
> +	dt_module_t *dmp;
>  
> +	if (size > max_size)
> +		size = max_size;
> +
> +	if (dt_read_scalar(addr_data, recs, &printaddr) < 0)
> +		return dt_set_errno(dtp, EDT_PRINT);
> +
> +	dmp = dt_module_lookup_by_name(dtp, modname);
> +	if (!dmp) {
> +		return dt_set_errno(dtp, EDT_PRINT);
> +	}
>  	dva.dv_dtp = dtp;
>  	dva.dv_fp = fp;
> -	dva.dv_ctfp = dtp->dt_ddefs->dm_ctfp;
> -	dva.dv_data = data;
> +	dva.dv_ctfp = dmp->dm_ctfp;
> +	dva.dv_data = addr_data + data_rec->dtrd_offset;
>  	dva.dv_size = size;
>  	dva.dv_last_depth = 0;
>  	dva.dv_startindent = DT_PRINT_STARTINDENT;
> diff --git a/libdtrace/dt_printf.h b/libdtrace/dt_printf.h
> index 9771c4a8..4f4386a5 100644
> --- a/libdtrace/dt_printf.h
> +++ b/libdtrace/dt_printf.h
> @@ -108,8 +108,10 @@ extern int dt_print_ustack(dtrace_hdl_t *, FILE *,
>      const char *, caddr_t, uint64_t);
>  extern int dt_print_mod(dtrace_hdl_t *, FILE *, const char *, caddr_t);
>  extern int dt_print_umod(dtrace_hdl_t *, FILE *, const char *, caddr_t);
> -extern int dt_print_type(dtrace_hdl_t *, FILE *, uint64_t, ctf_id_t,
> -			 caddr_t, size_t);
> +extern int dt_print_type(dtrace_hdl_t *dtp, FILE *fp, void *fmtdata,
> +			 const dtrace_probedata_t *data,
> +			 const dtrace_recdesc_t *recs,
> +			 uint_t nrecs, const void *buf, size_t len);
>  
>  #ifdef	__cplusplus
>  }
> -- 
> 2.43.5

Re: [PATCH v5 2/2] unittest/print: add test covering module-defined type

[Kris Van Hees]

On Thu, Aug 01, 2024 at 10:29:12PM +0100, Alan Maguire wrote:
> print() action tests do not cover module-defined types.  Add a test
> using a module-defined type that has not changed recently.
> 
> struct tun_page was chosen since it is identical in UEK6U3 and upstream
> and declared in tun.c which is a module on most distros.
> 
> Added tun to test/modules as suggested by Eugene.
> 
> Suggested-by: Nick Alcock <nick.alcock@oracle.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> Reviewed-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  test/modules                            |  1 +
>  test/unittest/print/tst.print.modtype.d | 18 ++++++++++++++++++
>  test/unittest/print/tst.print.modtype.r |  6 ++++++
>  3 files changed, 25 insertions(+)
>  create mode 100644 test/unittest/print/tst.print.modtype.d
>  create mode 100644 test/unittest/print/tst.print.modtype.r
> 
> diff --git a/test/modules b/test/modules
> index 53dc5544..0f01d6e0 100644
> --- a/test/modules
> +++ b/test/modules
> @@ -2,3 +2,4 @@ ext4
>  isofs
>  nfs
>  rds
> +tun
> diff --git a/test/unittest/print/tst.print.modtype.d b/test/unittest/print/tst.print.modtype.d
> new file mode 100644
> index 00000000..ed68b492
> --- /dev/null
> +++ b/test/unittest/print/tst.print.modtype.d
> @@ -0,0 +1,18 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2024, 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.
> + */
> +/* @@nosort */
> +
> +#pragma D option quiet
> +
> +BEGIN
> +{
> +	tun_page = (struct tun_page *)alloca(sizeof (struct tun_page));
> +	tun_page->page = (struct page *)0xfeedfacefeedface;
> +	tun_page->count = 123;
> +	print(tun_page);
> +	exit(0);
> +}
> diff --git a/test/unittest/print/tst.print.modtype.r b/test/unittest/print/tst.print.modtype.r
> new file mode 100644
> index 00000000..b9736607
> --- /dev/null
> +++ b/test/unittest/print/tst.print.modtype.r
> @@ -0,0 +1,6 @@
> +{ptr} = *
> +                                            (struct tun_page) {
> +                                             .page = (struct page *){ptr},
> +                                             .count = (int)123,
> +                                            }
> +
> -- 
> 2.43.5
>