[DTrace-devel] [PATCH v2 3/3] Clean up argument processing (and its testing) for trunc()

eugene.loh at oracle.com eugene.loh at oracle.com
Thu Feb 11 16:25:35 PST 2021


From: Eugene Loh <eugene.loh at oracle.com>

Add some argument vetting to dt_cg_act_trunc() and write the
action to the output buffer in anticipation of a future patch
that will implement trunc() on the consumer side.

Remove the obsolete D_TRUNC_SCALAR and D_TRUNC_PROTO errors
since they are now detected by earlier argument checking.

Clean up associated testing.  Note these name changes:
  err.D_TRUNC_PROTO.badmany.d -> err.D_PROTO_LEN.trunc_too_many_args.d
  err.D_TRUNC_PROTO.badnone.d -> err.D_PROTO_LEN.trunc_too_few_args.d
  err.D_TRUNC_SCALAR.bad.d    -> err.D_PROTO_ARG.trunc_non_scalar_trunc.d
  err.D_TRUNC_AGGARG.bad.d    -> err.D_TRUNC_AGGARG.trunc_no_agg.d

Further clean up is warranted.  For example, none of the D_*_AGGBAD
errors are tested, but it appears that they might never be triggered.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
 libdtrace/dt_cg.c                             | 42 +++++++++++++++++++
 libdtrace/dt_errtags.h                        |  2 -
 libdtrace/dt_ident.c                          | 36 ++++++----------
 .../err.D_NORMALIZE_PROTO.norm_too_few_args.d |  2 +-
 .../err.D_PROTO_ARG.norm_non_scalar_normal.d  |  2 +-
 ... err.D_PROTO_ARG.trunc_non_scalar_trunc.d} |  8 ++--
 .../aggs/err.D_PROTO_LEN.clear_too_few_args.d |  2 +-
 .../err.D_PROTO_LEN.clear_too_many_args.d     |  2 +-
 .../err.D_PROTO_LEN.lquantize_too_few_args.d  |  2 +-
 .../aggs/err.D_PROTO_LEN.trunc_too_few_args.d | 18 ++++++++
 .../err.D_PROTO_LEN.trunc_too_many_args.d     | 19 +++++++++
 ...ad.d => err.D_TRUNC_AGGARG.trunc_no_agg.d} |  5 +--
 .../unittest/aggs/err.D_TRUNC_PROTO.badmany.d | 14 -------
 test/unittest/aggs/err.D_TRUNC_SCALAR.bad.d   | 14 -------
 14 files changed, 102 insertions(+), 66 deletions(-)
 rename test/unittest/aggs/{err.D_TRUNC_PROTO.badnone.d => err.D_PROTO_ARG.trunc_non_scalar_trunc.d} (60%)
 create mode 100644 test/unittest/aggs/err.D_PROTO_LEN.trunc_too_few_args.d
 create mode 100644 test/unittest/aggs/err.D_PROTO_LEN.trunc_too_many_args.d
 rename test/unittest/aggs/{err.D_TRUNC_AGGARG.bad.d => err.D_TRUNC_AGGARG.trunc_no_agg.d} (61%)
 delete mode 100644 test/unittest/aggs/err.D_TRUNC_PROTO.badmany.d
 delete mode 100644 test/unittest/aggs/err.D_TRUNC_SCALAR.bad.d

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 310d687c..77898692 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -1101,6 +1101,48 @@ dt_cg_act_tracemem(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
 static void
 dt_cg_act_trunc(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
 {
+	dt_node_t	*anp, *trunc;
+	dt_ident_t	*aid;
+	char		n[DT_TYPE_NAMELEN];
+	int		argc = 0;
+
+	for (anp = dnp->dn_args; anp != NULL; anp = anp->dn_list)
+		argc++;
+	assert(argc == 1 || argc == 2);
+
+	anp = dnp->dn_args;
+	assert(anp != NULL);
+	if (anp->dn_kind != DT_NODE_AGG)
+		dnerror(dnp, D_TRUNC_AGGARG,
+			"%s( ) argument #1 is incompatible with prototype:\n"
+			"\tprototype: aggregation\n\t argument: %s\n",
+			dnp->dn_ident->di_name,
+			dt_node_type_name(anp, n, sizeof(n)));
+
+	trunc = anp->dn_list;
+	if (argc == 2)
+		assert(trunc != NULL && dt_node_is_scalar(trunc));
+
+	aid = anp->dn_ident;
+	if (aid->di_gen == pcb->pcb_hdl->dt_gen &&
+	    !(aid->di_flags & DT_IDFLG_MOD))
+		dnerror(dnp, D_TRUNC_AGGBAD,
+			"undefined aggregation: @%s\n", aid->di_name);
+
+	/*
+	 * FIXME: Needs implementation
+	 * TODO: Emit code to truncate the given aggregation.
+	 * DEPENDS ON: How aggregations are implemented using eBPF (hashmap?).
+	 * AGGID = aid->di_id
+	 */
+	dt_cg_store_val(pcb, anp, DTRACEACT_LIBACT, NULL, DT_ACT_TRUNC);
+#if 0
+	/*
+	 * FIXME: There is an optional trunction value.
+	 * if (argc == 1), the optional value is missing, but "0" may be specified.
+	 */
+	dt_cg_store_val(pcb, trunc, DTRACEACT_LIBACT, NULL, DT_ACT_TRUNC);
+#endif
 }
 
 static void
diff --git a/libdtrace/dt_errtags.h b/libdtrace/dt_errtags.h
index 9abf0d4e..73d1bc8b 100644
--- a/libdtrace/dt_errtags.h
+++ b/libdtrace/dt_errtags.h
@@ -203,8 +203,6 @@ typedef enum {
 	D_NORMALIZE_SCALAR,		/* normalize() value must be scalar */
 	D_NORMALIZE_AGGARG,		/* aggregation arg type mismatch */
 	D_NORMALIZE_AGGBAD,		/* normalize() aggregation not def. */
-	D_TRUNC_PROTO,			/* trunc() prototype mismatch */
-	D_TRUNC_SCALAR,			/* trunc() value must be scalar */
 	D_TRUNC_AGGARG,			/* aggregation arg type mismatch */
 	D_TRUNC_AGGBAD,			/* trunc() aggregation not def. */
 	D_PROV_BADNAME,			/* invalid provider name */
diff --git a/libdtrace/dt_ident.c b/libdtrace/dt_ident.c
index 317e3360..b70dbbe0 100644
--- a/libdtrace/dt_ident.c
+++ b/libdtrace/dt_ident.c
@@ -322,36 +322,31 @@ dt_idcook_args(dt_node_t *dnp, dt_ident_t *idp, int argc, dt_node_t *ap)
 	char n1[DT_TYPE_NAMELEN];
 	char n2[DT_TYPE_NAMELEN];
 
-	if (argc != 1) {
+	if (argc != 1)
 		xyerror(D_PROTO_LEN, "%s[ ] prototype mismatch: %d arg%s"
 		    "passed, 1 expected\n", idp->di_name, argc,
 		    argc == 1 ? " " : "s ");
-	}
 
-	if (ap->dn_kind != DT_NODE_INT) {
+	if (ap->dn_kind != DT_NODE_INT)
 		xyerror(D_PROTO_ARG, "%s[ ] argument #1 is incompatible with "
 		    "prototype:\n\tprototype: %s\n\t argument: %s\n",
 		    idp->di_name, "integer constant",
 		    dt_type_name(ap->dn_ctfp, ap->dn_type, n1, sizeof(n1)));
-	}
 
-	if (yypcb->pcb_pdesc == NULL) {
+	if (yypcb->pcb_pdesc == NULL)
 		xyerror(D_ARGS_NONE, "%s[ ] may not be referenced outside "
 		    "of a probe clause\n", idp->di_name);
-	}
 
-	if (prp == NULL) {
+	if (prp == NULL)
 		xyerror(D_ARGS_MULTI,
 		    "%s[ ] may not be referenced because probe description %s "
 		    "matches an unstable set of probes\n", idp->di_name,
 		    dtrace_desc2str(yypcb->pcb_pdesc, n1, sizeof(n1)));
-	}
 
-	if (ap->dn_value >= prp->argc) {
+	if (ap->dn_value >= prp->argc)
 		xyerror(D_ARGS_IDX, "index %lld is out of range for %s %s[ ]\n",
 		    (longlong_t)ap->dn_value, dtrace_desc2str(yypcb->pcb_pdesc,
 		    n1, sizeof(n1)), idp->di_name);
-	}
 
 	/*
 	 * Look up the native and translated argument types for the probe.
@@ -363,15 +358,13 @@ dt_idcook_args(dt_node_t *dnp, dt_ident_t *idp, int argc, dt_node_t *ap)
 	xnp = prp->xargv[ap->dn_value];
 	nnp = prp->nargv[prp->mapping[ap->dn_value]];
 
-	if (xnp->dn_type == CTF_ERR) {
+	if (xnp->dn_type == CTF_ERR)
 		xyerror(D_ARGS_TYPE, "failed to resolve translated type for "
 		    "%s[%lld]\n", idp->di_name, (longlong_t)ap->dn_value);
-	}
 
-	if (nnp->dn_type == CTF_ERR) {
+	if (nnp->dn_type == CTF_ERR)
 		xyerror(D_ARGS_TYPE, "failed to resolve native type for "
 		    "%s[%lld]\n", idp->di_name, (longlong_t)ap->dn_value);
-	}
 
 	if (dtp->dt_xlatemode == DT_XL_STATIC && (
 	    nnp == xnp || dt_node_is_argcompat(nnp, xnp))) {
@@ -432,28 +425,24 @@ dt_idcook_regs(dt_node_t *dnp, dt_ident_t *idp, int argc, dt_node_t *ap)
 	dtrace_hdl_t *dtp = yypcb->pcb_hdl;
 	char n[DT_TYPE_NAMELEN];
 
-	if (argc != 1) {
+	if (argc != 1)
 		xyerror(D_PROTO_LEN, "%s[ ] prototype mismatch: %d arg%s"
 		    "passed, 1 expected\n", idp->di_name,
 		    argc, argc == 1 ? " " : "s ");
-	}
 
-	if (ap->dn_kind != DT_NODE_INT) {
+	if (ap->dn_kind != DT_NODE_INT)
 		xyerror(D_PROTO_ARG, "%s[ ] argument #1 is incompatible with "
 		    "prototype:\n\tprototype: %s\n\t argument: %s\n",
 		    idp->di_name, "integer constant",
 		    dt_type_name(ap->dn_ctfp, ap->dn_type, n, sizeof(n)));
-	}
 
-	if ((ap->dn_flags & DT_NF_SIGNED) && (int64_t)ap->dn_value < 0) {
+	if ((ap->dn_flags & DT_NF_SIGNED) && (int64_t)ap->dn_value < 0)
 		xyerror(D_REGS_IDX, "index %lld is out of range for array %s\n",
 		    (longlong_t)ap->dn_value, idp->di_name);
-	}
 
-	if (dt_type_lookup("uint64_t", &dtt) == -1) {
+	if (dt_type_lookup("uint64_t", &dtt) == -1)
 		xyerror(D_UNKNOWN, "failed to resolve type of %s: %s\n",
 		    idp->di_name, dtrace_errmsg(dtp, dtrace_errno(dtp)));
-	}
 
 	idp->di_ctfp = dtt.dtt_ctfp;
 	idp->di_type = dtt.dtt_type;
@@ -469,12 +458,11 @@ dt_idcook_type(dt_node_t *dnp, dt_ident_t *idp, int argc, dt_node_t *args)
 		dtrace_hdl_t *dtp = yypcb->pcb_hdl;
 		dtrace_typeinfo_t dtt;
 
-		if (dt_type_lookup(idp->di_iarg, &dtt) == -1) {
+		if (dt_type_lookup(idp->di_iarg, &dtt) == -1)
 			xyerror(D_UNKNOWN,
 			    "failed to resolve type %s for identifier %s: %s\n",
 			    (const char *)idp->di_iarg, idp->di_name,
 			    dtrace_errmsg(dtp, dtrace_errno(dtp)));
-		}
 
 		idp->di_ctfp = dtt.dtt_ctfp;
 		idp->di_type = dtt.dtt_type;
diff --git a/test/unittest/aggs/err.D_NORMALIZE_PROTO.norm_too_few_args.d b/test/unittest/aggs/err.D_NORMALIZE_PROTO.norm_too_few_args.d
index e886590c..129e7c22 100644
--- a/test/unittest/aggs/err.D_NORMALIZE_PROTO.norm_too_few_args.d
+++ b/test/unittest/aggs/err.D_NORMALIZE_PROTO.norm_too_few_args.d
@@ -6,7 +6,7 @@
  */
 
 /*
- * ASSERTION: THe normalize() action requires 2 arguments (agg and normal).
+ * ASSERTION: The normalize() action requires 2 arguments (agg and normal).
  *
  * SECTION: Aggregations/Data Normalization
  */
diff --git a/test/unittest/aggs/err.D_PROTO_ARG.norm_non_scalar_normal.d b/test/unittest/aggs/err.D_PROTO_ARG.norm_non_scalar_normal.d
index c5b317ed..9a117b55 100644
--- a/test/unittest/aggs/err.D_PROTO_ARG.norm_non_scalar_normal.d
+++ b/test/unittest/aggs/err.D_PROTO_ARG.norm_non_scalar_normal.d
@@ -8,7 +8,7 @@
 /*
  * ASSERTION: The second argument to normalize() should be a scalar.
  *
- * SECTION: Aggregations/Data Nonmalization
+ * SECTION: Aggregations/Data Normalization
  */
 
 BEGIN
diff --git a/test/unittest/aggs/err.D_TRUNC_PROTO.badnone.d b/test/unittest/aggs/err.D_PROTO_ARG.trunc_non_scalar_trunc.d
similarity index 60%
rename from test/unittest/aggs/err.D_TRUNC_PROTO.badnone.d
rename to test/unittest/aggs/err.D_PROTO_ARG.trunc_non_scalar_trunc.d
index a3ec9c71..292f76d5 100644
--- a/test/unittest/aggs/err.D_TRUNC_PROTO.badnone.d
+++ b/test/unittest/aggs/err.D_PROTO_ARG.trunc_non_scalar_trunc.d
@@ -1,13 +1,13 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 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.
  */
-/* @@xfail: dtv2 */
 
 BEGIN
 {
-	trunc();
-	exit(1);
+	@a = count();
+	trunc(@a, @a);
+	exit(0);
 }
diff --git a/test/unittest/aggs/err.D_PROTO_LEN.clear_too_few_args.d b/test/unittest/aggs/err.D_PROTO_LEN.clear_too_few_args.d
index 112081ec..060b9095 100644
--- a/test/unittest/aggs/err.D_PROTO_LEN.clear_too_few_args.d
+++ b/test/unittest/aggs/err.D_PROTO_LEN.clear_too_few_args.d
@@ -8,7 +8,7 @@
 /*
  * ASSERTION: The clear() action requires one argument (agg).
  *
- * SECTION: Aggregations/Data Normalization
+ * SECTION: Aggregations/Clearing Aggregations
  */
 
 BEGIN
diff --git a/test/unittest/aggs/err.D_PROTO_LEN.clear_too_many_args.d b/test/unittest/aggs/err.D_PROTO_LEN.clear_too_many_args.d
index d5fd70e8..ad6c0bb2 100644
--- a/test/unittest/aggs/err.D_PROTO_LEN.clear_too_many_args.d
+++ b/test/unittest/aggs/err.D_PROTO_LEN.clear_too_many_args.d
@@ -8,7 +8,7 @@
 /*
  * ASSERTION: The clear() action accepts only one argument (agg).
  *
- * SECTION: Aggregations/Data Normalization
+ * SECTION: Aggregations/Clearing Aggregations
  */
 
 BEGIN
diff --git a/test/unittest/aggs/err.D_PROTO_LEN.lquantize_too_few_args.d b/test/unittest/aggs/err.D_PROTO_LEN.lquantize_too_few_args.d
index 5f0e8c38..0ce50c46 100644
--- a/test/unittest/aggs/err.D_PROTO_LEN.lquantize_too_few_args.d
+++ b/test/unittest/aggs/err.D_PROTO_LEN.lquantize_too_few_args.d
@@ -6,7 +6,7 @@
  */
 
 /*
- * ASSERTION: lquantize() should have et least 3 arguments
+ * ASSERTION: lquantize() requires at least 3 arguments
  *
  * SECTION: Aggregations/Aggregations
  */
diff --git a/test/unittest/aggs/err.D_PROTO_LEN.trunc_too_few_args.d b/test/unittest/aggs/err.D_PROTO_LEN.trunc_too_few_args.d
new file mode 100644
index 00000000..e3cee92b
--- /dev/null
+++ b/test/unittest/aggs/err.D_PROTO_LEN.trunc_too_few_args.d
@@ -0,0 +1,18 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2021, 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 trunc() action requires one argument (agg).
+ *
+ * SECTION: Aggregations/Truncating Aggregations
+ */
+
+BEGIN
+{
+	trunc();
+	exit(0);
+}
diff --git a/test/unittest/aggs/err.D_PROTO_LEN.trunc_too_many_args.d b/test/unittest/aggs/err.D_PROTO_LEN.trunc_too_many_args.d
new file mode 100644
index 00000000..5d2d8d41
--- /dev/null
+++ b/test/unittest/aggs/err.D_PROTO_LEN.trunc_too_many_args.d
@@ -0,0 +1,19 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2006, 2021, 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 trunc() action accepts just 2 arguments.
+ *
+ * SECTION: Aggregations/Truncating Aggregations
+ */
+
+BEGIN
+{
+	@a = count();
+	trunc(@a, 4, 1);
+	exit(0);
+}
diff --git a/test/unittest/aggs/err.D_TRUNC_AGGARG.bad.d b/test/unittest/aggs/err.D_TRUNC_AGGARG.trunc_no_agg.d
similarity index 61%
rename from test/unittest/aggs/err.D_TRUNC_AGGARG.bad.d
rename to test/unittest/aggs/err.D_TRUNC_AGGARG.trunc_no_agg.d
index adbece15..9a194496 100644
--- a/test/unittest/aggs/err.D_TRUNC_AGGARG.bad.d
+++ b/test/unittest/aggs/err.D_TRUNC_AGGARG.trunc_no_agg.d
@@ -1,15 +1,14 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 2021, 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.
  */
-/* @@xfail: dtv2 */
 
 int i;
 
 BEGIN
 {
 	trunc(i);
-	exit(1);
+	exit(0);
 }
diff --git a/test/unittest/aggs/err.D_TRUNC_PROTO.badmany.d b/test/unittest/aggs/err.D_TRUNC_PROTO.badmany.d
deleted file mode 100644
index e78435e6..00000000
--- a/test/unittest/aggs/err.D_TRUNC_PROTO.badmany.d
+++ /dev/null
@@ -1,14 +0,0 @@
-/*
- * Oracle Linux DTrace.
- * Copyright (c) 2006, 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.
- */
-/* @@xfail: dtv2 */
-
-BEGIN
-{
-	@[0] = count();
-	trunc(@, 10, 20);
-	exit(1);
-}
diff --git a/test/unittest/aggs/err.D_TRUNC_SCALAR.bad.d b/test/unittest/aggs/err.D_TRUNC_SCALAR.bad.d
deleted file mode 100644
index 200a8889..00000000
--- a/test/unittest/aggs/err.D_TRUNC_SCALAR.bad.d
+++ /dev/null
@@ -1,14 +0,0 @@
-/*
- * Oracle Linux DTrace.
- * Copyright (c) 2006, 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.
- */
-/* @@xfail: dtv2 */
-
-BEGIN
-{
-	@[0] = count();
-	trunc(@, @);
-	exit(1);
-}
-- 
2.18.4




More information about the DTrace-devel mailing list