Re: [PATCH 20/22] test: Annotate some XFAILs

Kris Van Hees
2024-09-14T18:29:39Z

Partial
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

I am applying this for all tests
EXCEPT test/unittest/lquantize/tst.32bit-bug26268136.sh

That test is supposed to exercise a reported bug concerning overflow. But
the test commit message is being updated to indicate that the test does pass
with BEGIN or proc:::start as trigger probe, but fails with proc:::exit.
That would mean that the test verifies correctly that the bug has been fixed.
But it also indicates that there is another (almost certainly unrelated) bug
that causes bad code to be generated if the trigger probe is proc:::exit.

So, this test should be adjusted to use a trigger probe that works and a *NEW*
test should be added to exercise the problem that is being seen with the
proc:::exit probe.

There, I'm applying the patch changes to all tests except that one, and a new
patch should be submitteed for review to address the issue with the one test.

On Thu, Aug 29, 2024 at 01:22:17AM -0400, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
>
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
> test/unittest/assocs/tst.invalidref.d | 2 +-
> test/unittest/builtinvar/tst.ipl.d | 2 +-
> test/unittest/builtinvar/tst.ipl1.d | 2 +-
> test/unittest/builtinvar/tst.vtimestamp.d | 2 +-
> test/unittest/builtinvar/tst.vtimestamp2.d | 2 +-
> .../lquantize/tst.32bit-bug26268136.sh | 29 ++++++++++++++++++-
> .../printa/err.D_PRINTF_ARG_TYPE.jstack.d | 2 +-
> test/unittest/printa/tst.jstack.d | 2 +-
> .../tst.jstack_unprintable-bug26045010.sh | 2 +-
> test/unittest/variables/bvar/tst.ipl.d | 2 +-
> test/unittest/variables/bvar/tst.vtimestamp.d | 2 +-
> 11 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/test/unittest/assocs/tst.invalidref.d b/test/unittest/assocs/tst.invalidref.d
> index 0ed17959..34367c6b 100644
> --- a/test/unittest/assocs/tst.invalidref.d
> +++ b/test/unittest/assocs/tst.invalidref.d
> @@ -4,7 +4,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
> +/* @@xfail: dtv2 no support for index of assoc array; last_cmds[1][3]=0 dumps BPF */
>
> /*
> * Test to ensure that invalid stores to a global associative array
> diff --git a/test/unittest/builtinvar/tst.ipl.d b/test/unittest/builtinvar/tst.ipl.d
> index c2b9850f..41a89b00 100644
> --- a/test/unittest/builtinvar/tst.ipl.d
> +++ b/test/unittest/builtinvar/tst.ipl.d
> @@ -4,7 +4,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
> +/* @@xfail: dtv2: need ipl support */
>
> /*
> * ASSERTION:
> diff --git a/test/unittest/builtinvar/tst.ipl1.d b/test/unittest/builtinvar/tst.ipl1.d
> index 1b489229..654b16ce 100644
> --- a/test/unittest/builtinvar/tst.ipl1.d
> +++ b/test/unittest/builtinvar/tst.ipl1.d
> @@ -4,7 +4,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
> +/* @@xfail: dtv2: need ipl support */
>
> /*
> * ASSERTION:
> diff --git a/test/unittest/builtinvar/tst.vtimestamp.d b/test/unittest/builtinvar/tst.vtimestamp.d
> index c9e2a112..11ffae7b 100644
> --- a/test/unittest/builtinvar/tst.vtimestamp.d
> +++ b/test/unittest/builtinvar/tst.vtimestamp.d
> @@ -4,7 +4,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
> +/* @@xfail: dtv2: need vtimestamp support */
>
> /*
> * ASSERTION:
> diff --git a/test/unittest/builtinvar/tst.vtimestamp2.d b/test/unittest/builtinvar/tst.vtimestamp2.d
> index 087a11e3..68a57ea8 100644
> --- a/test/unittest/builtinvar/tst.vtimestamp2.d
> +++ b/test/unittest/builtinvar/tst.vtimestamp2.d
> @@ -4,7 +4,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
> +/* @@xfail: dtv2: need vtimestamp support */
>
> /*
> * ASSERTION:
> diff --git a/test/unittest/lquantize/tst.32bit-bug26268136.sh b/test/unittest/lquantize/tst.32bit-bug26268136.sh
> index d5f143f5..91f4b65c 100755
> --- a/test/unittest/lquantize/tst.32bit-bug26268136.sh
> +++ b/test/unittest/lquantize/tst.32bit-bug26268136.sh
> @@ -5,7 +5,34 @@
> # Licensed under the Universal Permissive License v 1.0 as shown at
> # http://oss.oracle.com/licenses/upl.
> #
> -# @@xfail: dtv2
> +# @@xfail: dtv2 since BPF verifier fails, see test source code
> +
> +# This test passes with BEGIN or even proc:::start but BPF verifier fails with proc:::exit.
> +#
> +# When the BPF verifier dives into
> +# uint64_t *dt_get_agg(const dt_dctx_t *dctx, uint32_t id, const char *key, uint64_t ival, const char *dflt) {
> +# uint64_t *genp;
> +# uint64_t *valp;
> +#
> +# /* get the gen value */
> +# genp = bpf_map_lookup_elem(&agggen, &id);
> +# if (genp == 0)
> +# return dt_no_agg();
> +#
> +# /* place the variable ID at the beginning of the key */
> +# *(uint32_t *)key = id;
> +#
> +# /* try to look up the key */
> +# valp = bpf_map_lookup_elem(dctx->agg, key);
> +# it tries to look up the key. For the first argument (dctx->agg, where DCTX_AGG==64),
> +# the BPF verifier shows
> +# (79) r1 = *(u64 *)(r8 +64) R1= invP(id=0)
> +# Since nothing is known about the first argument, the BPF verifier complains about
> +# bpf_map_lookup_elem():
> +# R1 type=inv expected=map_ptr
> +# In contrast, with proc:::start, we get
> +# (79) r1 = *(u64 *)(r8 +64) R1= map_ptr(id=0,off=0,ks=12,vs=56,imm=0)
> +# and the BPF verifier is then happy with the bpf_map_lookup_elem() call.
>
> if [ $# != 1 ]; then
> echo expected one argument: '<'dtrace-path'>'
> diff --git a/test/unittest/printa/err.D_PRINTF_ARG_TYPE.jstack.d b/test/unittest/printa/err.D_PRINTF_ARG_TYPE.jstack.d
> index f04f9778..d2fccff7 100644
> --- a/test/unittest/printa/err.D_PRINTF_ARG_TYPE.jstack.d
> +++ b/test/unittest/printa/err.D_PRINTF_ARG_TYPE.jstack.d
> @@ -4,7 +4,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
> +/* @@xfail: dtv2 jstack not implemented */
> BEGIN
> {
> @[jstack()] = count();
> diff --git a/test/unittest/printa/tst.jstack.d b/test/unittest/printa/tst.jstack.d
> index 4ef1ed58..7a404a09 100644
> --- a/test/unittest/printa/tst.jstack.d
> +++ b/test/unittest/printa/tst.jstack.d
> @@ -4,7 +4,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
> +/* @@xfail: dtv2 jstack not implemented */
>
> BEGIN
> {
> diff --git a/test/unittest/ustack/tst.jstack_unprintable-bug26045010.sh b/test/unittest/ustack/tst.jstack_unprintable-bug26045010.sh
> index c5fca2a5..e6d8b43c 100755
> --- a/test/unittest/ustack/tst.jstack_unprintable-bug26045010.sh
> +++ b/test/unittest/ustack/tst.jstack_unprintable-bug26045010.sh
> @@ -5,7 +5,7 @@
> # Licensed under the Universal Permissive License v 1.0 as shown at
> # http://oss.oracle.com/licenses/upl.
> #
> -# @@xfail: dtv2
> +# @@xfail: dtv2 jstack not implemented
> if [ $# != 1 ]; then
> echo expected one argument: '<'dtrace-path'>'
> exit 2
> diff --git a/test/unittest/variables/bvar/tst.ipl.d b/test/unittest/variables/bvar/tst.ipl.d
> index a61e3b5b..7e2a432f 100644
> --- a/test/unittest/variables/bvar/tst.ipl.d
> +++ b/test/unittest/variables/bvar/tst.ipl.d
> @@ -4,7 +4,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
> +/* @@xfail: dtv2 need ipl support */
>
> /*
> * ASSERTION: The 'ipl' variable can be accessed and is not -1.
> diff --git a/test/unittest/variables/bvar/tst.vtimestamp.d b/test/unittest/variables/bvar/tst.vtimestamp.d
> index dc985ad1..b72eb8a7 100644
> --- a/test/unittest/variables/bvar/tst.vtimestamp.d
> +++ b/test/unittest/variables/bvar/tst.vtimestamp.d
> @@ -4,7 +4,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
> +/* @@xfail: dtv2 need vtimestamp support */
>
> /*
> * ASSERTION: The 'vtimestamp' variable can be accessed and is not -1.
> --
> 2.43.5
>