Re: [PATCH v1] perf test: Fixes for check branch stack sampling

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [Medium] Incorrect regex in `test_user_branches()` renders the test a false negative for leaked kernel addresses.
- [Low] Contradictory test output in `test_syscall()` when skipped.
- [Low] Missing closing bracket in the failure log output of `test_kernel_branches()`.
--

commit 5cc9b2c1d3de6813cdf5dae014b695e4edf82d5d
Author: Ian Rogers <irogers@google.com>

perf test: Fixes for check branch stack sampling

This commit updates the branch stack sampling tests to filter cross-boundary
branches properly and increases the sampling duration to make syscall
sampling more reliable. It also redirects unnecessary test output to
temporary files to unclutter the logs and improves the pass/fail reporting.

> diff --git a/tools/perf/tests/shell/test_brstack.sh b/tools/perf/tests/shell/test_brstack.sh
> index 85233d435be63..d317ed0e063ef 100755
> --- a/tools/perf/tests/shell/test_brstack.sh
> +++ b/tools/perf/tests/shell/test_brstack.sh
[ ... ]
> @@ -73,59 +75,81 @@ test_user_branches() {
>  	perf script -i "$TMPDIR/perf.data" --fields brstack | \
>  		tr ' ' '\n' > "$TMPDIR/perf.script"
>  
> -	# There should be no kernel addresses with the u option, in either
> -	# source or target addresses.
> -	if grep -E -m1 "0x[89a-f][0-9a-f]{15}" $TMPDIR/perf.script; then
> -		echo "ERROR: Kernel address found in user mode"
> +	# There should be no kernel addresses in the target with the u option.
> +	if grep -q -E -m1 "^0x[0-9a-f]{16}/[89a-f][0-9a-f]{15}/" $TMPDIR/perf.script; then

The perf script output normally prefixes addresses with 0x, but this regex
appears to expect the target address immediately after the slash to lack a
0x prefix.

It also requires the source address to be exactly 16 hex digits, but user
space addresses often lack zero-padding and look like 0x400000.

Does this regex fail to match these output formats, preventing it from
detecting leaked kernel addresses in the test?

> +		echo "Testing user branch stack sampling [Failed kernel address found in user mode]"
>  		err=1
>  	fi
[ ... ]
>  test_kernel_branches() {
>  	echo "Testing that k option only includes kernel source addresses"
>  
[ ... ]
> +	# Look for no target addresses without top bit set
> +	if grep -q -E -m1 "^0x[0-9a-f]{0,16}/0x[0-7][0-9a-f]{0,15}/" $TMPDIR/perf.script; then
> +		echo "Testing that k option [Failed user branches found with kernel filter"

This isn't a bug, but there is a missing closing ] bracket at the end of the
failure string.

> +		err=1
> +	fi
[ ... ]
>  test_syscall() {
>  	echo "Testing syscalls"
> +	start_err=$err
> +	err=0
>  	# skip if perf doesn't have enough privileges
> -	if ! perf record --branch-filter any,k -o- -- true > /dev/null; then
> -		echo "skip: not enough privileges"
> +	if ! perf record --branch-filter any,k -o- -- true > "$TMPDIR/record.txt" 2>&1; then
> +		echo "Testing syscalls [Skipped: not enough privileges]"
>  	else
>  		perf record -o $TMPDIR/perf.data --branch-filter \
> -			any_call,save_type,u,k -c 10000 -- \
> -			perf bench syscall basic --loop 1000
> +			any_call,save_type,u,k -c 10007 -- \
> +			perf bench syscall basic --loop 8000  > "$TMPDIR/record.txt" 2>&1
>  		perf script -i $TMPDIR/perf.data --fields brstacksym | \
>  			tr ' ' '\n' > $TMPDIR/perf.script
>  
>  		check_branches "getppid[^ ]*/SYSCALL/"
>  	fi
> +	if [ $err -eq 0 ]; then
> +		echo "Testing syscalls [Passed]"

Unlike test_kernel_branches(), there is no early return after the skip
message.

If perf record lacks privileges, will the test execution fall through to here
and incorrectly print that the test passed immediately after printing that
it was skipped?

> +		err=$start_err
> +	else
> +		echo "Testing syscalls [Failed]"
> +	fi
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260408060303.51231-1-irogers@google.com?part=1