public inbox for dtrace@lists.linux.dev
 help / color / mirror / Atom feed
From: Kris Van Hees <kris.van.hees@oracle.com>
To: eugene.loh@oracle.com
Cc: dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [PATCH 20/22] test: Annotate some XFAILs
Date: Sat, 14 Sep 2024 14:29:28 -0400	[thread overview]
Message-ID: <ZuXWCAex2rOYfrOI@oracle.com> (raw)
In-Reply-To: <20240829052219.3234-20-eugene.loh@oracle.com>

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
> 

  reply	other threads:[~2024-09-14 18:29 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-29  5:21 [PATCH 01/22] test: Handle dtrace:::ERROR arg3 specially eugene.loh
2024-08-29  5:21 ` [PATCH 02/22] test: Clean up tests still expecting obsolete "at DIF offset NN" eugene.loh
2024-08-29  5:22 ` [PATCH 03/22] Action clear() should clear only one aggregation eugene.loh
2024-09-06 21:43   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 04/22] Remove unused "next" arg from dt_flowindent() eugene.loh
2024-09-06 22:01   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 05/22] Set the ERROR PRID in BPF code eugene.loh
2024-09-07  0:20   ` Kris Van Hees
2024-09-07  1:25     ` Eugene Loh
2024-09-07  2:03       ` Kris Van Hees
2024-09-12 20:33         ` Kris Van Hees
2024-09-13 17:21           ` Eugene Loh
2024-08-29  5:22 ` [PATCH 06/22] Fix provider lookup to use prv not prb eugene.loh
2024-09-13 20:01   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 07/22] Supply a default probe_info() eugene.loh
2024-09-14  0:31   ` Kris Van Hees
2024-09-14  1:59     ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 08/22] dtprobed: Fix comment typo eugene.loh
2024-09-14 15:41   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 09/22] Clean up dtsd_* members eugene.loh
2024-09-14 15:40   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 10/22] Simplify dtrace_stmt_create() attr init eugene.loh
2024-09-14 16:25   ` Kris Van Hees
2024-09-14 16:32     ` [DTrace-devel] " Kris Van Hees
2024-09-19 17:38       ` Eugene Loh
2024-09-19 17:42         ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 11/22] DTPPT_POST_OFFSETS is unused eugene.loh
2024-09-14 16:35   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 12/22] Remove apparently redundant assignment eugene.loh
2024-09-14 16:37   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 13/22] Eliminate unused args to dt_spec_buf_add_data() eugene.loh
2024-09-14 17:06   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 14/22] Both dted_uarg and dofe_uarg are unused eugene.loh
2024-09-14 17:08   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 15/22] test: Clean up the specsize tests eugene.loh
2024-09-14 17:57   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 16/22] test: Fix the speculative tests that checked bufsize eugene.loh
2024-09-14 18:00   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 17/22] test: Tweak spec sizes to bracket size jumps more narrowly eugene.loh
2024-09-14 18:07   ` Kris Van Hees
2024-09-17 18:05     ` Eugene Loh
2024-08-29  5:22 ` [PATCH 18/22] test: Remove tst.DTRACEFLT_BADADDR2.d dependency on specific PC eugene.loh
2024-09-14 18:10   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 19/22] test: Fix tst.probestar.d trigger eugene.loh
2024-09-14 18:13   ` Kris Van Hees
2024-10-17 22:53     ` Eugene Loh
2024-08-29  5:22 ` [PATCH 20/22] test: Annotate some XFAILs eugene.loh
2024-09-14 18:29   ` Kris Van Hees [this message]
2024-08-29  5:22 ` [PATCH 21/22] test: Fix DIRNAME eugene.loh
2024-08-29 20:25   ` [DTrace-devel] " Sam James
2024-09-14 18:43   ` Kris Van Hees
2024-08-29  5:22 ` [PATCH 22/22] test: Update tst.newprobes.sh xfail message eugene.loh
2024-09-14 18:45   ` Kris Van Hees
2024-08-29 15:57 ` [PATCH 01/22] test: Handle dtrace:::ERROR arg3 specially Kris Van Hees

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZuXWCAex2rOYfrOI@oracle.com \
    --to=kris.van.hees@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    --cc=eugene.loh@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox