[PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[Zehui Xu]

GCC 14 recently added -fmin-function-alignment option and the
root Makefile uses it to replace -falign-functions when available.
However, this flag can cause issues when passed to the Rust
Makefile and affect the bindgen process. Bindgen relies on
libclang to parse C code, and currently does not support the
-fmin-function-alignment flag, leading to compilation failures
when GCC 14 is used.

This patch addresses the issue by adding -fmin-function-alignment
to the bindgen_skip_c_flags in rust/Makefile. This prevents the
flag from causing compilation issues.

Link: https://lore.kernel.org/linux-kbuild/20240222133500.16991-1-petr.pavlu@suse.com/
Signed-off-by: Zehui Xu <zehuixu@whu.edu.cn>
---
Since -falign-functions does not affect bindgen output, we do not
need logic to add it back to the flags. Thanks to the community's
help, especially Miguel Ojeda. Hope this patch is free of problems
and can be submitted smoothly : )

v1:
* https://lore.kernel.org/all/20240730222053.37066-1-zehuixu@whu.edu.cn/

v2:
* Added -falign-functions to bindgen_extra_c_flags when skipping 
  -fmin-function-alignment to maintain function alignment settings in GCC 14
* Used reasonable length and moved email content out of the commit message
* Used "Link" tag instead of "Reference:" and removed empty lines between tags
* Specified the base commit
* https://lore.kernel.org/all/20240731034112.6060-1-zehuixu@whu.edu.cn/

v3:
* Removed logic from patch v2 which adds -falign-functions

 rust/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/Makefile b/rust/Makefile
index 1f10f92737f2..0c8736cce64f 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -227,7 +227,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=% \
+	-fstrict-flex-arrays=% -fmin-function-alignment=% \
 	--param=% --param asan-%
 
 # Derived from `scripts/Makefile.clang`.

base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
-- 
2.45.2

Re: [PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[Alice Ryhl]

On Wed, Jul 31, 2024 at 3:45 PM Zehui Xu <zehuixu@whu.edu.cn> wrote:
>
> GCC 14 recently added -fmin-function-alignment option and the
> root Makefile uses it to replace -falign-functions when available.
> However, this flag can cause issues when passed to the Rust
> Makefile and affect the bindgen process. Bindgen relies on
> libclang to parse C code, and currently does not support the
> -fmin-function-alignment flag, leading to compilation failures
> when GCC 14 is used.
>
> This patch addresses the issue by adding -fmin-function-alignment
> to the bindgen_skip_c_flags in rust/Makefile. This prevents the
> flag from causing compilation issues.
>
> Link: https://lore.kernel.org/linux-kbuild/20240222133500.16991-1-petr.pavlu@suse.com/
> Signed-off-by: Zehui Xu <zehuixu@whu.edu.cn>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[Neal Gompa]

On Wednesday, July 31, 2024 9:43:46 AM EDT Zehui Xu wrote:
> GCC 14 recently added -fmin-function-alignment option and the
> root Makefile uses it to replace -falign-functions when available.
> However, this flag can cause issues when passed to the Rust
> Makefile and affect the bindgen process. Bindgen relies on
> libclang to parse C code, and currently does not support the
> -fmin-function-alignment flag, leading to compilation failures
> when GCC 14 is used.
> 
> This patch addresses the issue by adding -fmin-function-alignment
> to the bindgen_skip_c_flags in rust/Makefile. This prevents the
> flag from causing compilation issues.
> 
> Link:
> https://lore.kernel.org/linux-kbuild/20240222133500.16991-1-petr.pavlu@suse
> .com/ Signed-off-by: Zehui Xu <zehuixu@whu.edu.cn>
> ---
> Since -falign-functions does not affect bindgen output, we do not
> need logic to add it back to the flags. Thanks to the community's
> help, especially Miguel Ojeda. Hope this patch is free of problems
> and can be submitted smoothly : )
> 
> v1:
> * https://lore.kernel.org/all/20240730222053.37066-1-zehuixu@whu.edu.cn/
> 
> v2:
> * Added -falign-functions to bindgen_extra_c_flags when skipping
>   -fmin-function-alignment to maintain function alignment settings in GCC 14
> * Used reasonable length and moved email content out of the commit message
> * Used "Link" tag instead of "Reference:" and removed empty lines between
> tags * Specified the base commit
> * https://lore.kernel.org/all/20240731034112.6060-1-zehuixu@whu.edu.cn/
> 
> v3:
> * Removed logic from patch v2 which adds -falign-functions
> 
>  rust/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rust/Makefile b/rust/Makefile
> index 1f10f92737f2..0c8736cce64f 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -227,7 +227,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=% \
> +	-fstrict-flex-arrays=% -fmin-function-alignment=% \
>  	--param=% --param asan-%
> 
>  # Derived from `scripts/Makefile.clang`.
> 
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b

