Re: [PATCH v2 01/22] test: Handle dtrace:::ERROR arg3 specially

Kris Van Hees
2024-09-05T15:47:29Z

On Thu, Aug 29, 2024 at 11:34:53PM -0400, Kris Van Hees wrote:
> Attachment did not work apparently... Inlining...
>
> #!/bin/bash
> #
> # Oracle Linux DTrace.
> # Copyright (c) 2024, 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 ERROR arg3 value reports the correct PC value.
> #
> # SECTION: Variables/Built-in Variables/arg3
> ##
>
> dtrace=$1
>
> DIRNAME="$tmpdir/arg3-ERROR.$$.$RANDOM"
> mkdir -p $DIRNAME
> cd $DIRNAME
>
> $dtrace $dt_flags -xdisasm=8 -Sqs /dev/stdin << EOT &> D.out
> BEGIN
> {
> x = (int *)64;
> y = *x; /* 1st ERROR */
> }
>
> BEGIN
> {
> x = (int *)64;
> y = *x; /* 2nd ERROR */
> }
>
> BEGIN
> {
> x = (int *)64;
> y = *x; /* 3rd ERROR */
> }
>
> BEGIN
> {
> x = (int *)64;
> y = *x; /* 4th ERROR */
> }
>
> BEGIN
> {
> exit(0);
> }
> EOT
>
> awk '/call dt_probe_error/ {
> sites[int($1)] = 1;
> next;
> }
>
> /error on enabled probe/ {
> if (!($NF in sites)) {
> print;
> print " No call to dt_probe_error found at PC " $NF;
> } else
> ok++;

To deal with the potential case that arg3 would have a constnat value for all
instances that happens to point to a dt_probe_error call, change the else
portion to:

} else {
ok++;
delete sites[$NF];
}

which ensures that any given site can only match once.

