Re: [RFC v1 19/19] ptwrite uprobes: Add self tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing test scripts and source files cause selftests build failures and unconditional skips.
--

commit cdbf7a2bf57258522e6b6951a74593c1f59e7dd4
Author: Andi Kleen <ak@kernel.org>

ptwrite uprobes: Add self tests

This commit adds selftests for ptwrite uprobes. It tests various cases using 
both the main probes and the perf decoder.

> diff --git a/tools/testing/selftests/uprobes/Makefile b/tools/testing/selftests/uprobes/Makefile
> new file mode 100644
> index 0000000000000..9fc65c1f04c84
> --- /dev/null
> +++ b/tools/testing/selftests/uprobes/Makefile
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# ptwrite uprobe selftests (x86-64).
> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> +CFLAGS += -O2 -Wall -no-pie
> +
> +TEST_GEN_FILES := ptw_probe
> +TEST_PROGS := run_ptw.sh run_module.sh run_perfprobe.sh run_decode.sh

[Severity: Medium]
Will this break the selftests build during a `make install`? The Makefile
lists run_module.sh and run_perfprobe.sh in TEST_PROGS, but these files do
not appear to be included in this patch.

[ ... ]

> diff --git a/tools/testing/selftests/uprobes/run_decode.sh b/tools/testing/selftests/uprobes/run_decode.sh
> new file mode 100755
> index 0000000000000..b427442041aa9
> --- /dev/null
> +++ b/tools/testing/selftests/uprobes/run_decode.sh
> @@ -0,0 +1,177 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# run_decode.sh - ptwrite decoder selftests.
> +# Exercises the decoder CLI and PT/perf integration.
> +# Root + tracefs + perf + gcc + a PTWRITE-capable CPU required.
> +set -u
> +DIR=$(dirname "$(readlink -f "$0")")
> +SRC=${1:-"$DIR/manual_ptw.c"}

[Severity: Medium]
Does this cause the test to unconditionally skip? The SRC variable defaults
to manual_ptw.c, but this source file seems to be missing from the commit. 
If the file is added in a subsequent version, should it also be included in
TEST_FILES in the Makefile to ensure it is installed correctly during
`make install`?

[ ... ]

> +if ! gcc -O2 -no-pie -o "$BIN" "$SRC" 2>/dev/null; then
> +	echo "1..0 # SKIP test program build failed"
> +	exit 0
> +fi
> +
> +TV=$(objdump -d "$BIN" 2>/dev/null |
> +	awk '/^[0-9a-f]+ <target>:/{print $1;exit}' | tr -d ':')

[Severity: Medium]
Is manual_ptw.c missing from the commit? This logic expects the compiled
test binary to contain a `target` function, which further suggests that the
omitted C file is strictly required for this test to function.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831150651.1134594-1-ak@kernel.org?part=19