Re: [PATCH v17 5/5] perf test: Add inject ASLR test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The `test_regs_stripping` test contains broken logic that causes it to unconditionally pass, resulting in a false negative.
- [Medium] User space ASLR report tests omit raw hex address filtering, causing flaky test failures.
- [Medium] The kernel ASLR test is vulnerable to race conditions on high-performance systems due to an extremely brief workload, causing flaky test failures.
- [Low] Test harness artifacts are overwritten by subsequent tests upon failure, impeding debugging.
--

commit 130aed1f94b88ff586372e491e562e4121d94067
Author: Ian Rogers <irogers@google.com>

perf test: Add inject ASLR test

This commit adds a new shell test suite for the perf inject --aslr feature.
It provides coverage for basic user space sample remapping, pipe mode,
callchain remapping, dropped samples leaking physical addresses, and kernel
address remapping tests.

diff --git a/tools/perf/tests/shell/inject_aslr.sh b/tools/perf/tests/shell/inject_aslr.sh
new file mode 100755
index 0000000000000..db54975082599
--- /dev/null
+++ b/tools/perf/tests/shell/inject_aslr.sh

[ ... ]

> +prog="perf test -w noploop"
> +[ "$(uname -m)" = "s390x" ] && prog="$prog 3"
> +err=0
> +kprog="dd if=/dev/zero of=/dev/null bs=1M count=500"

[Severity: Medium]
Will this workload complete too quickly on high-performance systems?

On modern machines, a 500MB transfer might complete in a few milliseconds.
At default sampling frequencies, this extremely short duration could yield
zero kernel samples, causing test_kernel_aslr and test_kernel_report_aslr
to spuriously fail since they assert kernel samples are present.

Is it possible to increase the transfer size or use a different workload
to ensure enough samples are gathered?

[ ... ]

> +test_report_aslr() {
> +  echo "Test perf report consistency"
> +  local data
> +  data=$(mktemp "${temp_dir}/perf.data.report.XXXXXX")
> +  local data2
> +  data2=$(mktemp "${temp_dir}/perf.data2.report.XXXXXX")
> +  local data_clean
> +  data_clean=$(mktemp "${temp_dir}/perf.data.clean.XXXXXX")
> +
> +  perf record -e task-clock:u -o "${data}" ${prog}
> +  # Use -b to inject build-ids and force ordered events processing in both
> +  perf inject -b -i "${data}" -o "${data_clean}"
> +  perf inject -v -b --aslr -i "${data}" -o "${data2}"
> +
> +  local report1="${temp_dir}/report1"
> +  local report2="${temp_dir}/report2"
> +  local report1_clean="${temp_dir}/report1.clean"
> +  local report2_clean="${temp_dir}/report2.clean"
> +  local diff_file="${temp_dir}/diff"

[Severity: Low]
Will these fixed filenames cause debugging issues if multiple tests fail?

Since ${temp_dir} is shared globally across the test script, these paths
evaluate to the same files in test_report_aslr, test_pipe_report_aslr,
and test_pipe_out_report_aslr.

Because the test script accumulates failures with err=1 and continues
execution, passing tests will silently overwrite the artifacts of earlier
failing tests.

Should these names include the test function name to preserve artifacts
properly for debugging?

> +
> +  perf report -i "${data_clean}" --stdio > "${report1}"
> +  perf report -i "${data2}" --stdio > "${report2}"
> +
> +  # Strip headers and compare lines with percentages
> +  grep '%' "${report1}" | grep -v '^#' | sort > "${report1_clean}" || true
> +  grep '%' "${report2}" | grep -v '^#' | sort > "${report2_clean}" || true
> +
> +  diff -u -w "${report1_clean}" "${report2_clean}" > "${diff_file}" || true

[Severity: Medium]
Does this output comparison need raw hex address filtering to prevent flaky
test failures?

If unresolved symbols are present in user space samples (such as [vdso] or
libc), perf report will output their raw hex addresses. Because the --aslr
injection shifts these addresses, the text output differs, and the diff
will spuriously fail the test.

Other tests like test_kernel_report_aslr use a filter to handle this:
grep -v -E '0x[0-9a-f]{8,}|0000000000000000'

Would it make sense to apply that same filtering here, and similarly
in test_pipe_report_aslr and test_pipe_out_report_aslr?

[ ... ]

> +  local script_dump="${temp_dir}/script_regs_dump"
> +  perf script -D -i "${rdata2}" > "${script_dump}" 2>/dev/null || true
> +  if grep -q "PERF_SAMPLE_REGS_USER" "${script_dump}"; then
> +    echo "User registers stripping test [Failed - register dumps still present]"
> +    err=1
> +  else
> +    echo "User registers stripping test [Success]"
> +  fi
> +}

[Severity: Medium]
Will this grep condition ever evaluate to true?

The test checks for the literal string "PERF_SAMPLE_REGS_USER" in the
perf script -D output. However, the tool formats this flag natively as
"REGS_USER" and prints payloads as "... user regs: mask ...".

Because "PERF_SAMPLE_REGS_USER" is never printed, the grep command will
unconditionally fail. This causes the test to always report success, even
if the --aslr register stripping logic fails.

Could this be updated to match the actual output strings generated by perf?

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