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

[Song Liu <songliubraving@meta.com>]

> On Oct 15, 2024, at 1:37 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> On Tue, Oct 15, 2024 at 04:58:56PM -0300, Arnaldo Carvalho de Melo wrote:
>> So I'm trying adding extra bounds checking, marking the index as
>> volatile, adding compiler barriers, etc, all the fun with the verifier,
>> but got distracted with other stuff, coming back to this now.
> 
>> Ok, the following seems to do the trick:
> 
>> [acme@dell-per740-01 perf-tools]$ git diff
>> diff --git a/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c b/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c
>> index 3b30aa74a3ae..ef87a04ff8d0 100644
>> --- a/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c
>> +++ b/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c
>> @@ -486,6 +486,7 @@ static int augment_sys_enter(void *ctx, struct syscall_enter_args *args)
>>                                augmented = true;
>>                } else if (size < 0 && size >= -6) { /* buffer */
>>                        index = -(size + 1);
>> +                       index &= 7; // To satisfy the bounds checking with the verifier in some kernels
>>                        aug_size = args->args[index];
>> 
>>                        if (aug_size > TRACE_AUG_MAX_BUF)
>> 
>> I'll now test it without Howard's patch to see if it fixes the RHEL8 +
>> clang 17 case.
> 
> It works with this one-liner + the simplified patch from Howard and also
> on this other system (RHEL9), as well as with Fedora 40, it would be
> nice if someone could test with clang 16 and report back the version of
> the kernel tested as well as the distro name/release, that way I can try
> to get my hands on such as system and test there as well.

I am testing this one-liner with clang 18.1.8 and an older kernel (it is 
our version of 5.12 kernel, but the BPF verifier there should be similar
to recent kernels). verifier still fails the program. IIUC, the compiler 
is smart enough to just remove the one-liner completely.


Thanks,
Song