[DTrace-devel] [PATCH v2 3/3] Clean up argument processing (and its testing) for trunc()
Kris Van Hees
kris.van.hees at oracle.com
Fri Mar 19 11:40:28 PDT 2021
On Thu, Feb 11, 2021 at 07:25:35PM -0500, eugene.loh at oracle.com wrote:
> 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>
Reviewed-by: Kris Van Hees <kris.van.hees 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
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list