[DTrace-devel] [PATCH 3/4] Fix fbt:::return and pid:::return arg1
Kris Van Hees
kris.van.hees at oracle.com
Wed May 4 06:09:11 UTC 2022
On Wed, May 04, 2022 at 01:01:43AM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> Also, remove the unused PT_REGS_BPF_*().
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> libdtrace/dt_cg.c | 8 ++-
> libdtrace/dt_cg.h | 2 +-
> libdtrace/dt_prov_fbt.c | 9 ++-
> libdtrace/dt_prov_pid.c | 6 +-
> libdtrace/dt_pt_regs.h | 25 +------
> test/unittest/fbtprovider/tst.return0.d | 6 +-
> test/unittest/fbtprovider/tst.return_arg1.sh | 70 ++++++++++++++++++++
> test/unittest/pid/tst.ret1.d | 3 +-
> test/unittest/pid/tst.ret2.d | 3 +-
> 9 files changed, 95 insertions(+), 37 deletions(-)
> create mode 100755 test/unittest/fbtprovider/tst.return_arg1.sh
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 3c97a6c9..4a41c981 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -266,20 +266,22 @@ dt_cg_tramp_copy_regs(dt_pcb_t *pcb, int rp)
> }
>
> /*
> - * Copy arguments from a dt_pt_regs structure referenced by the 'rp' argument.
> + * Copy arguments for entry probes.
> + *
> + * A dt_pt_regs structure is referenced by the 'rp' argument.
> *
> * The caller must ensure that %r7 contains the value set by the
> * dt_cg_tramp_prologue*() functions.
> */
> void
> -dt_cg_tramp_copy_args_from_regs(dt_pcb_t *pcb, int rp)
> +dt_cg_tramp_copy_entry_args(dt_pcb_t *pcb, int rp)
Sorry, but no. This function exists specifically to copy the values passed
for a probe firing into the mstate. It does not matter whether this is for
entry probes or not. In fact, various providers do not use entry and return
probes. Let's not sacrifice the common case in favour of a specific case.
> {
> dt_irlist_t *dlp = &pcb->pcb_ir;
> int i, bpf_probe_read_func_id;
>
> /*
> * for (i = 0; i < PT_REGS_ARGC; i++)
> - * dctx->mst->argv[i] = PT_REGS_BPF_ARGi((dt_pt_regs *)rp);
> + * dctx->mst->argv[i] = PT_REGS_ARGi((dt_pt_regs *)rp);
> * // lddw %r0, [%rp + PT_REGS_ARGi]
> * // stdw [%r7 + DMST_ARG(i)], %r0
> */
> diff --git a/libdtrace/dt_cg.h b/libdtrace/dt_cg.h
> index 5752151b..57282b0b 100644
> --- a/libdtrace/dt_cg.h
> +++ b/libdtrace/dt_cg.h
> @@ -24,7 +24,7 @@ extern void dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act);
> extern void dt_cg_tramp_prologue(dt_pcb_t *pcb);
> extern void dt_cg_tramp_clear_regs(dt_pcb_t *pcb);
> extern void dt_cg_tramp_copy_regs(dt_pcb_t *pcb, int rp);
> -extern void dt_cg_tramp_copy_args_from_regs(dt_pcb_t *pcb, int rp);
> +extern void dt_cg_tramp_copy_entry_args(dt_pcb_t *pcb, int rp);
> extern void dt_cg_tramp_call_clauses(dt_pcb_t *pcb, const dt_probe_t *prp,
> dt_activity_t act);
> extern void dt_cg_tramp_return(dt_pcb_t *pcb);
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 62fea51e..88638bc6 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -167,9 +167,14 @@ static void trampoline(dt_pcb_t *pcb)
> * // (%r7 = dctx->mst)
> * // (%r8 = dctx->ctx)
> */
> -
> dt_cg_tramp_copy_regs(pcb, BPF_REG_8);
> - dt_cg_tramp_copy_args_from_regs(pcb, BPF_REG_8);
> + if (strcmp(pcb->pcb_probe->desc->prb, "return") == 0) {
> + dt_irlist_t *dlp = &pcb->pcb_ir;
> +
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, PT_REGS_RET));
> + emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(1), BPF_REG_0));
How about putting this in a dt_cg_tramp_copy_ret_from_regs() function where you
copy this one value and then zero the remaining arguments in the mstate? The
way you do it now arg1 through arg9 never get cleared.
> + } else
> + dt_cg_tramp_copy_entry_args(pcb, BPF_REG_8);
> dt_cg_tramp_epilogue(pcb);
> }
>
> diff --git a/libdtrace/dt_prov_pid.c b/libdtrace/dt_prov_pid.c
> index af894f79..725e8cde 100644
> --- a/libdtrace/dt_prov_pid.c
> +++ b/libdtrace/dt_prov_pid.c
> @@ -211,7 +211,11 @@ static void trampoline(dt_pcb_t *pcb)
> */
>
> dt_cg_tramp_copy_regs(pcb, BPF_REG_8);
> - dt_cg_tramp_copy_args_from_regs(pcb, BPF_REG_8);
> + if (strcmp(pcb->pcb_probe->desc->prb, "return") == 0) {
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, PT_REGS_RET));
> + emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(1), BPF_REG_0));
> + } else
> + dt_cg_tramp_copy_entry_args(pcb, BPF_REG_8);
>
> /*
> * Retrieve the PID of the process that caused the probe to fire.
> diff --git a/libdtrace/dt_pt_regs.h b/libdtrace/dt_pt_regs.h
> index 5f851573..057f1fdb 100644
> --- a/libdtrace/dt_pt_regs.h
> +++ b/libdtrace/dt_pt_regs.h
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2019, 2022, 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.
> */
> @@ -19,7 +19,7 @@ extern "C" {
>
> #if defined(__amd64)
> typedef struct pt_regs dt_pt_regs;
> -
> +# define PT_REGS_RET offsetof(dt_pt_regs, rax)
> # define PT_REGS_ARG0 offsetof(dt_pt_regs, rdi)
> # define PT_REGS_ARG1 offsetof(dt_pt_regs, rsi)
> # define PT_REGS_ARG2 offsetof(dt_pt_regs, rdx)
> @@ -30,17 +30,9 @@ typedef struct pt_regs dt_pt_regs;
> # define PT_REGS_ARGSTKBASE 1
> # define PT_REGS_IP offsetof(dt_pt_regs, rip)
> # define PT_REGS_SP offsetof(dt_pt_regs, rsp)
> -
> -# define PT_REGS_BPF_ARG0(r) ((r)->rdi)
> -# define PT_REGS_BPF_ARG1(r) ((r)->rsi)
> -# define PT_REGS_BPF_ARG2(r) ((r)->rdx)
> -# define PT_REGS_BPF_ARG3(r) ((r)->rcx)
> -# define PT_REGS_BPF_ARG4(r) ((r)->r8)
> -# define PT_REGS_BPF_ARG5(r) ((r)->r9)
> -# define PT_REGS_BPF_IP(r) ((r)->rip)
> -# define PT_REGS_BPF_SP(r) ((r)->rsp)
> #elif defined(__aarch64__)
> typedef struct user_pt_regs dt_pt_regs;
> +# define PT_REGS_RET offsetof(dt_pt_regs, regs[0])
> # define PT_REGS_ARG0 offsetof(dt_pt_regs, regs[0])
> # define PT_REGS_ARG1 offsetof(dt_pt_regs, regs[1])
> # define PT_REGS_ARG2 offsetof(dt_pt_regs, regs[2])
> @@ -53,17 +45,6 @@ typedef struct user_pt_regs dt_pt_regs;
> # define PT_REGS_ARGSTKBASE 0
> # define PT_REGS_IP offsetof(dt_pt_regs, pc)
> # define PT_REGS_SP offsetof(dt_pt_regs, sp)
> -
> -# define PT_REGS_BPF_ARG0(r) ((r)->regs[0])
> -# define PT_REGS_BPF_ARG1(r) ((r)->regs[1])
> -# define PT_REGS_BPF_ARG2(r) ((r)->regs[2])
> -# define PT_REGS_BPF_ARG3(r) ((r)->regs[3])
> -# define PT_REGS_BPF_ARG4(r) ((r)->regs[4])
> -# define PT_REGS_BPF_ARG5(r) ((r)->regs[5])
> -# define PT_REGS_BPF_ARG6(r) ((r)->regs[6])
> -# define PT_REGS_BPF_ARG7(r) ((r)->regs[7])
> -# define PT_REGS_BPF_IP(r) ((r)->pc)
> -# define PT_REGS_BPF_SP(r) ((r)->sp)
> #else
> # error ISA not supported
> #endif
> diff --git a/test/unittest/fbtprovider/tst.return0.d b/test/unittest/fbtprovider/tst.return0.d
> index 3d58e84f..0b7091b3 100644
> --- a/test/unittest/fbtprovider/tst.return0.d
> +++ b/test/unittest/fbtprovider/tst.return0.d
> @@ -1,18 +1,16 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2021, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2022, 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: simple fbt provider arg0 and probfunc print test.
> + * ASSERTION: simple fbt provider arg0 and probefunc print test.
> *
> * SECTION: FBT Provider/Probe arguments
> */
>
> -/* @@runtest-opts: -Z */
> -
> #pragma D option quiet
> #pragma D option statusrate=10ms
>
> diff --git a/test/unittest/fbtprovider/tst.return_arg1.sh b/test/unittest/fbtprovider/tst.return_arg1.sh
> new file mode 100755
> index 00000000..bb8342a8
> --- /dev/null
> +++ b/test/unittest/fbtprovider/tst.return_arg1.sh
> @@ -0,0 +1,70 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2022, 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/fbt.return_arg1.$$.$RANDOM"
> +mkdir -p $DIRNAME
> +cd $DIRNAME
> +
> +cat << EOF > main.c
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#define N 7
> +int main(int c, char **v) {
> + int i, fds[N];
> + for (i = 0; i < N; i++)
> + printf("%d\n", fds[i] = open("/dev/zero", O_RDONLY));
> + for (; i--; )
> + close(fds[i]);
> + return 0;
> +}
> +EOF
> +
> +/usr/bin/gcc main.c
> +if [ $? -ne 0 ]; then
> + echo "compilation failure"
> + exit 1
> +fi
> +
> +$dtrace $dt_flags -c ./a.out -o out.D -qZn '
> +fbt:vmlinux:do_sys_open*:return
> +/pid == $target/
> +{
> + printf("%d\n", arg1);
> +}' > out.C 2>&1
> +
> +if [ $? -ne 0 ]; then
> + echo "DTrace failure"
> + cat out.C
> + cat out.D
> + exit 1
> +fi
> +
> +# multiple fbt::do_sys_open*:return probes might fire per open()
> +if [ `awk 'NF' out.D | uniq -c | awk '{print $1}' | uniq | wc -1` -ne 1 ]; then
> + echo "inconsistent numbers of probes firing"
> + cat out.C
> + cat out.D
> + exit 1
> +fi
> +
> +awk 'NF' out.D | uniq > out.cmp
> +diff -q out.cmp out.C
> +
> +if [ $? -ne 0 ]; then
> + echo "diff failure"
> + cat out.C
> + cat out.D
> + diff out.cmp out.C
> + exit 1
> +fi
> +
> +exit 0
> diff --git a/test/unittest/pid/tst.ret1.d b/test/unittest/pid/tst.ret1.d
> index bdf7d274..398e2cc8 100644
> --- a/test/unittest/pid/tst.ret1.d
> +++ b/test/unittest/pid/tst.ret1.d
> @@ -1,10 +1,9 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2022, 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.
> */
> -/* @@xfail: dtv2 */
> /* @@runtest-opts: $_pid */
> /* @@trigger: pid-tst-ret1 */
> /* @@trigger-timing: before */
> diff --git a/test/unittest/pid/tst.ret2.d b/test/unittest/pid/tst.ret2.d
> index d3d482e3..a542646a 100644
> --- a/test/unittest/pid/tst.ret2.d
> +++ b/test/unittest/pid/tst.ret2.d
> @@ -1,10 +1,9 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2022, 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.
> */
> -/* @@xfail: dtv2 */
> /* @@runtest-opts: $_pid */
> /* @@trigger: pid-tst-ret2 */
> /* @@trigger-timing: before */
> --
> 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