Re: [PATCH v7 8/8] perf test: Add Arm CoreSight callchain test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Test script relies on non-portable GNU-specific extensions (`grep -Pzq`, `grep -Pzob`, `head -c`), breaking execution on embedded/BusyBox environments.
--

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

perf test: Add Arm CoreSight callchain test

This commit adds a CoreSight shell test for synthesized callchains using the
new callchain workload. It traces the workload with cs_etm and verifies that
the instruction samples show the expected callchain push and pop.

> diff --git a/tools/perf/tests/shell/coresight/callchain.sh b/tools/perf/tests/shell/coresight/callchain.sh
> new file mode 100755
> index 0000000000000..e9f907f60e3a4
> --- /dev/null
> +++ b/tools/perf/tests/shell/coresight/callchain.sh

[ ... ]

> +skip_if_system_is_not_ready()
> +{
> +	[ "$(uname -m)" = "aarch64" ] || {
> +		echo "Skip: arm64 only test" >&2
> +		return 2
> +	}
> +
> +	perf list | grep -Pzq 'cs_etm//' || {

[Severity: Medium]
Is this grep command safe for embedded environments like BusyBox?

The -P (PCRE), -z, and -q combinations are GNU-specific extensions.
On minimal environments without PCRE support or full GNU grep, this command
will fail, causing the test to incorrectly skip or error out on ARM targets
that otherwise support the event.

> +		echo "Skip: cs_etm event is not available" >&2
> +		return 2
> +	}
> +
> +	return 0
> +}

[ ... ]

> +find_after_line()
> +{
> +	local regex="$1"
> +	local file="$2"
> +	local start="$3"
> +	local offset
> +	local line
> +
> +	# Search in byte offset
> +	offset=$(
> +		tail -n +"$start" "$file" |
> +		grep -Pzob -m1 "$regex" |

[Severity: Medium]
Could this multiline regex search fail on non-GNU grep implementations?

Using -Pzob relies heavily on GNU extensions. BusyBox grep, which is
commonly used on embedded ARM boards, lacks -P, -z, and -b flags.

> +		tr '\0' '\n' |
> +		sed -n 's/^\([0-9][0-9]*\):.*/\1/p;q'
> +	)
> +
> +	if [ -z "$offset" ]; then
> +		echo "Failed to match regex after line $start" >&2
> +		echo "Regex:" >&2
> +		printf '%s\n' "$regex" >&2
> +		echo "Context from line $start:" >&2
> +		sed -n "${start},$((start + 100))p" "$file" >&2
> +		return 1
> +	fi
> +
> +	# Conver from offset to line
> +	line=$(
> +		tail -n +"$start" "$file" |
> +		head -c "$offset" |

[Severity: Medium]
Does head -c break POSIX compatibility here?

The -c option is a GNU extension and is not mandated by POSIX. This could
cause the test to fail on minimal filesystems or minimal rootfs setups that
use standard POSIX tools.

> +		wc -l
> +	)
> +
> +	echo "$((start + line))"
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-b4-arm_cs_callchain_support_v1-v7-0-1ba770c862ae@arm.com?part=8