Re: [PATCH v3 1/7] compiler-clang: add build check for clang 10.0.1

[Nick Desaulniers]

On Tue, Nov 3, 2020 at 4:38 PM Jarkko Sakkinen <jarkko@xxxxxxxxxx> wrote:
>
> On Tue, Nov 03, 2020 at 10:48:27AM -0800, Nick Desaulniers wrote:
> > On Mon, Nov 2, 2020 at 8:55 PM Jarkko Sakkinen <jarkko@xxxxxxxxxx> wrote:
> > >
> > > On Wed, Sep 02, 2020 at 03:59:05PM -0700, Nick Desaulniers wrote:
> > > > +#define CLANG_VERSION (__clang_major__ * 10000       \
> > > > +                  + __clang_minor__ * 100    \
> > > > +                  + __clang_patchlevel__)
> > > > +
> > > > +#if CLANG_VERSION < 100001
> > > > +# error Sorry, your version of Clang is too old - please use 10.0.1 or newer.
> > > > +#endif
> > >
> > >
> > > I'm trying to compile a BPF enabled test kernel for a live system and I
> > > get this error even though I have much newer clang:
> > >
> > > ➜  ~ (master) ✔ clang --version
> > > Ubuntu clang version 11.0.0-2
> > > Target: x86_64-pc-linux-gnu
> > > Thread model: posix
> > > InstalledDir: /usr/bin
> > >
> > > Tried to Google for troubleshooter tips but this patch is basically the
> > > only hit I get :-)
> >
> > To check the values of the above preprocessor defines, please run:
> > $ clang -dM -E - < /dev/null | grep -e __clang_m -e __clang_p
> >
> > If you have multiple versions of clang installed, you might not be
> > running the version you think you are.  Particularly, if you're using
> > bcc, idk if it includes a copy of clang?  If that's the case, we may
> > have to work out how we can support older versions of clang for the
> > express purposes of bpf.
>
> ➜  ~ (master) ✔ clang -dM -E - < /dev/null | grep -e __clang_m -e __clang_p
> #define __clang_major__ 11
> #define __clang_minor__ 0
> #define __clang_patchlevel__ 0
>
> I'm compiling the kernel itself with GCC.
>
> Here's an example BPF script that fails on me:
>
> struct sgx_enclave_add_pages {
>         unsigned long src;
>         unsigned long offset;
>         unsigned long length;
>         unsigned long secinfo;
>         unsigned long flags;
>         unsigned long count;
> };
>
> kprobe:sgx_ioctl
> {
>         if (arg1 == 0xc030a401) {
>                 printf("sgx_ioctl: %d, %lu\n", pid, ((struct sgx_enclave_add_pages *)(arg2))->offset);
>         }
>
> }
> Note that it relies on code not yet in the mainline.
>
> If I don't declare structs, things work just fine. E.g. the following
> works:
>
> kprobe:sgx_encl_get_backing
> {
>         printf("%s\n", func)
> }
>
> BTW, I don't really understand how scripts/clang-version.sh is even
> supposed to work, if you compile the kernel itself with GCC. In that
> case there would be no output, right? And thus version gets set to
> zero...

That script is only used by KBUILD.  include/linux/compiler-clang.h is
what's included into include/linux/compiler_types.h and causes the
error.  The eBFP tools must be including kernel headers and defining
`__clang__`.  Forgive my complete ignorance of eBPF, but how do you
build that script?  I assume the tool is using Clang, since eBPF
relies on the LLVM backend (not sure if the GCC eBPF backend is good
to go quite yet), and that version of clang is older.

I wonder if we should guard the version check with __BPF_TRACING__
similar to arch/x86/include/asm/cpufeature.h? Care to test:
```
diff --git a/include/linux/compiler-clang.h
b/include/linux/compiler-clang.h
index dd7233c48bf3..98cff1b4b088 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -8,8 +8,10 @@
                     + __clang_patchlevel__)

 #if CLANG_VERSION < 100001
+#ifndef __BPF_TRACING__
 # error Sorry, your version of Clang is too old - please use 10.0.1 or newer.
 #endif
+#endif

 /* Compiler specific definitions for Clang compiler */
```
-- 
Thanks,
~Nick Desaulniers