Looks good to me.

Reviewed-by: Neal Gompa <neal@gompa.dev>

-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[Neal Gompa]

On Wed, Aug 7, 2024 at 3:52 AM Neal Gompa <neal@gompa.dev> wrote:
>
> On Wednesday, July 31, 2024 9:43:46 AM EDT Zehui Xu wrote:
> > GCC 14 recently added -fmin-function-alignment option and the
> > root Makefile uses it to replace -falign-functions when available.
> > However, this flag can cause issues when passed to the Rust
> > Makefile and affect the bindgen process. Bindgen relies on
> > libclang to parse C code, and currently does not support the
> > -fmin-function-alignment flag, leading to compilation failures
> > when GCC 14 is used.
> >
> > This patch addresses the issue by adding -fmin-function-alignment
> > to the bindgen_skip_c_flags in rust/Makefile. This prevents the
> > flag from causing compilation issues.
> >
> > Link:
> > https://lore.kernel.org/linux-kbuild/20240222133500.16991-1-petr.pavlu@suse
> > .com/ Signed-off-by: Zehui Xu <zehuixu@whu.edu.cn>
> > ---
> > Since -falign-functions does not affect bindgen output, we do not
> > need logic to add it back to the flags. Thanks to the community's
> > help, especially Miguel Ojeda. Hope this patch is free of problems
> > and can be submitted smoothly : )
> >
> > v1:
> > * https://lore.kernel.org/all/20240730222053.37066-1-zehuixu@whu.edu.cn/
> >
> > v2:
> > * Added -falign-functions to bindgen_extra_c_flags when skipping
> >   -fmin-function-alignment to maintain function alignment settings in GCC 14
> > * Used reasonable length and moved email content out of the commit message
> > * Used "Link" tag instead of "Reference:" and removed empty lines between
> > tags * Specified the base commit
> > * https://lore.kernel.org/all/20240731034112.6060-1-zehuixu@whu.edu.cn/
> >
> > v3:
> > * Removed logic from patch v2 which adds -falign-functions
> >
> >  rust/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/rust/Makefile b/rust/Makefile
> > index 1f10f92737f2..0c8736cce64f 100644
> > --- a/rust/Makefile
> > +++ b/rust/Makefile
> > @@ -227,7 +227,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=% \
> > +     -fstrict-flex-arrays=% -fmin-function-alignment=% \
> >       --param=% --param asan-%
> >
> >  # Derived from `scripts/Makefile.clang`.
> >
> > base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
>
> Looks good to me.
>
> Reviewed-by: Neal Gompa <neal@gompa.dev>
>

To add, can we get this in as a fix for Linux 6.11? I can't build
Fedora kernels with Rust stuff enabled without it since GCC 14 is
shipped in Fedora Linux 40+.


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[Miguel Ojeda]

On Thu, Aug 8, 2024 at 9:38 AM Neal Gompa <neal@gompa.dev> wrote:
>
> To add, can we get this in as a fix for Linux 6.11? I can't build
> Fedora kernels with Rust stuff enabled without it since GCC 14 is
> shipped in Fedora Linux 40+.

Yeah, that is the plan -- I am picking another batch now.

Cheers,
Miguel

