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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The regular expressions for matching `perf script` output do not account for the timestamp field, guaranteeing test failure.
- [Low] The `.size` directive for the `main` function incorrectly excludes its `ret` instruction.
- [Low] The `adr` instruction is used to reference a symbol in a different section, which can cause a linker error.
--

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

perf test: Add Arm CoreSight callchain test

This commit adds a shell test for synthesized callchains from Arm CoreSight
trace data. It builds a test program for syscalls, records them, and
decodes with itrace callchain synthesis enabled to verify the push and pop
callchains.

> diff --git a/tools/perf/tests/shell/test_arm_coresight_callchain.sh b/tools/perf/tests/shell/test_arm_coresight_callchain.sh
> new file mode 100755
> index 0000000000000..0e5a5d1129ae7
> --- /dev/null
> +++ b/tools/perf/tests/shell/test_arm_coresight_callchain.sh

[ ... ]

> +do_svc:
> +	stp	x29, x30, [sp, #-16]!
> +	mov	x29, sp
> +
> +	mov	x0, #1
> +	adr	x1, msg

[Severity: Low]
Can this adr instruction cause a linker error?

The adr instruction computes a pc-relative offset but has a strict +/- 1 MB
range limit. If the linker places .text and .rodata further apart than 1 MB
(due to page size alignment or hardening constraints), the build will fail
with an out-of-range relocation error.

Would it be safer to use adrp and add here?

[ ... ]

> +main:
> +	stp	x29, x30, [sp, #-16]!
> +	mov	x29, sp
> +
> +	bl	foo
> +
> +	mov	w0, #0
> +	ldp	x29, x30, [sp], #16
> +	.size main, .-main
> +	ret

[Severity: Low]
Does this .size directive incorrectly exclude the ret instruction from the
main function bounds?

Placing it before the ret instruction might cause profiling tools to fail to
correctly attribute an execution sample on that instruction to the main
function.

[ ... ]

> +callchain_push_regex()
> +{
> +	printf '%s' \
> +'callchain[[:space:]]+[0-9]+ \[[0-9]+\][[:space:]]+10 instructions:[[:space:]]*\n'\

[Severity: High]
Will this regex fail to match because of the perf script timestamp field?

By default, perf script output includes a timestamp field, formatting lines
as "comm tid [cpu] time: event:". The test invokes perf script without
restricting the output fields (for example, it does not use -F,-time to
disable the timestamp).

Since this regex strictly requires spaces immediately between the cpu
identifier and the event name, it seems it will fail to match the timestamp
and cause the test to fail.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-b4-arm_cs_callchain_support_v1-v6-0-f9f49f53c9dd@arm.com?part=8