Re: [PATCH v3 14/14] perf test: Remove /usr/bin/cc dependency from Intel PT shell test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Instruction endianness mismatch on Big-Endian ARM (BE-8) and RISC-V.
- [Low] Missing fallback for `gettid()` causes build failures on older libc environments.
--

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

perf test: Remove /usr/bin/cc dependency from Intel PT shell test

This commit removes the dependency on an external C compiler in the
test_intel_pt.sh shell test by introducing built-in thloop and jitdump
workloads. It also adds multi-architecture compatibility by dynamically
encoding opcodes across several architectures without requiring external
tools.

[ ... ]

> diff --git a/tools/perf/tests/workloads/jitdump.c b/tools/perf/tests/workloads/jitdump.c
> new file mode 100644
> index 0000000000000..225d3f3dc6e3b
> --- /dev/null
> +++ b/tools/perf/tests/workloads/jitdump.c
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0

[ ... ]

> +static int write_jitdump(FILE *f, void *addr, const void *dat, size_t sz, uint64_t *idx)
> +{
> +	struct jr_code_load rec = {
> +		.p.id = JIT_CODE_LOAD,
> +		.p.total_size = sizeof(rec) + sz,
> +		.p.timestamp = get_timestamp(),
> +		.pid = getpid(),
> +		.tid = gettid(),

[Severity: Low]
Does this code build successfully on older libc environments?

Older distributions do not declare gettid(), and unlike other perf files
that use syscall(__NR_gettid) as a fallback, this file lacks a fallback
definition. Because tools/perf/ is built with -Werror, could this
implicit function declaration break the build on those systems?

> +		.vma = (unsigned long)addr,
> +		.code_addr = (unsigned long)addr,
> +		.code_size = sz,
> +		.code_index = ++*idx,
> +	};
> +
> +	if (fwrite(&rec, sizeof(rec), 1, f) != 1 || fwrite(dat, sz, 1, f) != 1)
> +		return -1;
> +	return 0;
> +}
> +
> +static void close_jitdump(FILE *f)
> +{
> +	fclose(f);
> +}
> +
> +static int jitdump(int argc __maybe_unused, const char **argv __maybe_unused)
> +{
> +#if defined(__x86_64__) || defined(__i386__)
> +	/* Code to execute: mov CHK_BYTE, %eax ; ret */
> +	uint8_t dat[] = { 0xb8, CHK_BYTE, 0x00, 0x00, 0x00, 0xc3 };
> +#elif defined(__aarch64__)
> +	/* Code to execute: mov w0, #CHK_BYTE ; ret */
> +	uint32_t dat[] = { 0x52800000 | (CHK_BYTE << 5), 0xd65f03c0 };

[Severity: Medium]
Will these uint32_t arrays cause an instruction endianness mismatch on
Big-Endian ARM (BE-8) and RISC-V hosts?

Since uint32_t arrays are laid out in memory according to the host data
endianness, and these architectures expect Little-Endian instruction
fetches even in Big-Endian data modes, running this test on a Big-Endian
ARM or RISC-V host might result in byte-swapped, invalid instructions and
trigger a SIGILL crash.

Should these instructions be encoded as uint8_t byte arrays instead,
similar to how it is handled for s390x, to remain agnostic to host data
endianness?

> +#elif defined(__riscv)
> +	/* Code to execute: li a0, CHK_BYTE ; ret */
> +	uint32_t dat[] = { ((CHK_BYTE & 0xfff) << 20) | 0x513, 0x00008067 };

[ ... ]

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