[DTrace-devel] [PATCH 1/4] Clean up dtrace::: arg handling and testing
Kris Van Hees
kris.van.hees at oracle.com
Wed May 4 05:27:40 UTC 2022
On Wed, May 04, 2022 at 01:01:41AM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> Currently, the dtrace provider trampoline sets probe arguments to the
> values being passed in some function call. However:
>
> *) The BEGIN and END probes have no arguments defined. This
> point is documented explicitly. The probe arguments are
> certainly not the arguments passed into internal functions
> BEGIN_probe() and END_probe() -- no such arguments are passed!
>
> *) The ERROR probe does have defined arguments, but they are
> set up in clause code, not in the trampoline.
>
> Therefore, do not copy probe args in the dtrace-provider trampoline.
The legacy implementation has always populated the argument data from
the registers used for argument passing evne when no such arguments were
actually supported for the probe. The current implementation remains true to
that way of doing things and it is harmless.
The ERROR probe has its own trampoline code because it gets inlined in the
probe BPF program. So the trampoline() function in the dtrace provider never
performs any argument setup anyway for ERROR probes.
So, I would argue that we should continue to copy the values from the argument
passing registers, if only to remain consistent with the legacy implementation.
> The following tests exercise BEGIN-probe arguments but do not check
> their values. This is meaningless but not explicitly forbidden.
> Arguably, the tst.arg*.d tests could be eliminated in favor of
> their tst.arg*clause.d cousins. For now, we leave these tests alone.
>
> demo/builtin/argX.d
> unittest/builtinvar/tst.arg0.d
> unittest/builtinvar/tst.arg1.d
> unittest/builtinvar/tst.arg1to8.d
>
> The following tests require argX!=-1 to pass. They date back to a
> period of development when built-in variables were not yet supported
> and were all set to -1. In general, however, this test is not reliable:
> there is no reason why an undefined variable cannot be -1. Support for
> various defined probe arguments is tested elsewhere. Remove these tests.
>
> unittest/variables/bvar/tst.arg[0-9].d
No, please leave these tests but certainly update them to no longer treat -1
as a special value. That is long overdue. But we still want to be able to
test that these builtin variables can be accessed.
> The following test was illegal in the first place and has never passed.
> Remove it.
>
> unittest/variables/bvar/tst.args.d
Well, no, let's rewrite it into a test that correctly tries to access the
arguments through the args[] array.
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> libdtrace/dt_prov_dtrace.c | 1 -
> test/unittest/variables/bvar/tst.arg0.d | 23 -----------------------
> test/unittest/variables/bvar/tst.arg1.d | 23 -----------------------
> test/unittest/variables/bvar/tst.arg2.d | 23 -----------------------
> test/unittest/variables/bvar/tst.arg3.d | 23 -----------------------
> test/unittest/variables/bvar/tst.arg4.d | 23 -----------------------
> test/unittest/variables/bvar/tst.arg5.d | 23 -----------------------
> test/unittest/variables/bvar/tst.arg6.d | 23 -----------------------
> test/unittest/variables/bvar/tst.arg7.d | 23 -----------------------
> test/unittest/variables/bvar/tst.arg8.d | 23 -----------------------
> test/unittest/variables/bvar/tst.arg9.d | 23 -----------------------
> test/unittest/variables/bvar/tst.args.d | 24 ------------------------
> 12 files changed, 255 deletions(-)
> delete mode 100644 test/unittest/variables/bvar/tst.arg0.d
> delete mode 100644 test/unittest/variables/bvar/tst.arg1.d
> delete mode 100644 test/unittest/variables/bvar/tst.arg2.d
> delete mode 100644 test/unittest/variables/bvar/tst.arg3.d
> delete mode 100644 test/unittest/variables/bvar/tst.arg4.d
> delete mode 100644 test/unittest/variables/bvar/tst.arg5.d
> delete mode 100644 test/unittest/variables/bvar/tst.arg6.d
> delete mode 100644 test/unittest/variables/bvar/tst.arg7.d
> delete mode 100644 test/unittest/variables/bvar/tst.arg8.d
> delete mode 100644 test/unittest/variables/bvar/tst.arg9.d
> delete mode 100644 test/unittest/variables/bvar/tst.args.d
>
> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> index a7fc2cf5..4d401574 100644
> --- a/libdtrace/dt_prov_dtrace.c
> +++ b/libdtrace/dt_prov_dtrace.c
> @@ -156,7 +156,6 @@ static void trampoline(dt_pcb_t *pcb)
> emit(dlp, BPF_CALL_HELPER(BPF_FUNC_map_update_elem));
>
> dt_cg_tramp_copy_regs(pcb, BPF_REG_8);
> - dt_cg_tramp_copy_args_from_regs(pcb, BPF_REG_8);
>
> dt_cg_tramp_epilogue_advance(pcb, act);
> }
> diff --git a/test/unittest/variables/bvar/tst.arg0.d b/test/unittest/variables/bvar/tst.arg0.d
> deleted file mode 100644
> index a745db0d..00000000
> --- a/test/unittest/variables/bvar/tst.arg0.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg0' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg0
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg0);
> - exit(arg0 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.arg1.d b/test/unittest/variables/bvar/tst.arg1.d
> deleted file mode 100644
> index 7d2cd65a..00000000
> --- a/test/unittest/variables/bvar/tst.arg1.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg1' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg1
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg1);
> - exit(arg1 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.arg2.d b/test/unittest/variables/bvar/tst.arg2.d
> deleted file mode 100644
> index 08cf98aa..00000000
> --- a/test/unittest/variables/bvar/tst.arg2.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg2' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg2
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg2);
> - exit(arg2 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.arg3.d b/test/unittest/variables/bvar/tst.arg3.d
> deleted file mode 100644
> index a7eea667..00000000
> --- a/test/unittest/variables/bvar/tst.arg3.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg3' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg3
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg3);
> - exit(arg3 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.arg4.d b/test/unittest/variables/bvar/tst.arg4.d
> deleted file mode 100644
> index defc09ce..00000000
> --- a/test/unittest/variables/bvar/tst.arg4.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg4' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg4
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg4);
> - exit(arg4 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.arg5.d b/test/unittest/variables/bvar/tst.arg5.d
> deleted file mode 100644
> index 48ae3f12..00000000
> --- a/test/unittest/variables/bvar/tst.arg5.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg5' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg5
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg5);
> - exit(arg5 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.arg6.d b/test/unittest/variables/bvar/tst.arg6.d
> deleted file mode 100644
> index cd77dff3..00000000
> --- a/test/unittest/variables/bvar/tst.arg6.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg6' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg6
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg6);
> - exit(arg6 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.arg7.d b/test/unittest/variables/bvar/tst.arg7.d
> deleted file mode 100644
> index 314fdb1f..00000000
> --- a/test/unittest/variables/bvar/tst.arg7.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg7' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg7
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg7);
> - exit(arg7 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.arg8.d b/test/unittest/variables/bvar/tst.arg8.d
> deleted file mode 100644
> index dc03db3b..00000000
> --- a/test/unittest/variables/bvar/tst.arg8.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg8' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg8
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg8);
> - exit(arg8 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.arg9.d b/test/unittest/variables/bvar/tst.arg9.d
> deleted file mode 100644
> index 4456bba0..00000000
> --- a/test/unittest/variables/bvar/tst.arg9.d
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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: The 'arg9' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/arg9
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(arg9);
> - exit(arg9 != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> diff --git a/test/unittest/variables/bvar/tst.args.d b/test/unittest/variables/bvar/tst.args.d
> deleted file mode 100644
> index 09536766..00000000
> --- a/test/unittest/variables/bvar/tst.args.d
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2020, 2021, 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 */
> -
> -/*
> - * ASSERTION: The 'args' variable can be accessed and is not -1.
> - *
> - * SECTION: Variables/Built-in Variables/args
> - */
> -
> -#pragma D option quiet
> -
> -BEGIN {
> - trace(args);
> - exit(args != -1 ? 0 : 1);
> -}
> -
> -ERROR {
> - exit(1);
> -}
> --
> 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