Re: [PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[Gary Guo]

On Wed, 31 Jul 2024 16:43:46 +0300
Zehui Xu <zehuixu@whu.edu.cn> wrote:

> GCC 14 recently added -fmin-function-alignment option and the
> root Makefile uses it to replace -falign-functions when available.
> However, this flag can cause issues when passed to the Rust
> Makefile and affect the bindgen process. Bindgen relies on
> libclang to parse C code, and currently does not support the
> -fmin-function-alignment flag, leading to compilation failures
> when GCC 14 is used.
> 
> This patch addresses the issue by adding -fmin-function-alignment
> to the bindgen_skip_c_flags in rust/Makefile. This prevents the
> flag from causing compilation issues.

This patch itself looks fine to me, so

Reviewed-by: Gary Guo <gary@garyguo.net>

We use -fmin-function-alignment for GCC and use -falign-functions for
clang. These options affect codegen only so it's fine to ignore them
for bindgen.

Although it appears to me that we currently don't set function
alignments for Rust codegen. Maybe that's worth considering adding?

- Gary

> 
> Link: https://lore.kernel.org/linux-kbuild/20240222133500.16991-1-petr.pavlu@suse.com/
> Signed-off-by: Zehui Xu <zehuixu@whu.edu.cn>
> ---
> Since -falign-functions does not affect bindgen output, we do not
> need logic to add it back to the flags. Thanks to the community's
> help, especially Miguel Ojeda. Hope this patch is free of problems
> and can be submitted smoothly : )
> 
> v1:
> * https://lore.kernel.org/all/20240730222053.37066-1-zehuixu@whu.edu.cn/
> 
> v2:
> * Added -falign-functions to bindgen_extra_c_flags when skipping 
>   -fmin-function-alignment to maintain function alignment settings in GCC 14
> * Used reasonable length and moved email content out of the commit message
> * Used "Link" tag instead of "Reference:" and removed empty lines between tags
> * Specified the base commit
> * https://lore.kernel.org/all/20240731034112.6060-1-zehuixu@whu.edu.cn/
> 
> v3:
> * Removed logic from patch v2 which adds -falign-functions
> 
>  rust/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Re: [PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[Miguel Ojeda]

On Wed, Aug 7, 2024 at 12:57 PM Gary Guo <gary@garyguo.net> wrote:
>
> This patch itself looks fine to me, so
>
> Reviewed-by: Gary Guo <gary@garyguo.net>
>
> We use -fmin-function-alignment for GCC and use -falign-functions for
> clang. These options affect codegen only so it's fine to ignore them
> for bindgen.

Thanks Gary.

> Although it appears to me that we currently don't set function
> alignments for Rust codegen. Maybe that's worth considering adding?

Yeah, agreed. I think currently there is no way for that in `rustc`
(apart from `fn_align`) -- I have created an issue about it and a few
others and linked them all at
https://github.com/Rust-for-Linux/linux/issues/355.

Cheers,
Miguel

Re: [PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[Gary Guo]

On Thu, 8 Aug 2024 17:07:51 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Wed, Aug 7, 2024 at 12:57 PM Gary Guo <gary@garyguo.net> wrote:
> >
> > This patch itself looks fine to me, so
> >
> > Reviewed-by: Gary Guo <gary@garyguo.net>
> >
> > We use -fmin-function-alignment for GCC and use -falign-functions for
> > clang. These options affect codegen only so it's fine to ignore them
> > for bindgen.  
> 
> Thanks Gary.
> 
> > Although it appears to me that we currently don't set function
> > alignments for Rust codegen. Maybe that's worth considering adding?  
> 
> Yeah, agreed. I think currently there is no way for that in `rustc`
> (apart from `fn_align`) -- I have created an issue about it and a few
> others and linked them all at
> https://github.com/Rust-for-Linux/linux/issues/355.


I think `-Cllvm-args=-align-all-functions=foo` should work?

Best,
Gary

Re: [PATCH v3] rust: Kbuild: Skip -fmin-function-alignment in bindgen flags

[Miguel Ojeda]

On Wed, Jul 31, 2024 at 3:45 PM Zehui Xu <zehuixu@whu.edu.cn> wrote:
>
> GCC 14 recently added -fmin-function-alignment option and the
> root Makefile uses it to replace -falign-functions when available.
> However, this flag can cause issues when passed to the Rust
> Makefile and affect the bindgen process. Bindgen relies on
> libclang to parse C code, and currently does not support the
> -fmin-function-alignment flag, leading to compilation failures
> when GCC 14 is used.
>
> This patch addresses the issue by adding -fmin-function-alignment
> to the bindgen_skip_c_flags in rust/Makefile. This prevents the
> flag from causing compilation issues.
>
> Link: https://lore.kernel.org/linux-kbuild/20240222133500.16991-1-petr.pavlu@suse.com/
> Signed-off-by: Zehui Xu <zehuixu@whu.edu.cn>

Applied to `rust-fixes` -- thanks everyone!

    [ Matthew and Gary confirm function alignment should not change
      the ABI in a way that bindgen would care about, thus we did
      not need the extra logic for bindgen from v2. - Miguel ]

    [ Reworded title. - Miguel ]

Cheers,
Miguel