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

Kris Van Hees
2024-08-30T03:25:02Z

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
>