Re: [PATCH v4 5/6] perf arm_spe: Decode Arm N1 IMPDEF events

Re: [PATCH v4 5/6] perf arm_spe: Decode Arm N1 IMPDEF events

[Arnaldo Carvalho de Melo]

On Tue, Apr 14, 2026 at 03:52:02PM +0000, sashiko-bot@kernel.org 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 <james.clark@linaro.org>
> 
> 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

Re: [PATCH v4 5/6] perf arm_spe: Decode Arm N1 IMPDEF events

[James Clark]

On 06/05/2026 2:01 am, Arnaldo Carvalho de Melo wrote:
> On Tue, Apr 14, 2026 at 03:52:02PM +0000, sashiko-bot@kernel.org 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 <james.clark@linaro.org>
>>
>> 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/adiB0V6Unmo0-AkC@google.com/#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.