> }
>
> END {
> if (ok != 4) {
> print "\nFound " ok " valid calls to dt_probe_error, expected 4";
> exit 1;
> }
>
> print "OK";
> exit 0;
> }' D.out
>
> rc=$?
>
> [ $rc -ne 0 ] && cat D.out
>
> rm -rf $DIRNAME
>
> exit $rc
>
> On Thu, Aug 29, 2024 at 11:24:53PM -0400, Kris Van Hees wrote:
> > How about something a bit simpler like the attached? We don't really need to
> > get too specific because as long as the reported PC in the ERROR reporting
> > matches a dt_probe_error call site, we know that the reported PC is valid.
> >
> > I think that would be sufficient as a test for the arg3 value.
> >
> > On Thu, Aug 29, 2024 at 06:18:19PM -0400, eugene.loh@oracle.com wrote:
> > > From: Eugene Loh <eugene.loh@oracle.com>
> > >
> > > The ERROR probe's arg3 reports the culprit PC, whose value can vary
> > > with minor implementation changes. On the one hand, we do not want
> > > tests to be overly sensitive to this value. On the other hand, we
> > > do want to check the value is correct.
> > >
> > > Therefore:
> > >
> > > *) Change tests that dump ERROR's args to omit arg3.
> > >
> > > *) Add a new test that checks that ERROR's arg3 is correct.
> > >
> > > Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> > > ---
> > > test/unittest/error/tst.DTRACEFLT_UNKNOWN.d | 6 +-
> > > test/unittest/error/tst.DTRACEFLT_UNKNOWN.r | 2 +-
> > > .../regression/tst.DTRACEFLT_BADADDR.d_path.d | 8 +-
> > > .../regression/tst.DTRACEFLT_BADADDR.d_path.r | 2 +-
> > > .../unittest/variables/bvar/tst.arg3-ERROR.sh | 172 ++++++++++++++++++
> > > 5 files changed, 181 insertions(+), 9 deletions(-)
> > > create mode 100755 test/unittest/variables/bvar/tst.arg3-ERROR.sh
> > >
> > > diff --git a/test/unittest/error/tst.DTRACEFLT_UNKNOWN.d b/test/unittest/error/tst.DTRACEFLT_UNKNOWN.d
> > > index 001903ff..bfc77bf5 100644
> > > --- a/test/unittest/error/tst.DTRACEFLT_UNKNOWN.d
> > > +++ b/test/unittest/error/tst.DTRACEFLT_UNKNOWN.d
> > > @@ -1,6 +1,6 @@
> > > /*
> > > * Oracle Linux DTrace.
> > > - * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
> > > + * Copyright (c) 2006, 2024, 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,8 +19,8 @@
> > >
> > > ERROR
> > > {
> > > - printf("The arguments are %u %u %u %u %u\n",
> > > - arg1, arg2, arg3, arg4, arg5);
> > > + printf("The arguments are %u %u PC %u %u\n",
> > > + arg1, arg2, arg4, arg5);
> > > printf("The value of arg4 = %u\n", DTRACEFLT_UNKNOWN);
> > > exit(0);
> > > }
> > > diff --git a/test/unittest/error/tst.DTRACEFLT_UNKNOWN.r b/test/unittest/error/tst.DTRACEFLT_UNKNOWN.r
> > > index b11f6c99..3e7caac4 100644
> > > --- a/test/unittest/error/tst.DTRACEFLT_UNKNOWN.r
> > > +++ b/test/unittest/error/tst.DTRACEFLT_UNKNOWN.r
> > > @@ -1,4 +1,4 @@
> > > -The arguments are 2 2 4 1 64
> > > +The arguments are 2 2 PC 1 64
> > > The value of arg4 = 0
> > >
> > > -- @@stderr --
> > > diff --git a/test/unittest/regression/tst.DTRACEFLT_BADADDR.d_path.d b/test/unittest/regression/tst.DTRACEFLT_BADADDR.d_path.d
> > > index c23f9503..ec519f4f 100644
> > > --- a/test/unittest/regression/tst.DTRACEFLT_BADADDR.d_path.d
> > > +++ b/test/unittest/regression/tst.DTRACEFLT_BADADDR.d_path.d
> > > @@ -1,10 +1,10 @@
> > > /*
> > > * Oracle Linux DTrace.
> > > - * Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
> > > + * Copyright (c) 2015, 2024, 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 */
> > > +/* @@xfail: dtv2 d_path */
> > >
> > > /*
> > > * ASSERTION:
> > > @@ -18,8 +18,7 @@
> > >
> > > ERROR
> > > {
> > > - printf("The arguments are %u %u %u %u %u\n",
> > > - arg1, arg2, arg3, arg4, arg5);
> > > + printf("The arguments are %u %u PC %u %u\n", arg1, arg2, arg4, arg5);
> > > printf("The value of arg4 should be %u\n", DTRACEFLT_BADADDR);
> > > printf("The value of arg5 should be %u\n", 0x18);
> > > exit(0);
> > > @@ -29,4 +28,5 @@ BEGIN
> > > {
> > > d = d_path((struct path *)0x18);
> > > trace(d);
> > > + exit(1);
> > > }
> > > diff --git a/test/unittest/regression/tst.DTRACEFLT_BADADDR.d_path.r b/test/unittest/regression/tst.DTRACEFLT_BADADDR.d_path.r
> > > index 8c601a43..be1f6d5b 100644
> > > --- a/test/unittest/regression/tst.DTRACEFLT_BADADDR.d_path.r
> > > +++ b/test/unittest/regression/tst.DTRACEFLT_BADADDR.d_path.r
> > > @@ -1,4 +1,4 @@
> > > -The arguments are 2 1 28 1 24
> > > +The arguments are 2 1 PC 1 24
> > > The value of arg4 should be 1
> > > The value of arg5 should be 24
> > >
> > > diff --git a/test/unittest/variables/bvar/tst.arg3-ERROR.sh b/test/unittest/variables/bvar/tst.arg3-ERROR.sh
> > > new file mode 100755
> > > index 00000000..4b139be2
> > > --- /dev/null
> > > +++ b/test/unittest/variables/bvar/tst.arg3-ERROR.sh
> > > @@ -0,0 +1,172 @@
> > > +#!/bin/bash
> > > +#
> > > +# Oracle Linux DTrace.
> > > +# Copyright (c) 2024, 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 in the ERROR probe is correct.
> > > +#
> > > +# SECTION: Variables/Built-in Variables/arg3
> > > +##
> > > +
> > > +dtrace=$1
> > > +
> > > +DIRNAME="$tmpdir/arg3-ERROR.$$.$RANDOM"
> > > +mkdir -p $DIRNAME
> > > +cd $DIRNAME
> > > +
> > > +function dump_files {
> > > + for x in $*; do
> > > + echo ==== $x
> > > + cat $x
> > > + done
> > > +}
> > > +
> > > +# Run DTrace, also collecting disassembly.
> > > +
> > > +$dtrace $dt_flags -o D.out -xdisasm=8 -S -qn '
> > > +BEGIN /* dt_clause_0 */
> > > +{
> > > + nerrs = 0;
> > > +}
> > > +
> > > +BEGIN /* dt_clause_1 */
> > > +{
> > > + x = (int *)64;
> > > + y = *x;
> > > + trace(y);
> > > +}
> > > +
> > > +BEGIN /* dt_clause_2 */
> > > +{
> > > + x = (int *)64;
> > > + y = *x;
> > > + trace(y);
> > > +}
> > > +
> > > +BEGIN /* dt_clause_3 */
> > > +{
> > > + x = (int *)64;
> > > + y = *x;
> > > + trace(y);
> > > +}
> > > +
> > > +BEGIN /* dt_clause_4 */
> > > +{
> > > + x = (int *)64;
> > > + y = *x;
> > > + trace(y);
> > > +}
> > > +
> > > +BEGIN /* dt_clause_5 */
> > > +{
> > > + exit(0);
> > > +}
> > > +
> > > +ERROR
> > > +{
> > > + /* Report the problematic PC and continue execution. */
> > > + printf("Error %d at %x\n", ++nerrs, arg3);
> > > +}
> > > +' >& disasm.out
> > > +if [ $? -ne 0 ]; then
> > > + echo DTrace failure
> > > + dump_files disasm.out
> > > + exit 1
> > > +fi
> > > +
> > > +# Parse the disassembly for start PCs for the dt_clause_n.
> > > +
> > > +awk '
> > > +BEGIN { phase = 0 }
> > > +
> > > +# Look for disassembly of dtrace:::BEGIN.
> > > +phase == 0 && /^Disassembly of final program dtrace:::BEGIN:$/ { phase = 1; next }
> > > +phase == 0 { next }
> > > +
> > > +# Look for a blank line, denoting the start of the variable table.
> > > +phase == 1 && NF == 0 { phase = 2; next }
> > > +phase == 1 { next }
> > > +
> > > +# Look for a blank line, denoting the start of the BPF relocations.
> > > +phase == 2 && NF == 0 { phase = 3; next }
> > > +phase == 2 { next }
> > > +
> > > +# Report PC for each "dt_clause_n" (or a blank line to finish).
> > > +phase == 3 && /dt_clause_/ { print $3, $4; next }
> > > +phase == 3 && NF == 0 { exit(0) }
> > > +phase == 3 { next }
> > > +' disasm.out > dt_clause_start_pcs.txt
> > > +if [ $? -ne 0 ]; then
> > > + echo ERROR: awk
> > > + dump_files disasm.out dt_clause_start_pcs.txt
> > > + exit 1
> > > +fi
> > > +
> > > +# Confirm that we found expected clauses 0-5.
> > > +
> > > +for n in 0 1 2 3 4 5; do
> > > + echo dt_clause_$n >> dt_clause_start_pcs.txt.check
> > > +done
> > > +if ! awk '{print $2}' dt_clause_start_pcs.txt | diff - dt_clause_start_pcs.txt.check; then
> > > + echo ERROR: did not find expected dt_clause_n
> > > + dump_files disasm.out dt_clause_start_pcs.txt
> > > +fi
> > > +
> > > +# Dump the error PCs to a file.
> > > +#
> > > +# BEGIN has 6 clauses (0-5), but 1-4 have the problematic instructions
> > > +# we are looking for: "y = *x". Each time, we check if address x is 0,
> > > +# calling dt_probe_error() if necessary. Then we try to dereference x,
> > > +# again calling dt_probe_error() if necessary. Since x==64, the second
> > > +# dt_probe_error() is the problematic one. We look for those calls.
> > > +for n in 1 2 3 4; do
> > > + # For dt_clause_$n, find the starting PC.
> > > + pc=`awk '$2 == "dt_clause_'$n'" { print $1 }' dt_clause_start_pcs.txt`
> > > +
> > > + # Look for the starting PC and then the second dt_probe_error().
> > > + awk '
> > > + BEGIN { phase = 0 }
> > > +
> > > + # Look for disassembly of dtrace:::BEGIN.
> > > + phase == 0 && /^Disassembly of final program dtrace:::BEGIN:$/ { phase = 1; next }
> > > + phase == 0 { next }
> > > +
> > > + # Look for the start PC of dt_clause_n.
> > > + phase == 1 && ($1 + 0) == '$pc' { phase = 2; next }
> > > + phase == 1 { next }
> > > +
> > > + # Look for the first dt_probe_error() call.
> > > + phase == 2 && /dt_probe_error/ { phase = 3; next }
> > > + phase == 2 { next }
> > > +
> > > + # Look for the second dt_probe_error() call, reporting the PC and quitting.
> > > + phase == 3 && /dt_probe_error/ { print $1 + 0; exit(0) }
> > > + phase == 3 { next }
> > > + ' disasm.out >> err_pcs.txt
> > > +done
> > > +
> > > +# Do a sanity check on DTrace's error output.
> > > +
> > > +awk '/^dtrace: error on enabled probe ID [0-9]* \(ID 1: dtrace:::BEGIN): invalid address \(0x40) in action #[0-9] at BPF pc [0-9]*$/ { print $NF }' \
> > > + disasm.out > err_pcs.txt.chk1
> > > +if ! diff -q err_pcs.txt err_pcs.txt.chk1; then
> > > + echo ERROR: problem with DTrace error output
> > > + dump_files disasm.out err_pcs.txt err_pcs.txt.chk1
> > > + exit 1
> > > +fi
> > > +
> > > +# Check the D script output... the arg3 values reported by the dtrace:::ERROR probe.
> > > +
> > > +if ! awk 'NF != 0 { print strtonum("0x"$NF) }' D.out | diff -q - err_pcs.txt; then
> > > + echo ERROR: D script output looks wrong
> > > + dump_files D.out err_pcs.txt
> > > + exit 1
> > > +fi
> > > +
> > > +echo success
> > > +
> > > +exit 0
> > > --
> > > 2.43.5
> > >