[DTrace-devel] [PATCH 06/12] Add support for mod() and sym()

[Kris Van Hees]

On Fri, May 28, 2021 at 02:35:10PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> Also add a new test, since existing tests do not yet work.  (They depend
> on indexed aggregations.)  Further, the sym() tests are very weak.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_cg.c                          |  4 ++
>  libdtrace/dt_consume.c                     | 12 ++++++
>  test/unittest/actions/symmod/tst.symmod.sh | 49 ++++++++++++++++++++++
>  3 files changed, 65 insertions(+)
>  create mode 100755 test/unittest/actions/symmod/tst.symmod.sh
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index e7715aec..c111e06a 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1411,6 +1411,10 @@ dt_cg_act_stop(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>  static void
>  dt_cg_act_symmod(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>  {
> +	dt_node_t	*arg = dnp->dn_args;
> +
> +	assert(arg != NULL);
> +	dt_cg_store_val(pcb, arg, kind, NULL, 0);
>  }
>  
>  static void
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index 483f9f28..c0b74ce2 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -2069,6 +2069,18 @@ dt_consume_one(dtrace_hdl_t *dtp, FILE *fp, char *buf,
>  				continue;
>  			}
>  
> +			if (act == DTRACEACT_SYM) {
> +				if (dt_print_sym(dtp, fp, NULL, addr) < 0)
> +					return -1;
> +				continue;
> +			}
> +
> +			if (act == DTRACEACT_MOD) {
> +				if (dt_print_mod(dtp, fp, NULL, addr) < 0)
> +					return -1;
> +				continue;
> +			}

This really ought to be cases in a switch (act) { ... } statement.

> +
>  			switch (act) {
>  			case DTRACEACT_PRINTF:
>  				func = dtrace_fprintf;
> diff --git a/test/unittest/actions/symmod/tst.symmod.sh b/test/unittest/actions/symmod/tst.symmod.sh
> new file mode 100755
> index 00000000..d0c147af
> --- /dev/null
> +++ b/test/unittest/actions/symmod/tst.symmod.sh
> @@ -0,0 +1,49 @@
> +#!/bin/bash
> +#
> +# 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.
> +#
> +
> +dtrace=$1
> +
> +# pick a test symbol from /proc/kallmodsyms
> +ADD=`awk '/ksys_write/ {print $1}' /proc/kallmodsyms`
> +NAM=`awk '/ksys_write/ {print $4}' /proc/kallmodsyms`
> +MOD=`awk '/ksys_write/ {print $5}' /proc/kallmodsyms`

Surely you can do this without needing to invoke awk three times?

> +
> +# a blank module means the module is vmlinux
> +if [ x$MOD == x ]; then
> +	MOD=vmlinux
> +fi
> +
> +# run DTrace to test mod() and sym()
> +MYMOD=`build/run-dtrace -qn 'BEGIN {mod(0x'$ADD'); exit(0) }'`

Shouldn't you be using $dtrace instead?

> +if [ $? -ne 0 ]; then
> +	echo DTrace failed

I don't think this message adds any value.  I expect DTrace will already have
given output if it failed, and if not, this doesn't add anything to it because
it also does not identify which one failed.  I'd just leave it out and go with
whatever DTrace emits.

> +	exit 1
> +fi
> +MYNAM=`build/run-dtrace -qn 'BEGIN {sym(0x'$ADD'); exit(0) }'`

Shouldn't you be using $dtrace instead?  Also, why invoke dtrace twice?
Why not simply use a single invocation to get mod and sym info?

> +if [ $? -ne 0 ]; then
> +	echo DTrace failed

(Same as above.)

> +	exit 1
> +fi
> +MYNAM=$MYNAM

What is this supposed to do?

> +
> +# reporting
> +echo test $ADD $NAM $MOD
> +echo expect   $MOD  $MOD'`'$NAM
> +echo actual $MYMOD       $MYNAM
> +
> +if [ $MOD != $MYMOD ]; then
> +	echo fail: $MOD does not match $MYMOD
> +	exit 1
> +fi
> +if [ $MOD'`'$NAM != $MYNAM ]; then
> +	echo fail: $MOD'`'$NAM does not match $MYNAM
> +	exit 1
> +fi
> +
> +echo success

Unnecessary.  The 'exit 0' that follows is sufficient indication that the test
succeeded.

> +exit 0
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel