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

[Namhyung Kim <namhyung@kernel.org>]

Hello,

On Mon, Jun 22, 2026 at 11:17:41AM -0700, Ian Rogers wrote:
> On Fri, Jun 19, 2026 at 2:42 AM <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [Medium] Unconditionally setting `err=2` when skipping `test_callchain_aslr` masks prior test failures.
> > - [Medium] Skipping the test incorrectly triggers the test failure cleanup path, leaking the temporary directory in `/tmp/`.
> > --
> >
> > commit d772895777a1677561d1802ac157c32ce73e1379
> > Author: Thomas Richter <tmricht@linux.ibm.com>
> >
> > perf test: Skip failing 'perf test aslr' test case
> >
> > The test case 'perf test aslr' fails on s390. This commit fixes the failure
> > by skipping the subtest test_callchain_aslr and emitting a hint when the
> > --call-graph dwarf option is selected.
> >
> > Fixes: 60bc9fbc0589 ("perf test: Add inject ASLR test")

Shouldn't it be 190c45463844?

> >
> > > diff --git a/tools/perf/tests/shell/inject_aslr.sh b/tools/perf/tests/shell/inject_aslr.sh
> > > index c00461828ea79..7fdb43679dc84 100755
> > > --- a/tools/perf/tests/shell/inject_aslr.sh
> > > +++ b/tools/perf/tests/shell/inject_aslr.sh
> > > @@ -135,8 +135,15 @@ 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]"
> > > +      err=2
> >
> > [Severity: Medium]
> > Does unconditionally setting err=2 here mask prior test failures?
> >
> > If an earlier subtest fails, the global err variable is set to 1. When
> > test_callchain_aslr() decides to skip and assigns err=2, it overwrites
> > this previous failure state.
> >
> > Since the perf test framework treats an exit code of 2 as a SKIP, the test
> > suite will report the entire script as skipped, completely hiding the
> > actual failure from the prior subtest.

Right, it should not overwrite if it's already 1.

Thanks,
Namhyng

> >
> > [Severity: Medium]
> > Will skipping this test inadvertently trigger the test failure cleanup path
> > and leak the temporary directory?
> >
> > The cleanup() function in tools/perf/tests/shell/inject_aslr.sh explicitly
> > aborts deletion if the exit code or err is non-zero:
> >
> > cleanup() {
> >   if [ "${exit_code}" -ne 0 ] || [ "${err}" -ne 0 ]; then
> 
> 
> Thanks for the fix Thomas! I completely agree with what you've done,
> but it exposed this latent issue that we should make the exit_code/err
> comparison -eq 1 rather than -ne 0. Perhaps Arnaldo can fix this, or
> we can address it as a follow-up.
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> 
> Thanks,
> Ian
> 
> >     echo "Test failed! Preserving temp directory: ${temp_dir}"
> >     return
> >   fi
> >   ...
> > }
> >
> > Because cleanup() was not updated to handle the new skip state, it
> > mistakenly treats err=2 as a test failure. This unconditionally leaves
> > behind the /tmp/perf-test-aslr.XXXXXX directory every time the test skips.
> >
> > > +    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/20260619093531.4038655-1-tmricht@linux.ibm.com?part=1
> >