Re: [PATCH v9 9/9] perf test: Add Arm CoreSight callchain test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Rigid callchain regexes combined with instruction sampling artifacts will cause spurious test failures.
- [Medium] Checking for hardware event availability using `grep -Pzq` silently ties test coverage to PCRE support.
--

commit 533d8fca0c044c3396ca892c47d0539a4a5063b3
Author: Leo Yan <leo.yan@arm.com>

perf test: Add Arm CoreSight callchain test

Add a CoreSight shell test for synthesized callchains. The test uses the new
callchain workload to generate trace and decodes it with synthesis callchain.

> diff --git a/tools/perf/tests/shell/coresight/callchain.sh b/tools/perf/tests/shell/coresight/callchain.sh
> new file mode 100755
> index 0000000000000..13cca7dc11184
> --- /dev/null
> +++ b/tools/perf/tests/shell/coresight/callchain.sh
> @@ -0,0 +1,172 @@
[ ... ]
> +skip_if_system_is_not_ready()
> +{
> +	perf list | grep -Pzq 'cs_etm//' || {

[Severity: Medium]
Will this grep command fail on embedded systems where grep (like BusyBox)
might lack Perl-Compatible Regular Expression (-P) support?

If grep fails due to a lack of PCRE support, will this mistakenly report
"[Skip] cs_etm event is not available" and skip the test even when the
hardware is present? Should this use a standard POSIX grep -q instead, or
explicitly report a missing PCRE requirement?

> +		echo "[Skip] cs_etm event is not available" >&2
> +		return 2
> +	}
[ ... ]
> +check_callchain_flow()
> +{
> +	local file="$1"
> +	local l1 l2 l3 l4 l5 l6 l7
> +
> +	# Callchain push
> +	l1=$(find_after_line "$(callchain_regex_1)" "$file" 1) || return 1
> +	l2=$(find_after_line "$(callchain_regex_2)" "$file" "$((l1 + 1))") || return 1
> +	l3=$(find_after_line "$(callchain_regex_3)" "$file" "$((l2 + 1))") || return 1
> +	l4=$(find_after_line "$(callchain_regex_4)" "$file" "$((l3 + 1))") || return 1
> +
> +	# Callchain pop
> +	l5=$(find_after_line "$(callchain_regex_3)" "$file" "$((l4 + 1))") || return 1
> +	l6=$(find_after_line "$(callchain_regex_2)" "$file" "$((l5 + 1))") || return 1

[Severity: Medium]
Since this test relies on a 3-instruction sampling interval
(--itrace=g16i3il64), is there a risk of completely missing the return
epilogue of callchain_do_syscall() if the compiler generates an epilogue
shorter than 3 instructions?

For example, on ARM32 or certain AArch64 builds, the epilogue could be just
two instructions like "pop {r11, lr}; bx lr". Since the workload executes
exactly once, is there a chance this will fail to capture an instruction
sample where callchain_do_syscall is the top frame on the return path,
causing a spurious test failure?

> +	l7=$(find_after_line "$(callchain_regex_1)" "$file" "$((l6 + 1))") || return 1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-b4-arm_cs_callchain_support_v1-v9-0-f8fad931c413@arm.com?part=9