Re: [PATCH v2 0/2] perf trace: Fix support for the new BPF feature in clang 12

[Arnaldo Carvalho de Melo]

On Tue, Oct 15, 2024 at 11:32:45AM -0700, Namhyung Kim wrote:
> On Thu, Oct 10, 2024 at 07:14:00PM -0700, Howard Chu wrote:
> > Changes in v2:
> > - Resolved a clang-16 build error pointed out by Namhyung Kim
> >   <namhyung@xxxxxxxxxx>

> > The new augmentation feature in perf trace, along with the protocol
> > change (from payload to payload->value), breaks the clang 12 build.

> > perf trace actually builds for any clang version newer than clang 16.
> > However, as pointed out by Namhyung Kim <namhyung@xxxxxxxxxx> and Ian
> > Rogers <irogers@xxxxxxxxxx>, clang 16, which was released in 2023, is
> > still too new for most users. Additionally, as James Clark
> > <james.clark@xxxxxxxxxx> noted, some commonly used distributions do not
> > yet support clang 16. Therefore, breaking BPF features between clang 12
> > and clang 15 is not a good approach.

> > This patch series rewrites the BPF program in a way that allows it to
> > pass the BPF verifier, even when the BPF bytecode is generated by older
> > versions of clang.

> > However, I have only tested it till clang 14, as older versions are not
> > supported by my distribution.
 
> > Howard Chu (2):
> >   perf build: Change the clang check back to 12.0.1
> >   perf trace: Rewrite BPF code to pass the verifier
 
> Tested with clang 16.  And I think it's better to change the order of
> the commits so it can fix the problem first and then check the version.

So, I tested it on a RHEL8 system and it gets built with clang 17 but
then fails to load, the verifier complains about lack of bounds checking
for the index of the syscall array, with or without this last patch from
Howard.

I also simplified it to a more minimal version withour renaming
variables, so that we see what exactly fixed the problem, its available
at the perf-tools/tmp.perf-tools branch, I've talked about it with
Howard over chat.

Song Liu reproduced the problem (unsure with what clang and kernel
versions) and couldn't find a way to fix it using the usual tricks to
coax clang to keep the bounds checking for the verifier to get
satisfied.

More generally I'll use virtme-ng[1] to test with a wider range of
kernels, not just clang versions.

- Arnaldo

[1] https://kernel-recipes.org/en/2024/virtme-ng/