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

[Eugene Loh]

The cg part of the patch looks good, other than that the 
dt_regset_free_args() should be moved to right after the function call.  
I note that below.

This patch needs a positive test that checks correct operation of the 
new feature.  Here is a possibility:



#!/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];
         sprintf(s, "hello world");
         write(fd, s, strlen(s));
         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/
{
         copyout(strjoin("HELLO ", "WORLD"), arg1, 256);
         exit(0);
}' -c ./a.out

if [ $? -ne 0 ]; then
         echo "DTrace error"
         cat $tmpfile
         exit 1
fi

exit 0



And then the .r file should have HELLO WORLD.

And then perhaps the copyout tests should probably be in their own func/ 
subdirectory, as is done for copyin.

Why is copyoutstr() not part of this patch?  Is it just the extra work 
of figuring out the string length?

The test test/unittest/funcs/err.copyoutstrbadaddr.sh uses copyout().  
It should use copyoutstr().  (Yes, that problem predates this patch.)

This patch should remove the XFAIL on test/unittest/dif/copyout.d.

On 1/13/23 14:43, Kris Van Hees via DTrace-devel wrote:
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> +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");
> +
> +	/* Validate the source pointer. */
> +	dt_cg_node(src, dlp, drp);
> +	dt_cg_check_ptr_arg(dlp, drp, src);
> +
> +	/* Validate the destination pointer. */
> +	dt_cg_node(dst, dlp, drp);
> +	dt_cg_check_notnull(dlp, drp, dst->dn_reg);
> +
> +	/* Size for the copy operation. */
> +	dt_cg_node(size, dlp, drp);
> +
> +	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));
> +
> +	/*
> +	 * 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_regset_free_args(drp);

The dt_regset_free_args() should be moved to right after the 
BPF_CALL_HELPER().

> +	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  ");
> +}