Re: [PATCH v4 5/6] perf arm_spe: Decode Arm N1 IMPDEF events
James Clark <[email protected]> Wed, 6 May 2026 09:39:15 +0100
| Newsgroups | dev.linux.lists.sashiko,org.kernel.vger.linux-perf-users |
|---|---|
| Message-ID | <[email protected]> |
On 06/05/2026 2:01 am, Arnaldo Carvalho de Melo wrote: > On Tue, Apr 14, 2026 at 03:52:02PM +0000, [email protected] wrote: >> Sashiko AI review found 1 potential issue(s): >> - [Low] The relative include path `#include "../../arm64/include/asm/cputype.h"` is invalid and relies on accidental directory traversal through a compiler `-I` flag. >> -- >> >> commit 7fc153524fe60dc6d6f08fd73b7ea3f46e904e39 >> Author: James Clark <[email protected]> >> >> perf arm_spe: Decode Arm N1 IMPDEF events >> >> This commit adds a framework to decode the N1 IMPDEF event for Arm SPE, >> which isn't covered by the common list. It also increases the packet >> description buffer size to accommodate more strings without falling back >> to a hex dump. >> >> Link: https://developer.arm.com/documentation/100616/0401/Statistical-Profiling-Extension/implementation-defined-features-of-SPE >> >>> diff --git a/tools/perf/util/arm-spe-decoder/Build b/tools/perf/util/arm-spe-decoder/Build >>> index ab500e0efe244..97a298d1e2791 100644 >>> --- a/tools/perf/util/arm-spe-decoder/Build >>> +++ b/tools/perf/util/arm-spe-decoder/Build >>> @@ -1 +1,3 @@ >>> perf-util-y += arm-spe-pkt-decoder.o arm-spe-decoder.o >>> + >>> +CFLAGS_arm-spe-pkt-decoder.o += -I$(srctree)/tools/arch/arm64/include/ -I$(OUTPUT)arch/arm64/include/generated/ >> >> [ ... ] >> >>> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c >>> index 67ca356100e53..b74f887a48f2a 100644 >>> --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c >>> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c >>> @@ -15,6 +15,8 @@ >>> >>> #include "arm-spe-pkt-decoder.h" >>> >>> +#include "../../arm64/include/asm/cputype.h" >> >> Is this relative include path correct? >> >> It looks like this file is located in tools/perf/util/arm-spe-decoder/, so >> the relative path resolves to tools/perf/arm64/include/asm/cputype.h, >> which doesn't exist. >> >> It seems this only compiles because the newly added -I flag in the Build >> file causes the preprocessor to resolve the include relative to >> tools/arch/arm64/include/. The path then inadvertently evaluates back to the >> correct file by traversing backwards out of the include directory and diving >> back in. >> >> Could this be made less brittle by relying directly on the include flag, >> using #include <asm/cputype.h> instead? > > This is just a possible improvement and not strictly a problem as the -I > is structured today, so lets not postpone processing this patchset > because of this, this can be considered some other time. > > - Arnaldo Yeah, we also discussed it before here: https://lore.kernel.org/all/[email protected]/#t I thought sashiko pulled previous patch versions and other context, it would be nice if it read the discussion threads too. Maybe one day it will.