[DTrace-devel] [PATCH] Clean up fbtprovider tests
Eugene Loh
eugene.loh at oracle.com
Fri Oct 8 15:05:30 PDT 2021
On 10/8/21 3:45 PM, Kris Van Hees wrote:
> On Wed, Sep 15, 2021 at 08:39:34PM -0400, eugene.loh at oracle.com wrote:
>> From: Eugene Loh <eugene.loh at oracle.com>
>>
>> Specifically, add a futex trigger to the test suite and have tests
>> match the trigger pid rather than the dtrace pid.
> I have a question about this, especially for the last 2 tests... They are
> testing TLS variables and those are not implemented yet. Therefore, those
> tests passing either means that we got lucky somehow that the lack of actual
> implementation doesn't cause the test to fail or that the test premise itself
> it wrong.
I think that's kind of right. These two TLS tests aren't real
stringent; they exercise TLS with a single-threaded trigger! Meanwhile,
the TLS implementation is not 100% broken -- that is, smoke doesn't
start pouring out of the machine simply because one uses TLS. All that
happens is that we store and load from some table and, under
sufficiently gentle circumstances (like these tests), that's good enough.
> How about putting those two tests in their own patch along with whatever
> other changes ned to happen to make them xfail for now. That way we can at
> least get the first two in the tree.
Here are some suggestions, in diminishing order of (my) enthusiasm.
*) We just leave the patch as is. We have lots of tests that do not
100% exercise the functionality they test. Many tests "pass" so long as
the system does not explode when the subject feature is used. So, these
two tests passing is simply in that spirit. It is okay for these tests
to pass.
*) We "skip" the tests. They were not failing because TLS wasn't
completely implemented; they were failing for simple, unrelated reasons
that we are now aware of and can fix. We can try to make these tests
more stringent, but that would only make sense if TLS were working and
we could check the revised tests. Meanwhile, the tests don't do much
interesting.
*) We stick guardrails into DTrace for now so that if anyone tried to
use TLS the tool would complain and abort. That would flag other tests
that have the same problem (use TLS but do not fail). Arguably, we
should ensure that no TLS usage (user or test) should slip by.
>> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
>> ---
>> test/triggers/Build | 13 +-
>> test/triggers/futex.c | 152 ++++++++++++++++++++++
>> test/unittest/fbtprovider/tst.entry.d | 14 +-
>> test/unittest/fbtprovider/tst.entryargs.d | 49 ++++---
>> test/unittest/fbtprovider/tst.tls.d | 8 +-
>> test/unittest/fbtprovider/tst.tls2.d | 12 +-
>> 6 files changed, 206 insertions(+), 42 deletions(-)
>> create mode 100644 test/triggers/futex.c
>>
>> diff --git a/test/triggers/Build b/test/triggers/Build
>> index 54e1aff3..dcafa4a2 100644
>> --- a/test/triggers/Build
>> +++ b/test/triggers/Build
>> @@ -3,7 +3,14 @@
>> # Licensed under the Universal Permissive License v 1.0 as shown at
>> # http://oss.oracle.com/licenses/upl.
>>
>> -EXTERNAL_64BIT_TRIGGERS = testprobe readwholedir mmap bogus-ioctl open delaydie pid-tst-args1 pid-tst-float pid-tst-fork pid-tst-gcc pid-tst-ret1 pid-tst-ret2 pid-tst-vfork pid-tst-weak1 pid-tst-weak2 proc-tst-sigwait proc-tst-omp proc-tst-pthread-exec profile-tst-ufuncsort raise-tst-raise1 raise-tst-raise2 raise-tst-raise3 syscall-tst-args ustack-tst-basic ustack-tst-bigstack ustack-tst-bigstack-spin ustack-tst-spin ustack-tst-mtspin visible-constructor visible-constructor-static visible-constructor-static-unstripped
>> +EXTERNAL_64BIT_TRIGGERS = testprobe readwholedir mmap bogus-ioctl open delaydie futex \
>> + pid-tst-args1 pid-tst-float pid-tst-fork pid-tst-gcc \
>> + pid-tst-ret1 pid-tst-ret2 pid-tst-vfork pid-tst-weak1 pid-tst-weak2 \
>> + proc-tst-sigwait proc-tst-omp proc-tst-pthread-exec profile-tst-ufuncsort \
>> + raise-tst-raise1 raise-tst-raise2 raise-tst-raise3 syscall-tst-args \
>> + ustack-tst-basic ustack-tst-bigstack ustack-tst-bigstack-spin \
>> + ustack-tst-spin ustack-tst-mtspin \
>> + visible-constructor visible-constructor-static visible-constructor-static-unstripped
>>
>> EXTERNAL_64BIT_SDT_TRIGGERS = usdt-tst-argmap usdt-tst-args usdt-tst-forker usdt-tst-special
>> EXTERNAL_64BIT_TRIGGERS += $(EXTERNAL_64BIT_SDT_TRIGGERS)
>> @@ -11,7 +18,9 @@ EXTERNAL_64BIT_TRIGGERS += $(EXTERNAL_64BIT_SDT_TRIGGERS)
>> EXTERNAL_32BIT_TRIGGERS := visible-constructor-32
>> EXTERNAL_TRIGGERS = $(EXTERNAL_64BIT_TRIGGERS) $(if $(NATIVE_BITNESS_ONLY),,$(EXTERNAL_32BIT_TRIGGERS))
>>
>> -INTERNAL_64BIT_TRIGGERS = libproc-pldd libproc-consistency libproc-sleeper libproc-sleeper-pie libproc-dlmadopen libproc-lookup-by-name libproc-lookup-victim libproc-execing-bkpts libproc-execing-bkpts-victim
>> +INTERNAL_64BIT_TRIGGERS = libproc-pldd libproc-consistency libproc-sleeper \
>> + libproc-sleeper-pie libproc-dlmadopen libproc-lookup-by-name \
>> + libproc-lookup-victim libproc-execing-bkpts libproc-execing-bkpts-victim
>> INTERNAL_32BIT_TRIGGERS := libproc-sleeper-32 libproc-sleeper-pie-32
>> INTERNAL_TRIGGERS = $(INTERNAL_64BIT_TRIGGERS) $(if $(NATIVE_BITNESS_ONLY),,$(INTERNAL_32BIT_TRIGGERS))
>>
>> diff --git a/test/triggers/futex.c b/test/triggers/futex.c
>> new file mode 100644
>> index 00000000..a9f3611a
>> --- /dev/null
>> +++ b/test/triggers/futex.c
>> @@ -0,0 +1,152 @@
>> +/*
>> + * THIS PROGRAM IS ADAPTED FROM THE futex MAN PAGE.
>> + *
>> + * futex_demo.c
>> + *
>> + * Usage: futex_demo [nloops] (Default: 5)
>> + *
>> + * Demonstrate the use of futexes in a program where parent and child
>> + * use a pair of futexes located inside a shared anonymous mapping to
>> + * synchronize access to a shared resource: the terminal. The two
>> + * processes each write 'num-loops' messages to the terminal and employ
>> + * a synchronization protocol that ensures that they alternate in
>> + * writing messages.
>> + */
>> +/* #define _GNU_SOURCE */
>> +#include <stdio.h>
>> +#include <errno.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include <sys/wait.h>
>> +#include <sys/mman.h>
>> +#include <sys/syscall.h>
>> +#include <linux/futex.h>
>> +#include <sys/time.h>
>> +
>> +#define errExit(msg) do { perror(msg); exit(EXIT_FAILURE); \
>> + } while (0)
>> +
>> +static int *futex1, *futex2, *iaddr;
>> +
>> +static int
>> +futex(int *uaddr, int futex_op, int val,
>> + const struct timespec *timeout, int *uaddr2, int val3)
>> +{
>> + return syscall(SYS_futex, uaddr, futex_op, val, timeout, uaddr, val3);
>> +}
>> +
>> +/*
>> + * Acquire the futex pointed to by 'futexp': wait for its value to
>> + * become 1, and then set the value to 0.
>> + */
>> +
>> +static void
>> +fwait(int *futexp)
>> +{
>> + int s;
>> +
>> + /*
>> + * __sync_bool_compare_and_swap(ptr, oldval, newval) is a gcc
>> + * built-in function. It atomically performs the equivalent of:
>> + *
>> + * if (*ptr == oldval)
>> + * *ptr = newval;
>> + *
>> + * It returns true if the test yielded true and *ptr was updated.
>> + * The alternative here would be to employ the equivalent atomic
>> + * machine-language instructions. For further information, see
>> + * the GCC Manual.
>> + */
>> +
>> + while (1) {
>> +
>> + /* Is the futex available? */
>> +
>> + if (__sync_bool_compare_and_swap(futexp, 1, 0))
>> + break; /* Yes */
>> +
>> + /* Futex is not available; wait */
>> +
>> + s = futex(futexp, FUTEX_WAIT, 0, NULL, NULL, 0);
>> + if (s == -1 && errno != EAGAIN)
>> + errExit("futex-FUTEX_WAIT");
>> + }
>> +}
>> +
>> +/*
>> + * Release the futex pointed to by 'futexp': if the futex currently
>> + * has the value 0, set its value to 1 and the wake any futex waiters,
>> + * so that if the peer is blocked in fpost(), it can proceed.
>> + **/
>> +
>> +static void
>> +fpost(int *futexp)
>> +{
>> + int s;
>> +
>> + /* __sync_bool_compare_and_swap() was described in comments above */
>> +
>> + if (__sync_bool_compare_and_swap(futexp, 0, 1)) {
>> +
>> + s = futex(futexp, FUTEX_WAKE, 1, NULL, NULL, 0);
>> + if (s == -1)
>> + errExit("futex-FUTEX_WAKE");
>> + }
>> +}
>> +
>> +int
>> +main(int argc, char *argv[])
>> +{
>> + pid_t childPid;
>> + int j, nloops;
>> +
>> + setbuf(stdout, NULL);
>> +
>> + nloops = (argc > 1) ? atoi(argv[1]) : 5;
>> +
>> + /*
>> + * Create a shared anonymous mapping that will hold the futexes.
>> + * Since the futexes are being shared between processes, we
>> + * subsequently use the "shared" futex operations (i.e., not the
>> + * ones suffixed "_PRIVATE").
>> + */
>> +
>> + iaddr = mmap(NULL, sizeof(int) * 2, PROT_READ | PROT_WRITE,
>> + MAP_ANONYMOUS | MAP_SHARED, -1, 0);
>> + if (iaddr == MAP_FAILED)
>> + errExit("mmap");
>> +
>> + futex1 = &iaddr[0];
>> + futex2 = &iaddr[1];
>> +
>> + *futex1 = 0; /* State: unavailable */
>> + *futex2 = 1; /* State: available */
>> +
>> + /* Create a child process that inherits the shared anonymous mapping. */
>> +
>> + childPid = fork();
>> + if (childPid == -1)
>> + errExit("fork");
>> +
>> + if (childPid == 0) { /* Child */
>> + for (j = 0; j < nloops; j++) {
>> + fwait(futex1);
>> + printf("Child (%ld) %d\n", (long) getpid(), j);
>> + fpost(futex2);
>> + }
>> +
>> + exit(EXIT_SUCCESS);
>> + }
>> +
>> + /* Parent falls through to here */
>> +
>> + for (j = 0; j < nloops; j++) {
>> + fwait(futex2);
>> + printf("Parent (%ld) %d\n", (long) getpid(), j);
>> + fpost(futex1);
>> + }
>> +
>> + wait(NULL);
>> +
>> + exit(EXIT_SUCCESS);
>> +}
>> diff --git a/test/unittest/fbtprovider/tst.entry.d b/test/unittest/fbtprovider/tst.entry.d
>> index 0a88d15c..25fc6794 100644
>> --- a/test/unittest/fbtprovider/tst.entry.d
>> +++ b/test/unittest/fbtprovider/tst.entry.d
>> @@ -1,6 +1,6 @@
>> /*
>> * Oracle Linux DTrace.
>> - * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
>> + * Copyright (c) 2006, 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.
>> */
>> @@ -13,32 +13,32 @@
>> * SECTION: FBT Provider/Probes
>> */
>>
>> -/* @@xfail: dtv2 */
>> /* @@runtest-opts: -Z */
>> +/* @@trigger: futex */
>>
>> #pragma D option quiet
>> #pragma D option statusrate=10ms
>>
>> BEGIN
>> {
>> - me = pid;
>> num_entry = 0;
>> }
>>
>> syscall::futex:entry
>> -/me == pid/
>> +/pid == $target/
>> {
>> num_entry++;
>> }
>>
>> fbt::SyS_futex:entry,
>> -fbt::__x64_sys_futex:entry
>> -/me == pid && num_entry > 0/
>> +fbt::__x64_sys_futex:entry,
>> +fbt::__arm64_sys_futex:entry
>> +/pid == $target && num_entry > 0/
>> {
>> }
>>
>> syscall::futex:return
>> -/me == pid && num_entry > 0/
>> +/pid == $target && num_entry > 0/
>> {
>> exit(0);
>> }
>> diff --git a/test/unittest/fbtprovider/tst.entryargs.d b/test/unittest/fbtprovider/tst.entryargs.d
>> index a20f9352..fccdd1be 100644
>> --- a/test/unittest/fbtprovider/tst.entryargs.d
>> +++ b/test/unittest/fbtprovider/tst.entryargs.d
>> @@ -1,6 +1,6 @@
>> /*
>> * Oracle Linux DTrace.
>> - * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
>> + * Copyright (c) 2006, 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.
>> */
>> @@ -11,20 +11,19 @@
>> * SECTION: FBT Provider/Probes
>> */
>>
>> -/* @@xfail: dtv2 */
>> /* @@runtest-opts: -Z */
>> +/* @@trigger: futex */
>>
>> #pragma D option quiet
>> #pragma D option statusrate=10ms
>>
>> BEGIN
>> {
>> - me = pid;
>> num_entry = 0;
>> }
>>
>> syscall::futex:entry
>> -/me == pid/
>> +/pid == $target/
>> {
>> num_entry++;
>> self->arg0 = arg0;
>> @@ -36,63 +35,71 @@ syscall::futex:entry
>> }
>>
>> syscall::futex:entry
>> -/me == pid && num_entry > 0/
>> +/pid == $target && num_entry > 0/
>> {
>> printf("args %x %x %x %x %x %x\n", arg0, arg1, arg2, arg3, arg4, arg5);
>> }
>>
>> fbt::SyS_futex:entry,
>> -fbt::__x64_sys_futex:entry
>> -/me == pid && num_entry > 0/
>> +fbt::__x64_sys_futex:entry,
>> +fbt::__arm64_sys_futex:entry
>> +/pid == $target && num_entry > 0/
>> {
>> printf("args %x %x %x %x %x %x\n", arg0, arg1, arg2, arg3, arg4, arg5);
>> }
>>
>> fbt::SyS_futex:entry,
>> -fbt::__x64_sys_futex:entry
>> -/me == pid && num_entry > 0 && self->arg0 != arg0/
>> +fbt::__x64_sys_futex:entry,
>> +fbt::__arm64_sys_futex:entry
>> +/pid == $target && num_entry > 0 && self->arg0 != arg0/
>> {
>> printf("arg0 mismatch: %x vs %x\n", self->arg0, arg0);
>> }
>>
>> fbt::SyS_futex:entry,
>> -fbt::__x64_sys_futex:entry
>> -/me == pid && num_entry > 0 && self->arg1 != arg1/
>> +fbt::__x64_sys_futex:entry,
>> +fbt::__arm64_sys_futex:entry
>> +/pid == $target && num_entry > 0 && self->arg1 != arg1/
>> {
>> printf("arg1 mismatch: %x vs %x\n", self->arg1, arg1);
>> }
>>
>> fbt::SyS_futex:entry,
>> -fbt::__x64_sys_futex:entry
>> -/me == pid && num_entry > 0 && self->arg2 != arg2/
>> +fbt::__x64_sys_futex:entry,
>> +fbt::__arm64_sys_futex:entry
>> +/pid == $target && num_entry > 0 && self->arg2 != arg2/
>> {
>> printf("arg2 mismatch: %x vs %x\n", self->arg2, arg2);
>> }
>>
>> fbt::SyS_futex:entry,
>> -fbt::__x64_sys_futex:entry
>> -/me == pid && num_entry > 0 && self->arg3 != arg3/
>> +fbt::__x64_sys_futex:entry,
>> +fbt::__arm64_sys_futex:entry
>> +/pid == $target && num_entry > 0 && self->arg3 != arg3/
>> {
>> printf("arg3 mismatch: %x vs %x\n", self->arg3, arg3);
>> }
>>
>> fbt::SyS_futex:entry,
>> -fbt::__x64_sys_futex:entry
>> -/me == pid && num_entry > 0 && self->arg4 != arg4/
>> +fbt::__x64_sys_futex:entry,
>> +fbt::__arm64_sys_futex:entry
>> +/pid == $target && num_entry > 0 && self->arg4 != arg4/
>> {
>> printf("arg4 mismatch: %x vs %x\n", self->arg4, arg4);
>> }
>>
>> fbt::SyS_futex:entry,
>> -fbt::__x64_sys_futex:entry
>> -/me == pid && num_entry > 0 && self->arg5 != arg5/
>> +fbt::__x64_sys_futex:entry,
>> +fbt::__arm64_sys_futex:entry
>> +/pid == $target && num_entry > 0 && self->arg5 != arg5/
>> {
>> printf("arg5 mismatch: %x vs %x\n", self->arg5, arg5);
>> }
>>
>> fbt::SyS_ioctl:return,
>> -fbt::__x64_sys_ioctl:return
>> -/me == pid && num_entry > 0/
>> +fbt::__x64_sys_ioctl:return,
>> +fbt::__arm64_sys_ioctl:return
>> +/pid == $target && num_entry > 0/
>> {
>> exit(0);
>> }
>> diff --git a/test/unittest/fbtprovider/tst.tls.d b/test/unittest/fbtprovider/tst.tls.d
>> index dd57f0c6..31f6a5dd 100644
>> --- a/test/unittest/fbtprovider/tst.tls.d
>> +++ b/test/unittest/fbtprovider/tst.tls.d
>> @@ -11,7 +11,6 @@
>> * SECTION: FBT Provider/Probes
>> */
>>
>> -/* @@xfail: dtv2 */
>> /* @@runtest-opts: -Z */
>> /* @@trigger: pid-tst-args1 */
>>
>> @@ -20,7 +19,6 @@
>>
>> BEGIN
>> {
>> - me = pid;
>> num_entry = 0;
>> num_return = 0;
>> fails = 0;
>> @@ -29,7 +27,7 @@ BEGIN
>> fbt::SyS_ioctl:entry,
>> fbt::__arm64_sys_ioctl:entry,
>> fbt::__x64_sys_ioctl:entry
>> -/me == pid/
>> +/pid == $target/
>> {
>> num_entry++;
>> self->token = pid;
>> @@ -38,7 +36,7 @@ fbt::__x64_sys_ioctl:entry
>> fbt::SyS_ioctl:return,
>> fbt::__arm64_sys_ioctl:return,
>> fbt::__x64_sys_ioctl:return
>> -/me == pid && num_entry > 0/
>> +/pid == $target && num_entry > 0/
>> {
>> num_return++;
>> }
>> @@ -46,7 +44,7 @@ fbt::__x64_sys_ioctl:return
>> fbt::SyS_ioctl:return,
>> fbt::__arm64_sys_ioctl:return,
>> fbt::__x64_sys_ioctl:return
>> -/me == pid && num_entry > 0 && self->token != pid/
>> +/pid == $target && num_entry > 0 && self->token != pid/
>> {
>> fails++;
>> }
>> diff --git a/test/unittest/fbtprovider/tst.tls2.d b/test/unittest/fbtprovider/tst.tls2.d
>> index e8c44eab..88b02299 100644
>> --- a/test/unittest/fbtprovider/tst.tls2.d
>> +++ b/test/unittest/fbtprovider/tst.tls2.d
>> @@ -11,7 +11,6 @@
>> * SECTION: FBT Provider/Probes
>> */
>>
>> -/* @@xfail: dtv2 */
>> /* @@runtest-opts: -Z */
>> /* @@trigger: pid-tst-args1 */
>>
>> @@ -20,14 +19,13 @@
>>
>> BEGIN
>> {
>> - me = pid;
>> num_entry = 0;
>> num_return = 0;
>> fails = 0;
>> }
>>
>> syscall::ioctl:entry
>> -/me == pid/
>> +/pid == $target/
>> {
>> num_entry++;
>> self->token = pid;
>> @@ -36,27 +34,27 @@ syscall::ioctl:entry
>> fbt::SyS_ioctl:entry,
>> fbt::__arm64_sys_ioctl:entry,
>> fbt::__x64_sys_ioctl:entry
>> -/me == pid && num_entry > 0/
>> +/pid == $target && num_entry > 0/
>> {
>> }
>>
>> fbt::SyS_ioctl:return,
>> fbt::__arm64_sys_ioctl:return,
>> fbt::__x64_sys_ioctl:return
>> -/me == pid && num_entry > 0/
>> +/pid == $target && num_entry > 0/
>> {
>> }
>>
>> fbt::SyS_ioctl:return,
>> fbt::__arm64_sys_ioctl:return,
>> fbt::__x64_sys_ioctl:return
>> -/me == pid && num_entry > 0 && self->token != pid/
>> +/pid == $target && num_entry > 0 && self->token != pid/
>> {
>> fails++;
>> }
>>
>> syscall::ioctl:return
>> -/me == pid && num_entry > 0/
>> +/pid == $target && num_entry > 0/
>> {
>> num_return++;
>> }
>> --
>> 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