[DTrace-devel] [PATCH 1/3] Add support for copyout() subroutine

Kris Van Hees kris.van.hees at oracle.com
Fri Feb 24 05:57:59 UTC 2023


On Fri, Feb 03, 2023 at 10:31:29PM -0500, eugene.loh--- via DTrace-devel wrote:
> From: Kris Van Hees <kris.van.hees at oracle.com>
> 
> We also perform a bit of dead-code elimination.
> 
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>

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

... in slightly modified form after merging multiple patches.

> ---
>  libdtrace/dt_cg.c                             | 67 +++++++++++++------
>  test/unittest/dif/copyout.d                   |  1 -
>  .../err.D_PROTO_ARG.copyoutbadarg.d           |  0
>  .../copyout/err.D_PROTO_ARG.copyoutbadarg.r   |  4 ++
>  .../err.D_PROTO_LEN.copyouttoofew.d           |  0
>  .../copyout/err.D_PROTO_LEN.copyouttoofew.r   |  2 +
>  .../err.D_PROTO_LEN.copyouttoomany.d          |  0
>  .../copyout/err.D_PROTO_LEN.copyouttoomany.r  |  2 +
>  .../funcs/{ => copyout}/err.copyout.aarch64.x |  0
>  .../funcs/{ => copyout}/err.copyout.d         |  0
>  .../err.copyoutbadaddr.aarch64.x              |  0
>  .../funcs/{ => copyout}/err.copyoutbadaddr.sh |  0
>  test/unittest/funcs/copyout/tst.copyout.r     |  2 +
>  test/unittest/funcs/copyout/tst.copyout.sh    | 54 +++++++++++++++
>  .../funcs/err.D_PROTO_ARG.copyoutbadarg.r     |  4 --
>  .../funcs/err.D_PROTO_LEN.copyouttoofew.r     |  2 -
>  .../funcs/err.D_PROTO_LEN.copyouttoomany.r    |  2 -
>  test/unittest/funcs/err.copyoutstrbadaddr.sh  |  6 +-
>  .../err.D_ACT_SPEC.SpeculateWithCopyOut.d     |  2 +-
>  19 files changed, 116 insertions(+), 32 deletions(-)
>  rename test/unittest/funcs/{ => copyout}/err.D_PROTO_ARG.copyoutbadarg.d (100%)
>  create mode 100644 test/unittest/funcs/copyout/err.D_PROTO_ARG.copyoutbadarg.r
>  rename test/unittest/funcs/{ => copyout}/err.D_PROTO_LEN.copyouttoofew.d (100%)
>  create mode 100644 test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoofew.r
>  rename test/unittest/funcs/{ => copyout}/err.D_PROTO_LEN.copyouttoomany.d (100%)
>  create mode 100644 test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoomany.r
>  rename test/unittest/funcs/{ => copyout}/err.copyout.aarch64.x (100%)
>  rename test/unittest/funcs/{ => copyout}/err.copyout.d (100%)
>  rename test/unittest/funcs/{ => copyout}/err.copyoutbadaddr.aarch64.x (100%)
>  rename test/unittest/funcs/{ => copyout}/err.copyoutbadaddr.sh (100%)
>  create mode 100644 test/unittest/funcs/copyout/tst.copyout.r
>  create mode 100755 test/unittest/funcs/copyout/tst.copyout.sh
>  delete mode 100644 test/unittest/funcs/err.D_PROTO_ARG.copyoutbadarg.r
>  delete mode 100644 test/unittest/funcs/err.D_PROTO_LEN.copyouttoofew.r
>  delete mode 100644 test/unittest/funcs/err.D_PROTO_LEN.copyouttoomany.r
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 30c48155..53d969d5 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1295,20 +1295,6 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
>  	return -1;
>  }
>  
> -#ifdef FIXME
> -/*
> - * Utility function to determine if a given action description is destructive.
> - * The DIFOFLG_DESTRUCTIVE bit is set for us by the DIF assembler (see dt_as.c).
> - */
> -static int
> -dt_action_destructive(const dtrace_actdesc_t *ap)
> -{
> -	return (DTRACEACT_ISDESTRUCTIVE(ap->dtad_kind) ||
> -		(ap->dtad_kind == DTRACEACT_DIFEXPR &&
> -		 (ap->dtad_difo->dtdo_flags & DIFOFLG_DESTRUCTIVE)));
> -}
> -#endif
> -
>  static void
>  dt_cg_clsflags(dt_pcb_t *pcb, dtrace_actkind_t kind, const dt_node_t *dnp)
>  {
> @@ -1361,11 +1347,7 @@ dt_cg_clsflags(dt_pcb_t *pcb, dtrace_actkind_t kind, const dt_node_t *dnp)
>  	}
>  
>  	if (*cfp & DT_CLSFLAG_SPECULATE) {
> -#ifdef FIXME
> -		if (dt_action_destructive(ap))
> -#else
>  		if (DTRACEACT_ISDESTRUCTIVE(kind))
> -#endif
>  			dnerror(dnp, D_ACT_SPEC, "destructive actions "
>  			    "may not follow speculate( )\n");
>  		if (kind == DTRACEACT_EXIT)
> @@ -4719,6 +4701,53 @@ dt_cg_subr_copyinto(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  	TRACE_REGSET("    subr-copyinto:End  ");
>  }
>  
> +static void
> +dt_cg_subr_copyout(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> +{
> +	dt_node_t	*src = dnp->dn_args;
> +	dt_node_t	*dst = src->dn_list;
> +	dt_node_t	*size = dst->dn_list;
> +	uint_t		lbl_ok = dt_irlist_label(dlp);
> +
> +	dt_cg_clsflags(yypcb, DTRACEACT_PROC_DESTRUCTIVE, dnp);
> +
> +	TRACE_REGSET("    subr-copyout:Begin");
> +
> +	dt_cg_node(src, dlp, drp);
> +	dt_cg_node(dst, dlp, drp);
> +	dt_cg_node(size, dlp, drp);
> +
> +	/* Validate the pointers. */
> +	dt_cg_check_ptr_arg(dlp, drp, src, size);
> +	dt_cg_check_notnull(dlp, drp, dst->dn_reg);
> +
> +	if (dt_regset_xalloc_args(drp) == -1)
> +		longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> +
> +	emit(dlp,  BPF_MOV_REG(BPF_REG_1, dst->dn_reg));
> +	emit(dlp,  BPF_MOV_REG(BPF_REG_2, src->dn_reg));
> +	emit(dlp,  BPF_MOV_REG(BPF_REG_3, size->dn_reg));
> +	dt_regset_xalloc(drp, BPF_REG_0);
> +	emit(dlp,  BPF_CALL_HELPER(BPF_FUNC_probe_write_user));
> +	dt_regset_free_args(drp);
> +
> +	/*
> +	 * At this point the src is validated, so any problem must be with
> +	 * the dst address.
> +	 */
> +	emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_ok));
> +	dt_regset_free(drp, BPF_REG_0);
> +	dt_cg_probe_error(yypcb, DTRACEFLT_BADADDR, DT_ISREG, dst->dn_reg);
> +	emitl(dlp, lbl_ok,
> +		   BPF_NOP());
> +
> +	dt_regset_free(drp, src->dn_reg);
> +	dt_regset_free(drp, dst->dn_reg);
> +	dt_regset_free(drp, size->dn_reg);
> +
> +	TRACE_REGSET("    subr-copyout:End  ");
> +}
> +
>  static void
>  dt_cg_subr_strchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  {
> @@ -5195,7 +5224,7 @@ static dt_cg_subr_f *_dt_cg_subr[DIF_SUBR_MAX + 1] = {
>  	[DIF_SUBR_SPECULATION]		= &dt_cg_subr_speculation,
>  	[DIF_SUBR_PROGENYOF]		= &dt_cg_subr_progenyof,
>  	[DIF_SUBR_STRLEN]		= &dt_cg_subr_strlen,
> -	[DIF_SUBR_COPYOUT]		= NULL,
> +	[DIF_SUBR_COPYOUT]		= &dt_cg_subr_copyout,
>  	[DIF_SUBR_COPYOUTSTR]		= NULL,
>  	[DIF_SUBR_ALLOCA]		= &dt_cg_subr_alloca,
>  	[DIF_SUBR_BCOPY]		= &dt_cg_subr_bcopy,
> diff --git a/test/unittest/dif/copyout.d b/test/unittest/dif/copyout.d
> index 7a3fcf43..c7cc218e 100644
> --- a/test/unittest/dif/copyout.d
> +++ b/test/unittest/dif/copyout.d
> @@ -1,5 +1,4 @@
>  #pragma D option destructive
> -/* @@xfail: dtv2 */
>  
>  BEGIN
>  {
> diff --git a/test/unittest/funcs/err.D_PROTO_ARG.copyoutbadarg.d b/test/unittest/funcs/copyout/err.D_PROTO_ARG.copyoutbadarg.d
> similarity index 100%
> rename from test/unittest/funcs/err.D_PROTO_ARG.copyoutbadarg.d
> rename to test/unittest/funcs/copyout/err.D_PROTO_ARG.copyoutbadarg.d
> diff --git a/test/unittest/funcs/copyout/err.D_PROTO_ARG.copyoutbadarg.r b/test/unittest/funcs/copyout/err.D_PROTO_ARG.copyoutbadarg.r
> new file mode 100644
> index 00000000..6462940c
> --- /dev/null
> +++ b/test/unittest/funcs/copyout/err.D_PROTO_ARG.copyoutbadarg.r
> @@ -0,0 +1,4 @@
> +-- @@stderr --
> +dtrace: failed to compile script test/unittest/funcs/copyout/err.D_PROTO_ARG.copyoutbadarg.d: [D_PROTO_ARG] line 19: copyout( ) argument #2 is incompatible with prototype:
> +	prototype: uintptr_t
> +	 argument: string
> diff --git a/test/unittest/funcs/err.D_PROTO_LEN.copyouttoofew.d b/test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoofew.d
> similarity index 100%
> rename from test/unittest/funcs/err.D_PROTO_LEN.copyouttoofew.d
> rename to test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoofew.d
> diff --git a/test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoofew.r b/test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoofew.r
> new file mode 100644
> index 00000000..dffa57d7
> --- /dev/null
> +++ b/test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoofew.r
> @@ -0,0 +1,2 @@
> +-- @@stderr --
> +dtrace: failed to compile script test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoofew.d: [D_PROTO_LEN] line 19: copyout( ) prototype mismatch: 2 args passed, 3 expected
> diff --git a/test/unittest/funcs/err.D_PROTO_LEN.copyouttoomany.d b/test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoomany.d
> similarity index 100%
> rename from test/unittest/funcs/err.D_PROTO_LEN.copyouttoomany.d
> rename to test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoomany.d
> diff --git a/test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoomany.r b/test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoomany.r
> new file mode 100644
> index 00000000..3c8c5842
> --- /dev/null
> +++ b/test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoomany.r
> @@ -0,0 +1,2 @@
> +-- @@stderr --
> +dtrace: failed to compile script test/unittest/funcs/copyout/err.D_PROTO_LEN.copyouttoomany.d: [D_PROTO_LEN] line 20: copyout( ) prototype mismatch: 4 args passed, 3 expected
> diff --git a/test/unittest/funcs/err.copyout.aarch64.x b/test/unittest/funcs/copyout/err.copyout.aarch64.x
> similarity index 100%
> rename from test/unittest/funcs/err.copyout.aarch64.x
> rename to test/unittest/funcs/copyout/err.copyout.aarch64.x
> diff --git a/test/unittest/funcs/err.copyout.d b/test/unittest/funcs/copyout/err.copyout.d
> similarity index 100%
> rename from test/unittest/funcs/err.copyout.d
> rename to test/unittest/funcs/copyout/err.copyout.d
> diff --git a/test/unittest/funcs/err.copyoutbadaddr.aarch64.x b/test/unittest/funcs/copyout/err.copyoutbadaddr.aarch64.x
> similarity index 100%
> rename from test/unittest/funcs/err.copyoutbadaddr.aarch64.x
> rename to test/unittest/funcs/copyout/err.copyoutbadaddr.aarch64.x
> diff --git a/test/unittest/funcs/err.copyoutbadaddr.sh b/test/unittest/funcs/copyout/err.copyoutbadaddr.sh
> similarity index 100%
> rename from test/unittest/funcs/err.copyoutbadaddr.sh
> rename to test/unittest/funcs/copyout/err.copyoutbadaddr.sh
> diff --git a/test/unittest/funcs/copyout/tst.copyout.r b/test/unittest/funcs/copyout/tst.copyout.r
> new file mode 100644
> index 00000000..f13e5942
> --- /dev/null
> +++ b/test/unittest/funcs/copyout/tst.copyout.r
> @@ -0,0 +1,2 @@
> +HELLO WORLD; YOU HAVE A LONG MESsage to deliver
> +
> diff --git a/test/unittest/funcs/copyout/tst.copyout.sh b/test/unittest/funcs/copyout/tst.copyout.sh
> new file mode 100755
> index 00000000..748b138b
> --- /dev/null
> +++ b/test/unittest/funcs/copyout/tst.copyout.sh
> @@ -0,0 +1,54 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2023, 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
> +DIRNAME="$tmpdir/copyout.$$.$RANDOM"
> +mkdir -p $DIRNAME
> +cd $DIRNAME
> +
> +cat << EOF > main.c
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <string.h>
> +int main(int c, char **v) {
> +	int fd = open("/dev/null", O_WRONLY);
> +	char s[256];
> +
> +	/* the user buffer is filled with a lowercase message */
> +	sprintf(s, "hello world; you have a long message to deliver");
> +
> +	/* DTrace will intercept this call and overwrite the user buffer */
> +	write(fd, s, strlen(s));
> +
> +	close(fd);
> +	printf("%s\n", s);
> +	return 0;
> +}
> +EOF
> +
> +gcc main.c
> +if [ $? -ne 0 ]; then
> +	echo "compilation error"
> +	exit 1
> +fi
> +
> +$dtrace $dt_flags -qwn '
> +syscall::write:entry
> +/pid == $target/
> +{
> +	s = "HELLO WORLD; YOU HAVE A LONG MESSAGE TO DELIVER";
> +	copyout(s, arg1, 32);
> +	exit(0);
> +}' -c ./a.out
> +
> +if [ $? -ne 0 ]; then
> +	echo "DTrace error"
> +	exit 1
> +fi
> +
> +exit 0
> diff --git a/test/unittest/funcs/err.D_PROTO_ARG.copyoutbadarg.r b/test/unittest/funcs/err.D_PROTO_ARG.copyoutbadarg.r
> deleted file mode 100644
> index ac1f9928..00000000
> --- a/test/unittest/funcs/err.D_PROTO_ARG.copyoutbadarg.r
> +++ /dev/null
> @@ -1,4 +0,0 @@
> --- @@stderr --
> -dtrace: failed to compile script test/unittest/funcs/err.D_PROTO_ARG.copyoutbadarg.d: [D_PROTO_ARG] line 19: copyout( ) argument #2 is incompatible with prototype:
> -	prototype: uintptr_t
> -	 argument: string
> diff --git a/test/unittest/funcs/err.D_PROTO_LEN.copyouttoofew.r b/test/unittest/funcs/err.D_PROTO_LEN.copyouttoofew.r
> deleted file mode 100644
> index c6efc4fc..00000000
> --- a/test/unittest/funcs/err.D_PROTO_LEN.copyouttoofew.r
> +++ /dev/null
> @@ -1,2 +0,0 @@
> --- @@stderr --
> -dtrace: failed to compile script test/unittest/funcs/err.D_PROTO_LEN.copyouttoofew.d: [D_PROTO_LEN] line 19: copyout( ) prototype mismatch: 2 args passed, 3 expected
> diff --git a/test/unittest/funcs/err.D_PROTO_LEN.copyouttoomany.r b/test/unittest/funcs/err.D_PROTO_LEN.copyouttoomany.r
> deleted file mode 100644
> index 23718180..00000000
> --- a/test/unittest/funcs/err.D_PROTO_LEN.copyouttoomany.r
> +++ /dev/null
> @@ -1,2 +0,0 @@
> --- @@stderr --
> -dtrace: failed to compile script test/unittest/funcs/err.D_PROTO_LEN.copyouttoomany.d: [D_PROTO_LEN] line 20: copyout( ) prototype mismatch: 4 args passed, 3 expected
> diff --git a/test/unittest/funcs/err.copyoutstrbadaddr.sh b/test/unittest/funcs/err.copyoutstrbadaddr.sh
> index 6a1e32c1..5d4efe5e 100755
> --- a/test/unittest/funcs/err.copyoutstrbadaddr.sh
> +++ b/test/unittest/funcs/err.copyoutstrbadaddr.sh
> @@ -12,9 +12,9 @@ dtrace_script()
>  
>  	/*
>  	 * ASSERTION:
> -	 *	Verify that copyout() handles bad addresses.
> +	 *	Verify that copyoutstr() handles bad addresses.
>  	 *
> -	 * SECTION: Actions and Subroutines/copyout()
> +	 * SECTION: Actions and Subroutines/copyoutstr()
>  	 *
>  	 */
>  
> @@ -22,7 +22,7 @@ dtrace_script()
>  	{
>  		ptr = alloca(sizeof(char *));
>  		copyinto(curpsinfo->pr_envp, sizeof(char *), ptr);
> -		copyout(ptr, 0, sizeof(char *));
> +		copyoutstr(ptr, 0, sizeof(char *));
>  	}
>  
>  	ERROR
> diff --git a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOut.d b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOut.d
> index 908e9b9c..14cc1d5c 100644
> --- a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOut.d
> +++ b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOut.d
> @@ -5,7 +5,7 @@
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> -/* @@skip: dtv2, no copyout yet */
> +
>  /*
>   * ASSERTION: Destructive actions may never be speculative.
>   *
> -- 
> 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