Re: [PATCH V2 linux-next] perf test: Skip failing 'perf test aslr' test case

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Commit message contradicts script behavior regarding test skip exit code, hiding skipped status from the test runner.
--

commit 0ec9bd8ca22ab0ece501382558440bfde5e6fdd0
Author: Thomas Richter <tmricht@linux.ibm.com>

perf test: Skip failing 'perf test aslr' test case

This commit addresses a failure in the 'perf test aslr' test case on s390
due to missing dwarf call-graph support in perf inject. It modifies
the script to drop the stack user sample and emit a skip message instead of
failing.

> diff --git a/tools/perf/tests/shell/inject_aslr.sh b/tools/perf/tests/shell/inject_aslr.sh
> index c00461828ea79..d83c2591db8f6 100755
> --- a/tools/perf/tests/shell/inject_aslr.sh
> +++ b/tools/perf/tests/shell/inject_aslr.sh
> @@ -135,8 +135,14 @@ test_callchain_aslr() {
>      echo "Callchain ASLR test [Failed - no noploop samples in original file]"
>      err=1
>    elif [ -z "$new_addr" ]; then
> -    echo "Callchain ASLR test [Failed - could not find remapped address]"
> -    err=1
> +    if perf evlist -v -i "${data}" | grep -q 'sample_type:.*STACK_USER'; then
> +      echo "Dropping stack user sample as possible ASLR leak"
> +      echo "Call-graph dwarf not supported with 'perf inject --aslr'"
> +      echo "Callchain ASLR test [Skip]"

[Severity: Medium]
The commit message states "Since one subtest is skipped, the complete test is
reported as skipped." However, it appears err is not set to 2 (the standard skip
code) here, meaning the script will exit with 0 (Ok) rather than being reported
as skipped by the test runner.

If setting err=2 was intentionally avoided because the script's cleanup
function currently assumes any non-zero exit code is a failure and leaves
temporary directories behind, would it be better to redesign the error
handling to support returning 2 without leaking temp files, or should the
commit message be corrected to reflect the actual exit status?

> +    else
> +      echo "Callchain ASLR test [Failed - could not find remapped address]"
> +      err=1
> +    fi
>    elif [ "$orig_addr" = "$new_addr" ]; then
>      echo "Callchain ASLR test [Failed - addresses are not remapped]"
>      err=1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629110559.1283453-1-tmricht@linux.ibm.com?part=1