Re: [PATCH v2] perf riscv: Add perf_regs_load and enable the dwarf unwind test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] perf riscv: unchecked return from perf_sample__user_regs() causes crash on OOM
- [Low] perf riscv: missing explicit POSIX header inclusion for NULL in arch-tests.c
- [Low] perf riscv: misleading error message for sample_ustack() allocation failure
--

commit e16ea72472ccddd24553a422bc4e281ff9984621
Author: Chen Pei <cp0613@linux.alibaba.com>

perf riscv: Add perf_regs_load and enable the dwarf unwind test

This patch adds perf_regs_load() and the arch unwind sample helper for RISC-V,
wiring it into the gates. It also enables the generic dwarf unwind test driver
to support RISC-V.

> diff --git a/tools/perf/arch/riscv/tests/arch-tests.c b/tools/perf/arch/riscv/tests/arch-tests.c
> new file mode 100644
> index 0000000000000..756706a09609e
> --- /dev/null
> +++ b/tools/perf/arch/riscv/tests/arch-tests.c
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "tests/tests.h"
> +#include "arch-tests.h"
> +
> +struct test_suite *arch_tests[] = {
> +#ifdef HAVE_DWARF_UNWIND_SUPPORT
> +	&suite__dwarf_unwind,
> +#endif
> +	NULL,
        ^^^^

[Severity: Low]
Does this code need an explicit POSIX header inclusion for NULL?

The perf subsystem guidelines require explicit, direct header inclusions for
POSIX libc constants like NULL (e.g. <stddef.h>) to prevent compilation
failures on musl libc.

> +};

> diff --git a/tools/perf/arch/riscv/tests/dwarf-unwind.c b/tools/perf/arch/riscv/tests/dwarf-unwind.c
> new file mode 100644
> index 0000000000000..fe0c32ea0e05a
> --- /dev/null
> +++ b/tools/perf/arch/riscv/tests/dwarf-unwind.c
> @@ -0,0 +1,65 @@
[ ... ]
> +static int sample_ustack(struct perf_sample *sample,
> +		struct thread *thread, u64 *regs)
> +{
> +	struct stack_dump *stack = &sample->user_stack;
> +	struct map *map;
> +	unsigned long sp;
> +	u64 stack_size, *buf;
> +
> +	buf = malloc(STACK_SIZE);
> +	if (!buf) {
> +		pr_debug("failed to allocate sample uregs data\n");
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
Is this error message accurate?

Since the allocation in sample_ustack() is for the user stack (STACK_SIZE),
this appears to be a copy-paste error from test__arch_unwind_sample().

> +		return -1;
> +	}
[ ... ]
> +int test__arch_unwind_sample(struct perf_sample *sample,
> +		struct thread *thread)
> +{
> +	struct regs_dump *regs = perf_sample__user_regs(sample);
> +	u64 *buf;
> +
> +	buf = calloc(PERF_REGS_MAX, sizeof(u64));
> +	if (!buf) {
> +		pr_debug("failed to allocate sample uregs data\n");
> +		return -1;
> +	}
> +
> +	perf_regs_load(buf);
> +	regs->abi  = PERF_SAMPLE_REGS_ABI;
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Could regs be a NULL pointer here?

If perf_sample__user_regs(sample) fails its internal allocation (zalloc())
on OOM, it returns NULL. This would result in an unconditional NULL pointer
dereference when setting regs->abi.

> +	regs->regs = buf;
> +	regs->mask = PERF_REGS_MASK;
> +
> +	return sample_ustack(sample, thread, buf);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916073622.1045-1-cp0613@linux.alibaba.com?part=1