| Message ID | 20230815065346.131387-1-andrea.righi@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rust: fix bindgen build error with fstrict-flex-arrays | expand |
On Tue, Aug 15, 2023 at 8:54 AM Andrea Righi <andrea.righi@canonical.com> wrote: > > Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") enabled > '-fstrict-flex-arrays=3' globally, but bindgen does not recognized this It may be more accurate to say libclang here (bindgen forwards the options). Also, df8fc4e934c1 did it so only conditionally (if the C compiler supports it). This explains what you are seeing: if I am right, you are compiling with a modern enough GCC, which enables the option, but with an old enough Clang. > compiler option, triggering the following build error: > > error: unknown argument: '-fstrict-flex-arrays=3', err: true This should only be true with libclang < 16, since Clang 16 implemented the option, right? In fact, Clang 15 seems to work too -- it seems the compiler does not error if the option is not within [0,3] (unlike GCC). Kees: this should only affect `__builtin_object_size` and not `sizeof`, right? From a quick test across Clang 14/15/16 and different levels for the flag (0/1/2/3/4 and no flag), bindgen seems to generate the same output in all cases: `__IncompleteArrayField` is always generated for `[]` and `[0]`; and never for `[1]`, i.e. regardless of the flag. The `sizeof`s agree with the C side, and they are all the same (as expected because this only changes BOS). So I think the patch contents (i.e. ignoring the flag for bindgen) should be fine. Except we could have somewhere a `__builtin_object_size` influencing a layout. Since GCC seems to always treat it as not a constexpr (unlike Clang), I assume nobody is using it like that (since we compile the kernel with and without the flag). But it is still possible in theory -- it would require something like: struct X { int x[__is_constexpr(__builtin_object_size(s->c, 1)) ? 1 : 2]; }; to make GCC and Clang (as well as Clang `=3` vs. Clang without the flag) to compile but disagree on the size: https://godbolt.org/z/6TqPjGK3d Cheers, Miguel
On Tue, Aug 15, 2023 at 2:06 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > to make GCC and Clang (as well as Clang `=3` vs. Clang without the > flag) to compile but disagree on the size: To be clear: Clang `=3` vs. Clang without the flag should not be a problem for us because under `CC_IS_CLANG` we pass the flags unaltered to bindgen. But for GCC-built kernels with Rust enabled, the disagreement above could be a problem. Cheers, Miguel
On Tue, Aug 15, 2023 at 02:06:36PM +0200, Miguel Ojeda wrote: > On Tue, Aug 15, 2023 at 8:54 AM Andrea Righi <andrea.righi@canonical.com> wrote: > > > > Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") enabled > > '-fstrict-flex-arrays=3' globally, but bindgen does not recognized this > > It may be more accurate to say libclang here (bindgen forwards the options). > > Also, df8fc4e934c1 did it so only conditionally (if the C compiler > supports it). This explains what you are seeing: if I am right, you > are compiling with a modern enough GCC, which enables the option, but > with an old enough Clang. Oh yes, indeed! I'm using clang/libclang-14 with gcc-13, if I switch to clang-15 everything seems fine, so I can simply move to this version. > > > compiler option, triggering the following build error: > > > > error: unknown argument: '-fstrict-flex-arrays=3', err: true > > This should only be true with libclang < 16, since Clang 16 > implemented the option, right? > > In fact, Clang 15 seems to work too -- it seems the compiler does not > error if the option is not within [0,3] (unlike GCC). The combo gcc-13 + libclang-15 seems to work in my case, instead with libclang-16 I get this: BINDGEN rust/bindings/bindings_generated.rs thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', /build/rust-bindgen-0.56-DgAMvF/rust-bindgen-0.56-0.56.0/debian/vendor/proc-macro2-1.0.24/src/fallback.rs:693:9 Thanks, -Andrea
On Tue, Aug 15, 2023 at 2:44 PM Andrea Righi <andrea.righi@canonical.com> wrote: > > Oh yes, indeed! I'm using clang/libclang-14 with gcc-13, if I switch to > clang-15 everything seems fine, so I can simply move to this version. Thanks for confirming! > The combo gcc-13 + libclang-15 seems to work in my case, instead with > libclang-16 I get this: > > BINDGEN rust/bindings/bindings_generated.rs > thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', /build/rust-bindgen-0.56-DgAMvF/rust-bindgen-0.56-0.56.0/debian/vendor/proc-macro2-1.0.24/src/fallback.rs:693:9 Yeah, this is fixed in `bindgen` 0.62.0. We have upgraded in `rust-next`, commit 08ab786556ff ("rust: bindgen: upgrade to 0.65.1"). Cheers, Miguel
On Tue, Aug 15, 2023 at 02:06:36PM +0200, Miguel Ojeda wrote: > On Tue, Aug 15, 2023 at 8:54 AM Andrea Righi <andrea.righi@canonical.com> wrote: > > > > Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") enabled > > '-fstrict-flex-arrays=3' globally, but bindgen does not recognized this > > It may be more accurate to say libclang here (bindgen forwards the options). > > Also, df8fc4e934c1 did it so only conditionally (if the C compiler > supports it). This explains what you are seeing: if I am right, you > are compiling with a modern enough GCC, which enables the option, but > with an old enough Clang. > > > compiler option, triggering the following build error: > > > > error: unknown argument: '-fstrict-flex-arrays=3', err: true > > This should only be true with libclang < 16, since Clang 16 > implemented the option, right? > > In fact, Clang 15 seems to work too -- it seems the compiler does not > error if the option is not within [0,3] (unlike GCC). > > Kees: this should only affect `__builtin_object_size` and not `sizeof`, right? That's correct. It would affect __builtin_object_size, __builtin_dynamic_object_size, and -fsantize=bounds instrumentation.
On Tue, 15 Aug 2023 08:53:46 +0200 Andrea Righi <andrea.righi@canonical.com> wrote: > Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") enabled > '-fstrict-flex-arrays=3' globally, but bindgen does not recognized this > compiler option, triggering the following build error: > > error: unknown argument: '-fstrict-flex-arrays=3', err: true > > Add '-fstrict-flex-arrays' to the list of cflags that should be ignored > by bindgen. > > Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> Tested-by: Gary Guo <gary@garyguo.net> > --- > rust/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/rust/Makefile b/rust/Makefile > index 4124bfa01798..ae2f5421da25 100644 > --- a/rust/Makefile > +++ b/rust/Makefile > @@ -258,6 +258,7 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \ > -fno-reorder-blocks -fno-allow-store-data-races -fasan-shadow-offset=% \ > -fzero-call-used-regs=% -fno-stack-clash-protection \ > -fno-inline-functions-called-once -fsanitize=bounds-strict \ > + -fstrict-flex-arrays=% \ > --param=% --param asan-% > > # Derived from `scripts/Makefile.clang`.
On Tue, Aug 15, 2023 at 8:54 AM Andrea Righi <andrea.righi@canonical.com> wrote: > > Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") enabled > '-fstrict-flex-arrays=3' globally, but bindgen does not recognized this > compiler option, triggering the following build error: > > error: unknown argument: '-fstrict-flex-arrays=3', err: true > > Add '-fstrict-flex-arrays' to the list of cflags that should be ignored > by bindgen. > > Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> Applied to `rust-fixes` (with an extra summary of the discussion in the list), thanks everyone! Cheers, Miguel
diff --git a/rust/Makefile b/rust/Makefile index 4124bfa01798..ae2f5421da25 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -258,6 +258,7 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \ -fno-reorder-blocks -fno-allow-store-data-races -fasan-shadow-offset=% \ -fzero-call-used-regs=% -fno-stack-clash-protection \ -fno-inline-functions-called-once -fsanitize=bounds-strict \ + -fstrict-flex-arrays=% \ --param=% --param asan-% # Derived from `scripts/Makefile.clang`.
Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") enabled '-fstrict-flex-arrays=3' globally, but bindgen does not recognized this compiler option, triggering the following build error: error: unknown argument: '-fstrict-flex-arrays=3', err: true Add '-fstrict-flex-arrays' to the list of cflags that should be ignored by bindgen. Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") Signed-off-by: Andrea Righi <andrea.righi@canonical.com> --- rust/Makefile | 1 + 1 file changed, 1 insertion(+)