Skip to content

Commit

Permalink
Restore order of bcopy() arg evaluations
Browse files Browse the repository at this point in the history
In c26e2e5 ("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@oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>
  • Loading branch information
euloh authored and kvanhees committed Feb 26, 2023
1 parent 8dc870f commit d122722
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 4 deletions.
15 changes: 11 additions & 4 deletions libdtrace/dt_cg.c
Original file line number Diff line number Diff line change
Expand Up @@ -4523,18 +4523,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"
Expand Down
52 changes: 52 additions & 0 deletions test/unittest/funcs/bcopy/tst.bcopy_arg_order.d
Original file line number Diff line number Diff line change
@@ -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);
}
3 changes: 3 additions & 0 deletions test/unittest/funcs/bcopy/tst.bcopy_arg_order.r
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
...cdef.
...cdef.

57 changes: 57 additions & 0 deletions test/unittest/funcs/copyinto/tst.copyinto_arg_order.d
Original file line number Diff line number Diff line change
@@ -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);
}
5 changes: 5 additions & 0 deletions test/unittest/funcs/copyinto/tst.copyinto_arg_order.r
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
....lay.........
....lay.........

-- @@stderr --
Delay in ns needed in delay env var.

0 comments on commit d122722

Please sign in to comment.