Message-ID: <CAH0uvohW7_st0i=ek8rc_SucuQEQUgs+fWt12cc3jqiYfxAjmw@mail.gmail.com>
Date: Tue, 8 Oct 2024 00:27:24 -0700
From: Howard Chu <howardchu95@...il.com>
To: Alan Maguire <alan.maguire@...cle.com>
Cc: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>, James Clark <james.clark@...aro.org>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Ian Rogers <irogers@...gle.com>, 
	Adrian Hunter <adrian.hunter@...el.com>, Jiri Olsa <jolsa@...nel.org>, 
	Kan Liang <kan.liang@...ux.intel.com>, Namhyung Kim <namhyung@...nel.org>, 
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, 
	linux-perf-users <linux-perf-users@...r.kernel.org>
Subject: Re: [PATCH 1/1] perf build: Require at least clang 16.0.6 to build
 BPF skeletons

Hi Alan, Arnaldo and James,

This problem was solved in [PATCH 0/2] perf trace: Fix support for the
new BPF feature in clang 12 (Link:
https://lore.kernel.org/linux-perf-users/20241007051414.2995674-1-howardchu95@gmail.com/T/#t),
sorry I forgot to cc you two.

Alan's thought was correct. Thank you so much! :)

Thanks,
Howard


On Tue, Sep 24, 2024 at 11:45 AM Howard Chu <howardchu95@...il.com> wrote:
>
> Hello Alan,
>
> On Tue, Sep 24, 2024 at 2:30 AM Alan Maguire <alan.maguire@...cle.com> wrote:
> >
> > On 24/09/2024 03:00, Howard Chu wrote:
> > > Hello,
> > >
> > > I got some good news:
> > >
> > > Not being able to pass the BPF verifier is solely (or bi-solely)
> > > caused by these two functions:
> > >
> > > SEC("tp/syscalls/sys_enter_rename")
> > > int sys_enter_rename(struct syscall_enter_args *args)
> > >
> > > SEC("tp/syscalls/sys_enter_renameat2")
> > > int sys_enter_renameat2(struct syscall_enter_args *args)
> > >
> > > The problem with it is double-string augmentation, but I haven't
> > > figured out how to solve it. Can you please take a look please,
> > > Arnaldo? Now I just commented these two BPF functions and perf trace
> > > runs no problem, built by clang that's older than clang 16, for
> > > example clang 15.0.7.
> > >
> >
> > hi Howard,
> >
> > I could be wrong - and I don't know this code at all - but I _think_ the
> > problem is here:
> >
> >         oldpath_len = augmented_arg__read_str(&augmented_args->arg,
> > oldpath_arg, sizeof(augmented_args->arg.value));
> >         augmented_args->arg.size = PERF_ALIGN(oldpath_len + 1, sizeof(u64));
> >         len += augmented_args->arg.size;
> >
> >         struct augmented_arg *arg2 = (void *)&augmented_args->arg.value
> > + augmented_args->arg.size;
> >
> >         newpath_len = augmented_arg__read_str(arg2, newpath_arg,
> > sizeof(augmented_args->arg.value));
> >
> >
> > Notice that these strings share the augmented_args->arg.value for
> > storage, but in the second case (reading arg2) we have added
> > augmented_args->arg.size to the offset where we store the second string.
> > However at the same time we have also told augmented_arg__read_str()
> > that it has
> >
> > sizeof(augmented_args->arg.value)
> >
> >
> > ...to work with as arg_len, where in reality we should have specified
> >
> > sizeof(augmented_args->arg.value) - augmented_args->arg.size
> >
> > ...and ensured that after doing that subtraction, there is still space
> > to work with (i.e. the above is > 0).
> >
> > I _think_ the verifier is likely complaining that we can write past the
> > end of augmented_args->arg.value , does that sound right?
> >
> > Fixing the arg_len parameter for cases where we record more than one
> > augmented string by reducing arg_len available should solve this if so.
> > Hope this helps,
>
>
> Thanks for the help, but unfortunately it didn't work... But that's a
> valid analysis, I had the same idea as you, but I haven't been able to
> solve it. I'll give it more tries.
>
> Thanks,
> Howard
>
> >
> > Alan
> >
> > > perf $ clang -v
> > > Ubuntu clang version 15.0.7
> > > Target: x86_64-pc-linux-gnu
> > > Thread model: posix
> > > InstalledDir: /bin
> > > Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> > > Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> > > Candidate multilib: .;@m64
> > > Selected multilib: .;@m64
> > > perf $ ./perf trace -e write --max-events=1
> > >      0.000 ( 0.008 ms): gmain/2173 write(fd: 4, buf: \1\0\0\0\0\0\0\0,
> > > count: 8)                         =
> > >
> > > But plot twist, it won't build on clang-14
> > >
> > > perf $ clang -v
> > > Ubuntu clang version 14.0.6
> > > Target: x86_64-pc-linux-gnu
> > > Thread model: posix
> > > InstalledDir: /bin
> > > Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> > > Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> > > Candidate multilib: .;@m64
> > > Selected multilib: .;@m64
> > > perf $ ./perf trace -e write --max-events=1
> > > libbpf: prog 'sys_enter': BPF program load failed: Invalid argument
> > > libbpf: prog 'sys_enter': -- BEGIN PROG LOAD LOG --
> > > 0: R1=ctx() R10=fp0
> > > ; int sys_enter(struct syscall_enter_args *args) @
> > > augmented_raw_syscalls.bpf.c:509
> > > 0: (bf) r7 = r1                       ; R1=ctx() R7_w=ctx()
> > >
> > > This time the problem is my code in BTF general collector:
> > >
> > > ; payload_offset += written; @ augmented_raw_syscalls.bpf.c:497
> > >
> > > I'll resolve this.
> > >
> > > Thanks,
> > > Howard
> > >
> > > On Wed, Sep 18, 2024 at 8:13 PM Howard Chu <howardchu95@...il.com> wrote:
> > >>
> > >> Hello,
> > >>
> > >> I'm able to reproduce it on Ubuntu 24.
> > >>
> > >> perf $ cat /etc/os-release
> > >> PRETTY_NAME="Ubuntu 24.04.1 LTS"
> > >> NAME="Ubuntu"
> > >> VERSION_ID="24.04"
> > >> VERSION="24.04.1 LTS (Noble Numbat)"
> > >> VERSION_CODENAME=noble
> > >> ID=ubuntu
> > >> ID_LIKE=debian
> > >> HOME_URL="https://www.ubuntu.com/"
> > >> SUPPORT_URL="https://help.ubuntu.com/"
> > >> BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
> > >> PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
> > >> UBUNTU_CODENAME=noble
> > >> LOGO=ubuntu-logo
> > >>
> > >> perf $ uname -r
> > >> 6.11.0-061100-generic
> > >>
> > >> perf $ git log
> > >> commit 4c1af9bf97eb56d069421c3233ce61608458d5c8 (HEAD)
> > >> Author: Arnaldo Carvalho de Melo <acme@...hat.com>
> > >> Date:   Tue Sep 10 13:54:23 2024 -0300
> > >>
> > >>     perf trace: If a syscall arg is marked as 'const', assume it is
> > >> coming _from_ userspace
> > >>
> > >>
> > >> perf $ clang -v
> > >> Ubuntu clang version 15.0.7
> > >> Target: x86_64-pc-linux-gnu
> > >> Thread model: posix
> > >> InstalledDir: /bin
> > >> Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> > >> Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> > >> Candidate multilib: .;@m64
> > >> Selected multilib: .;@m64
> > >>
> > >> perf $ ./perf trace -e write --max-events=1
> > >> libbpf: prog 'sys_enter_rename': BPF program load failed: Permission denied
> > >> libbpf: prog 'sys_enter_rename': -- BEGIN PROG LOAD LOG --
> > >> 0: R1=ctx() R10=fp0
> > >> ; int sys_enter_rename(struct syscall_enter_args *args) @
> > >> augmented_raw_syscalls.bpf.c:275
> > >>
> > >>
> > >> With clang-16:
> > >>
> > >> perf $ clang -v
> > >> Ubuntu clang version 16.0.6 (23ubuntu4)
> > >> Target: x86_64-pc-linux-gnu
> > >> Thread model: posix
> > >> InstalledDir: /bin
> > >> Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> > >> Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/13
> > >> Candidate multilib: .;@m64
> > >> Selected multilib: .;@m64
> > >>
> > >> perf $ ./perf trace -e write --max-events=1
> > >>      0.000 ( 0.021 ms): sudo/4741 write(fd: 8, buf:
> > >> \27[?25l\27[37m\27[48;5;96m\27[H[trace] , count: 205) =
> > >>
> > >>> I'll try to build it and see. But Ubuntu 22 only has clang 11 to 15 so
> > >>> making 16 the minimum could be an issue.
> > >>
> > >> Yes, I think disabling any distro that doesn't come with clang 16
> > >> (released in 2023) is not a good idea. I'll try to tame the BPF
> > >> verifier.
> > >>
> > >> Thanks,
> > >> Howard
> > >>
> > >> On Tue, Sep 17, 2024 at 10:37 AM Howard Chu <howardchu95@...il.com> wrote:
> > >>>
> > >>> Hello James and Arnaldo,
> > >>>
> > >>> On Fri, Sep 13, 2024 at 3:42 AM Arnaldo Carvalho de Melo
> > >>> <arnaldo.melo@...il.com> wrote:
> > >>>>
> > >>>>
> > >>>>
> > >>>> On Fri, Sep 13, 2024, 7:20 AM James Clark <james.clark@...aro.org> wrote:
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> On 12/09/2024 15:50, Arnaldo Carvalho de Melo wrote:
> > >>>>>> On Thu, Sep 12, 2024 at 03:40:32PM +0100, James Clark wrote:
> > >>>>>>> On 11/09/2024 13:24, Arnaldo Carvalho de Melo wrote:
> > >>>>>>>> Howard reported problems using perf features that use BPF:
> > >>>>>>
> > >>>>>>>>     perf $ clang -v
> > >>>>>>>>     Debian clang version 15.0.6
> > >>>>>>>>     Target: x86_64-pc-linux-gnu
> > >>>>>>>>     Thread model: posix
> > >>>>>>>>     InstalledDir: /bin
> > >>>>>>>>     Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/12
> > >>>>>>>>     Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/12
> > >>>>>>>>     Candidate multilib: .;@m64
> > >>>>>>>>     Selected multilib: .;@m64
> > >>>>>>>>     perf $ ./perf trace -e write --max-events=1
> > >>>>>>>>     libbpf: prog 'sys_enter_rename': BPF program load failed: Permission denied
> > >>>>>>>>     libbpf: prog 'sys_enter_rename': -- BEGIN PROG LOAD LOG --
> > >>>>>>>>     0: R1=ctx() R10=fp0
> > >>>>>>>>
> > >>>>>>>> But it works with:
> > >>>>>>>>
> > >>>>>>>>     perf $ clang -v
> > >>>>>>>>     Debian clang version 16.0.6 (15~deb12u1)
> > >>>>>>>>     Target: x86_64-pc-linux-gnu
> > >>>>>>>>     Thread model: posix
> > >>>>>>>>     InstalledDir: /bin
> > >>>>>>>>     Found candidate GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/12
> > >>>>>>>>     Selected GCC installation: /bin/../lib/gcc/x86_64-linux-gnu/12
> > >>>>>>>>     Candidate multilib: .;@m64
> > >>>>>>>>     Selected multilib: .;@m64
> > >>>>>>>>     perf $ ./perf trace -e write --max-events=1
> > >>>>>>>>          0.000 ( 0.009 ms): gmain/1448 write(fd: 4, buf: \1\0\0\0\0\0\0\0, count: 8)                         = 8 (kworker/0:0-eve)
> > >>>>>>>>     perf $
> > >>>>>>>>
> > >>>>>>>> So lets make that the required version, if you happen to have a slightly
> > >>>>>>>> older version where this work, please report so that we can adjust the
> > >>>>>>>> minimum required version.
> > >>>>>>
> > >>>>>>> I wasn't able to reproduce the issue with either of these versions. But I
> > >>>>>>> suppose it could be an issue with only 15.0.6.
> > >>>>>>
> > >>>>>> Interesting, that complicates things, probably the best way then is to
> > >>>>>> try to build it, if it fails, mention that 15.0.6 is known to be
> > >>>>>> problematic and suggest working versions?
> > >>>>>>
> > >>>>>> - Arnaldo
> > >>>>>
> > >>>>> I still wasn't able to reproduce it with 15.0.6. And I double checked
> > >>>>> with V=1 that the build was using the right clang. I suppose it could be
> > >>>>> a build configuration issue, or maybe with a different kernel version?
> > >>>>
> > >>>>
> > >>>> Howard? If not reproducible we can revert it.
> > >>>>
> > >>>> - Arnaldo
> > >>>>
> > >>>>
> > >>>>>
> > >>>>> $  uname --kernel-release
> > >>>>> 6.8.0-76060800daily20240311-generic
> > >>>>>
> > >>>>> $ ../../llvm-project/build/bin/clang -v
> > >>>>> clang version 15.0.6 (https://github.com/llvm/llvm-project.git
> > >>>>> 088f33605d8a61ff519c580a71b1dd57d16a03f8)
> > >>>>> Target: x86_64-unknown-linux-gnu
> > >>>>> Thread model: posix
> > >>>>> InstalledDir: /home/james/workspace/linux/linux/../../llvm-project/build/bin
> > >>>>> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11
> > >>>>> Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/12
> > >>>>> Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/12
> > >>>>> Candidate multilib: .;@m64
> > >>>>> Selected multilib: .;@m64
> > >>>>>
> > >>>>> $ git log
> > >>>>> commit 003265bb6f028d7bcd7cbd92d6ba2b4e26382796
> > >>>>> (perf-tools-next/perf-tools-next)
> > >>>>>
> > >>>>> $ make O=../build/local/ CLANG=../../llvm-project/build/bin/clang -C \
> > >>>>>    tools/perf
> > >>>>>
> > >>>>> $ perf trace -e write --max-events=1
> > >>>>>
> > >>>>>       0.000 ( 0.026 ms): gnome-shell/5454 write(fd: 5, buf:
> > >>>>>         0x7fffa102d9b0, count: 8)                           = 8
> > >>>
> > >>> Thanks for the effort! But this output seems to be unaugmented? I
> > >>> think you are testing the perf/perf-tools-next branch that's slightly
> > >>> older (perf mem: Fix the wrong reference in parse_record_events()
> > >>> 003265bb6f028d7bcd7cbd92d6ba2b4e26382796), and I think that doesn't
> > >>> have the new perf trace feature yet...
> > >>>
> > >>> That's why the perf trace output looks like:
> > >>>
> > >>>>> $ perf trace -e write --max-events=1
> > >>>>>
> > >>>>>       0.000 ( 0.026 ms): gnome-shell/5454 write(fd: 5, buf:
> > >>>>>         0x7fffa102d9b0, count: 8)
> > >>>
> > >>> With the new buffer augmentation it should look like this:
> > >>>
> > >>> perf $ ./perf trace -e write --max-events=1
> > >>>      0.000 ( 0.011 ms): gmain/1408 write(fd: 4, buf: \1\0\0\0\0\0\0\0,
> > >>> count: 8)                         =
> > >>>
> > >>> The new perf trace can be found on
> > >>> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/
> > >>> perf-tools-next
> > >>>
> > >>> perf $ git log
> > >>> commit 1de5b5dcb8353f36581c963df2d359a5f151a0be (HEAD ->
> > >>> struct-aug-arnaldo-2, perf/perf-tools-next)
> > >>>
> > >>> Also, if you want to test the clang, you may want to checkout to the
> > >>> commit before the clang-16 guard (otherwise you can't test different
> > >>> clang version).
> > >>>
> > >>> perf $ git log --oneline
> > >>> 1de5b5dcb835 (HEAD -> struct-aug-arnaldo-2, perf/perf-tools-next) perf
> > >>> trace: Mark the 'head' arg in the set_robust_list syscall as coming
> > >>> from user space
> > >>> 0c1019e3463b perf trace: Mark the 'rseq' arg in the rseq syscall as
> > >>> coming from user space
> > >>> edf3ce0ed38e perf env: Find correct branch counter info on hybrid
> > >>> 9953807c9e01 perf evlist: Print hint for group
> > >>> eb9b9a6f5ab3 tools: Drop nonsensical -O6
> > >>> 89c0a55e550e perf pmu: To info add event_type_desc
> > >>> f08cc258431d perf evsel: Add accessor for tool_event
> > >>> 925320737ae2 perf pmus: Fake PMU clean up
> > >>> d3d5c1a00fcd perf list: Avoid potential out of bounds memory read
> > >>> 4ae354d73a8e perf help: Fix a typo ("bellow")
> > >>> 74298dd8acb8 perf ftrace: Detect whether ftrace is enabled on system
> > >>> 83420d5f5863 perf test shell probe_vfs_getname: Remove extraneous '='
> > >>> from probe line number regex
> > >>> 9327f0ecad48 perf build: Require at least clang 16.0.6 to build BPF
> > >>> skeletons. **********[[[[[[[[[[[[!!!!(This is the clang
> > >>> guard)]]]]]]]]]]*****
> > >>> 4c1af9bf97eb perf trace: If a syscall arg is marked as 'const', assume
> > >>> it is coming _from_ userspace
> > >>> e37b315c17df perf parse-events: Remove duplicated include in parse-events.c
> > >>>
> > >>> My machine is debian 12, and I cannot build perf trace with the new
> > >>> augmentation feature on clang-13, clang-14, and clang-15.
> > >>>
> > >>> perf $ uname -r
> > >>> 6.11.0-061100rc7-generic
> > >>>
> > >>> perf $ git log
> > >>> commit 4c1af9bf97eb56d069421c3233ce61608458d5c8 (HEAD)
> > >>>
> > >>> perf $ make CLANG=/bin/clang-13 && ./perf trace -e write --max-events=1
> > >>> ...
> > >>> libbpf: prog 'sys_enter': BPF program load failed: Invalid argument
> > >>> libbpf: prog 'sys_enter': -- BEGIN PROG LOAD LOG --
> > >>> 0: R1=ctx() R10=fp0
> > >>> ; int sys_enter(struct syscall_enter_args *args) @
> > >>> augmented_raw_syscalls.bpf.c:509
> > >>> 0: (bf) r7 = r1                       ; R1=ctx() R7_w=ctx()
> > >>>
> > >>>
> > >>> perf $ make CLANG=/bin/clang-14 && ./perf trace -e write --max-events=1
> > >>> ...
> > >>> libbpf: prog 'sys_enter': BPF program load failed: Invalid argument
> > >>> libbpf: prog 'sys_enter': -- BEGIN PROG LOAD LOG --
> > >>> 0: R1=ctx() R10=fp0
> > >>> ; int sys_enter(struct syscall_enter_args *args) @
> > >>> augmented_raw_syscalls.bpf.c:509
> > >>>
> > >>>
> > >>> perf $ make CLANG=/bin/clang-15 && ./perf trace -e write --max-events=1
> > >>> libbpf: prog 'sys_enter': BPF program load failed: Invalid argument
> > >>> libbpf: prog 'sys_enter': -- BEGIN PROG LOAD LOG --
> > >>> 0: R1=ctx() R10=fp0
> > >>>
> > >>>
> > >>> perf $ make CLANG=/bin/clang-16 && ./perf trace -e write --max-events=1
> > >>>      0.000 ( 0.010 ms): gmain/1408 write(fd: 4, buf: \1\0\0\0\0\0\0\0,
> > >>> count: 8)                         =
> > >>>
> > >>>
> > >>> Anyway thank you so much, I'll try to reproduce it on Ubuntu, like you
> > >>> mentioned.
> > >>>
> > >>> Thanks,
> > >>> Howard
> > >>>
> > >>>
> > >>>>>
> >

Powered by blists - more mailing lists