On Wed, Sep 02, 2020 at 03:59:05PM -0700, Nick Desaulniers wrote:
> During Plumbers 2020, we voted to just support the latest release of
> Clang for now. Add a compile time check for this.
>
> We plan to remove workarounds for older versions now, which will break
> in subtle and not so subtle ways.
>
> Suggested-by: Sedat Dilek <[email protected]>
> Suggested-by: Nathan Chancellor <[email protected]>
> Suggested-by: Kees Cook <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> Tested-by: Sedat Dilek <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> Reviewed-by: Miguel Ojeda <[email protected]>
> Reviewed-by: Sedat Dilek <[email protected]>
> Acked-by: Marco Elver <[email protected]>
> Acked-by: Nathan Chancellor <[email protected]>
> Acked-by: Sedat Dilek <[email protected]>
> Link: https://github.com/ClangBuiltLinux/linux/issues/9
> Link: https://github.com/ClangBuiltLinux/linux/issues/941
> ---
> include/linux/compiler-clang.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index cee0c728d39a..230604e7f057 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -3,6 +3,14 @@
> #error "Please don't include <linux/compiler-clang.h> directly, include <linux/compiler.h> instead."
> #endif
>
> +#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 :-)
> +
> /* Compiler specific definitions for Clang compiler */
>
> /* same as gcc, this was present in clang-2.6 so we can assume it works
> --
> 2.28.0.402.g5ffc5be6b7-goog
>
/Jarkko
On Tue, Nov 03, 2020 at 06:55:21AM +0200, Jarkko Sakkinen wrote:
> On Wed, Sep 02, 2020 at 03:59:05PM -0700, Nick Desaulniers wrote:
> > During Plumbers 2020, we voted to just support the latest release of
> > Clang for now. Add a compile time check for this.
> >
> > We plan to remove workarounds for older versions now, which will break
> > in subtle and not so subtle ways.
> >
> > Suggested-by: Sedat Dilek <[email protected]>
> > Suggested-by: Nathan Chancellor <[email protected]>
> > Suggested-by: Kees Cook <[email protected]>
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > Tested-by: Sedat Dilek <[email protected]>
> > Reviewed-by: Kees Cook <[email protected]>
> > Reviewed-by: Miguel Ojeda <[email protected]>
> > Reviewed-by: Sedat Dilek <[email protected]>
> > Acked-by: Marco Elver <[email protected]>
> > Acked-by: Nathan Chancellor <[email protected]>
> > Acked-by: Sedat Dilek <[email protected]>
> > Link: https://github.com/ClangBuiltLinux/linux/issues/9
> > Link: https://github.com/ClangBuiltLinux/linux/issues/941
> > ---
> > include/linux/compiler-clang.h | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > index cee0c728d39a..230604e7f057 100644
> > --- a/include/linux/compiler-clang.h
> > +++ b/include/linux/compiler-clang.h
> > @@ -3,6 +3,14 @@
> > #error "Please don't include <linux/compiler-clang.h> directly, include <linux/compiler.h> instead."
> > #endif
> >
> > +#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 :-)
Do you have a .config and command to reproduce the error?
Cheers,
Nathan
On Mon, Nov 02, 2020 at 11:38:14PM -0700, Nathan Chancellor wrote:
> On Tue, Nov 03, 2020 at 06:55:21AM +0200, Jarkko Sakkinen wrote:
> > On Wed, Sep 02, 2020 at 03:59:05PM -0700, Nick Desaulniers wrote:
> > > During Plumbers 2020, we voted to just support the latest release of
> > > Clang for now. Add a compile time check for this.
> > >
> > > We plan to remove workarounds for older versions now, which will break
> > > in subtle and not so subtle ways.
> > >
> > > Suggested-by: Sedat Dilek <[email protected]>
> > > Suggested-by: Nathan Chancellor <[email protected]>
> > > Suggested-by: Kees Cook <[email protected]>
> > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > Tested-by: Sedat Dilek <[email protected]>
> > > Reviewed-by: Kees Cook <[email protected]>
> > > Reviewed-by: Miguel Ojeda <[email protected]>
> > > Reviewed-by: Sedat Dilek <[email protected]>
> > > Acked-by: Marco Elver <[email protected]>
> > > Acked-by: Nathan Chancellor <[email protected]>
> > > Acked-by: Sedat Dilek <[email protected]>
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/9
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/941
> > > ---
> > > include/linux/compiler-clang.h | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > > index cee0c728d39a..230604e7f057 100644
> > > --- a/include/linux/compiler-clang.h
> > > +++ b/include/linux/compiler-clang.h
> > > @@ -3,6 +3,14 @@
> > > #error "Please don't include <linux/compiler-clang.h> directly, include <linux/compiler.h> instead."
> > > #endif
> > >
> > > +#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 :-)
>
> Do you have a .config and command to reproduce the error?
Attached.
> Cheers,
> Nathan
/Jarkko
On Mon, Nov 2, 2020 at 8:55 PM Jarkko Sakkinen <[email protected]> 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.
--
Thanks,
~Nick Desaulniers
On Tue, Nov 03, 2020 at 10:48:27AM -0800, Nick Desaulniers wrote:
> On Mon, Nov 2, 2020 at 8:55 PM Jarkko Sakkinen <[email protected]> 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...
> --
> Thanks,
> ~Nick Desaulniers
/Jarkko
On Tue, Nov 3, 2020 at 4:38 PM Jarkko Sakkinen <[email protected]> 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 <[email protected]> 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
On Tue, Nov 03, 2020 at 05:18:38PM -0800, Nick Desaulniers wrote:
> On Tue, Nov 3, 2020 at 4:38 PM Jarkko Sakkinen <[email protected]> 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 <[email protected]> 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
Thanks a lot for helping with this :-)
I'm using bpftrace as the frontend.
> 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:
Before I received this response, I did git revert for this commit
and things started working again.
> ```
> 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 */
> ```
> --
Shouldn't "#ifndef" be before the whole version check? Otherwise,
LGTM. Please CC me once there is a properly formed patch to try out.
> Thanks,
> ~Nick Desaulniers
/Jarkko
On Wed, Nov 04, 2020 at 03:34:49AM +0200, Jarkko Sakkinen wrote:
> Shouldn't "#ifndef" be before the whole version check? Otherwise,
> LGTM. Please CC me once there is a properly formed patch to try out.
(to my kernel org address).
/Jarkko
Hi,
I just ran into this and it's a real pain to figure out, because even
with the very latest Fedora 33 on my test machine, which provides clang
version 11.0.0:
$ clang --version
clang version 11.0.0 (Fedora 11.0.0-2.fc33)
Target: x86_64-unknown-linux-gnu
...the bpftrace program still chokes on some, but not all commands, in
ways that invisible to normal debugging. For example:
$ sudo bpftrace -e 'tracepoint:syscalls:sys_enter_vmsplice { @[kstack()]
= count(); }'
/lib/modules/5.10.0-rc4-hubbard-github+/source/include/linux/compiler-clang.h:12:3:
error: Sorry, your version of Clang is too old - please use 10.0.1 or
newer.
But Jarkko's recommended fix works! In other words, applying the diff
below fixes it for me. So I'm replying in order to note that the problem
is real and hoping that the fix is applied soon.
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index dd7233c48bf3..c2228b957fd7 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -7,9 +7,11 @@
+ __clang_minor__ * 100 \
+ __clang_patchlevel__)
+#ifndef __BPF_TRACING__
#if CLANG_VERSION < 100001
# 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,
--
John Hubbard
NVIDIA
On Mon, Nov 16, 2020 at 7:04 PM John Hubbard <[email protected]> wrote:
>
> Hi,
>
> I just ran into this and it's a real pain to figure out, because even
> with the very latest Fedora 33 on my test machine, which provides clang
> version 11.0.0:
Hi John,
Thanks for the report. The patch was picked up by AKPM and is in the -mm tree:
https://ozlabs.org/~akpm/mmots/broken-out/compiler-clang-remove-version-check-for-bpf-tracing.patch
--
Thanks,
~Nick Desaulniers
On Tue, Nov 17, 2020 at 10:46:29AM -0800, Nick Desaulniers wrote:
> On Mon, Nov 16, 2020 at 7:04 PM John Hubbard <[email protected]> wrote:
> >
> > Hi,
> >
> > I just ran into this and it's a real pain to figure out, because even
> > with the very latest Fedora 33 on my test machine, which provides clang
> > version 11.0.0:
>
> Hi John,
> Thanks for the report. The patch was picked up by AKPM and is in the -mm tree:
> https://ozlabs.org/~akpm/mmots/broken-out/compiler-clang-remove-version-check-for-bpf-tracing.patch
This should probably go to Linus as a regression fix in the next wave,
if that is possible.
Cheers,
Nathan
On Tue, 17 Nov 2020 19:31:18 -0700 Nathan Chancellor <[email protected]> wrote:
> On Tue, Nov 17, 2020 at 10:46:29AM -0800, Nick Desaulniers wrote:
> > On Mon, Nov 16, 2020 at 7:04 PM John Hubbard <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > I just ran into this and it's a real pain to figure out, because even
> > > with the very latest Fedora 33 on my test machine, which provides clang
> > > version 11.0.0:
> >
> > Hi John,
> > Thanks for the report. The patch was picked up by AKPM and is in the -mm tree:
> > https://ozlabs.org/~akpm/mmots/broken-out/compiler-clang-remove-version-check-for-bpf-tracing.patch
>
> This should probably go to Linus as a regression fix in the next wave,
> if that is possible.
Yes, I'll sent it along later this week.