[DTrace-devel] [PATCH v2 2/3] Restore order of bcopy() arg evaluations

Kris Van Hees kris.van.hees at oracle.com
Fri Feb 24 07:41:55 UTC 2023


On Fri, Feb 17, 2023 at 07:50:35PM -0500, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> In c26e2e5f279a ("Add support for copyinto() subroutine"), the order
> of arg evaluations for bcopy() was changed from evaluating the last
> arg (size) last to evaluating it first.  This has semantic implications
> when the order of arguments matters.
> 
> Restore the previous order (to agree with Solaris semantics) and add
> tests for both bcopy() and copyinto().
> 
> 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                             | 15 +++--
>  .../funcs/bcopy/tst.bcopy_arg_order.d         | 52 +++++++++++++++++
>  .../funcs/bcopy/tst.bcopy_arg_order.r         |  3 +
>  .../funcs/copyinto/tst.copyinto_arg_order.d   | 57 +++++++++++++++++++
>  .../funcs/copyinto/tst.copyinto_arg_order.r   |  5 ++
>  5 files changed, 128 insertions(+), 4 deletions(-)
>  create mode 100644 test/unittest/funcs/bcopy/tst.bcopy_arg_order.d
>  create mode 100644 test/unittest/funcs/bcopy/tst.bcopy_arg_order.r
>  create mode 100644 test/unittest/funcs/copyinto/tst.copyinto_arg_order.d
>  create mode 100644 test/unittest/funcs/copyinto/tst.copyinto_arg_order.r
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 53d969d5..44209a05 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -4514,18 +4514,25 @@ dt_cg_subr_bcopy_impl(dt_node_t *dnp, dt_node_t *dst, dt_node_t *src,
>  
>  	TRACE_REGSET("      subr-bcopy-impl:Begin");
>  
> -	/* Validate the size for the copy operation. */
> -	dt_cg_node(size, dlp, drp);
> +	/* Evaluate the arguments */
> +	if (is_bcopy) {
> +		dt_cg_node(src, dlp, drp);
> +		dt_cg_node(dst, dlp, drp);
> +		dt_cg_node(size, dlp, drp);
> +	} else {
> +		dt_cg_node(src, dlp, drp);
> +		dt_cg_node(size, dlp, drp);
> +		dt_cg_node(dst, dlp, drp);
> +	}
>  
> +	/* Validate the size for the copy operation. */
>  	emit(dlp,  BPF_BRANCH_IMM(BPF_JSLT, size->dn_reg, 0, lbl_badsize));
>  	emit(dlp,  BPF_BRANCH_IMM(BPF_JGT, size->dn_reg, maxsize, lbl_badsize));
>  
>  	/* Validate the source pointer. */
> -	dt_cg_node(src, dlp, drp);
>  	dt_cg_check_ptr_arg(dlp, drp, src, size);
>  
>  	/* Validate the destination pointer. */
> -	dt_cg_node(dst, dlp, drp);
>  	if (!(dst->dn_flags & DT_NF_ALLOCA))
>  		dnerror(dst, D_PROTO_ARG,
>  			"%s( ) argument #%d is incompatible with prototype:\n"
> diff --git a/test/unittest/funcs/bcopy/tst.bcopy_arg_order.d b/test/unittest/funcs/bcopy/tst.bcopy_arg_order.d
> new file mode 100644
> index 00000000..4c213931
> --- /dev/null
> +++ b/test/unittest/funcs/bcopy/tst.bcopy_arg_order.d
> @@ -0,0 +1,52 @@
> +/*
> + * 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.
> + */
> +
> +/*
> + * ASSERTION: Arguments are evaluated in the correct order.
> + *
> + * SECTION: Actions and Subroutines/bcopy()
> + */
> +
> +#pragma D option quiet
> +
> +BEGIN
> +{
> +	src = (char *)alloca(8);
> +	dst = (char *)alloca(8);
> +
> +	/* initialize src */
> +	src[ 0] = 'a'; src[ 1] = 'b'; src[ 2] = 'c'; src[ 3] = 'd';
> +	src[ 4] = 'e'; src[ 5] = 'f'; src[ 6] = 'g'; src[ 7] = 'h';
> +
> +	/* initialize dst with '.' */
> +	dst[ 0] = dst[ 1] = dst[ 2] = dst[ 3] =
> +	dst[ 4] = dst[ 5] = dst[ 6] = dst[ 7] = '.';
> +
> +	/* execute subroutine with order-dependent arguments */
> +	n = 2;
> +	bcopy((void*)(src + (n++)), (void*)(dst + (n++)), (n++));
> +
> +	/* print results */
> +	printf("%c%c%c%c%c%c%c%c\n",
> +	    dst[ 0], dst[ 1], dst[ 2], dst[ 3],
> +	    dst[ 4], dst[ 5], dst[ 6], dst[ 7]);
> +
> +	/* repeat all that but with order-independent arguments */
> +	dst[ 0] = dst[ 1] = dst[ 2] = dst[ 3] =
> +	dst[ 4] = dst[ 5] = dst[ 6] = dst[ 7] = '.';
> +	bcopy((void*)(src + 2), (void*)(dst + 3), 4);
> +	printf("%c%c%c%c%c%c%c%c\n",
> +	    dst[ 0], dst[ 1], dst[ 2], dst[ 3],
> +	    dst[ 4], dst[ 5], dst[ 6], dst[ 7]);
> +
> +	exit(0);
> +}
> +
> +ERROR
> +{
> +	exit(1);
> +}
> diff --git a/test/unittest/funcs/bcopy/tst.bcopy_arg_order.r b/test/unittest/funcs/bcopy/tst.bcopy_arg_order.r
> new file mode 100644
> index 00000000..11db2dd4
> --- /dev/null
> +++ b/test/unittest/funcs/bcopy/tst.bcopy_arg_order.r
> @@ -0,0 +1,3 @@
> +...cdef.
> +...cdef.
> +
> diff --git a/test/unittest/funcs/copyinto/tst.copyinto_arg_order.d b/test/unittest/funcs/copyinto/tst.copyinto_arg_order.d
> new file mode 100644
> index 00000000..a5a7b504
> --- /dev/null
> +++ b/test/unittest/funcs/copyinto/tst.copyinto_arg_order.d
> @@ -0,0 +1,57 @@
> +/*
> + * 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.
> + */
> +
> +/*
> + * ASSERTION: Arguments are evaluated in the correct order.
> + *
> + * SECTION: Actions and Subroutines/copyinto()
> + */
> +/* @@trigger: delaydie */
> +
> +#pragma D option quiet
> +
> +syscall::write:entry
> +/pid == $target/
> +{
> +	ptr = (char *)alloca(16);
> +
> +	/* initialize with '.' */
> +	ptr[ 0] = ptr[ 1] = ptr[ 2] = ptr[ 3] =
> +	ptr[ 4] = ptr[ 5] = ptr[ 6] = ptr[ 7] =
> +	ptr[ 8] = ptr[ 9] = ptr[10] = ptr[11] =
> +	ptr[12] = ptr[13] = ptr[14] = ptr[15] = '.';
> +
> +	/* execute subroutine with order-dependent arguments */
> +	n = 2;
> +	copyinto(arg1 + (n++), (n++), (void*)(ptr + (n++)));
> +
> +	/* print results */
> +	printf("%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c\n",
> +	    ptr[ 0], ptr[ 1], ptr[ 2], ptr[ 3],
> +	    ptr[ 4], ptr[ 5], ptr[ 6], ptr[ 7],
> +	    ptr[ 8], ptr[ 9], ptr[10], ptr[11],
> +	    ptr[12], ptr[13], ptr[14], ptr[15]);
> +
> +	/* repeat all that but with order-independent arguments */
> +	ptr[ 0] = ptr[ 1] = ptr[ 2] = ptr[ 3] =
> +	ptr[ 4] = ptr[ 5] = ptr[ 6] = ptr[ 7] =
> +	ptr[ 8] = ptr[ 9] = ptr[10] = ptr[11] =
> +	ptr[12] = ptr[13] = ptr[14] = ptr[15] = '.';
> +	copyinto(arg1 + 2, 3, (void *)(ptr + 4));
> +	printf("%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c\n",
> +	    ptr[ 0], ptr[ 1], ptr[ 2], ptr[ 3],
> +	    ptr[ 4], ptr[ 5], ptr[ 6], ptr[ 7],
> +	    ptr[ 8], ptr[ 9], ptr[10], ptr[11],
> +	    ptr[12], ptr[13], ptr[14], ptr[15]);
> +
> +	exit(0);
> +}
> +
> +ERROR
> +{
> +	exit(1);
> +}
> diff --git a/test/unittest/funcs/copyinto/tst.copyinto_arg_order.r b/test/unittest/funcs/copyinto/tst.copyinto_arg_order.r
> new file mode 100644
> index 00000000..e2bd6295
> --- /dev/null
> +++ b/test/unittest/funcs/copyinto/tst.copyinto_arg_order.r
> @@ -0,0 +1,5 @@
> +....lay.........
> +....lay.........
> +
> +-- @@stderr --
> +Delay in ns needed in delay env var.
> -- 
> 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