GCC 14 recently added the -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 and maintains compatibility.

In addition, since -falign-functions is not used when
-fmin-function-alignment is available but skipped and libclang
supports -falign-functions, this patch adds it back to the
bindgen_extra_c_flags to ensure the intended function alignment
is maintained.

Link: https://lore.kernel.org/linux-kbuild/20240222133500.16991-1-petr.pavlu@suse.com/
Signed-off-by: Zehui Xu <zehuixu@whu.edu.cn>
---
Hello again!

This is the second version of my patch. I discovered some issues right after
sending the email. Since this is my first kernel patch, I was too excited.
I will be more careful and calm during my checks next time. Compared to the
first patch, this version includes not only changes to the format of the
commit message (thanks to Miguel Ojeda for the patient guidance) but also
some code changes.

While preparing the second version of the patch, I realized that the root
Makefile replaces -falign-functions with -fmin-function-alignment, and if we
simply skip we'll lose this config. To ensure consistency, I added it back in
bindgen_extra_c_flags. Though I am uncertain if this is a good approach.

Looking for suggestions and thank you all for your time!

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 to wrap commit messages
* Moved email content out of the commit message
* Used the "Link" tag instead of "Reference:" and removed empty lines between tags
* Specified the base commit

 rust/Makefile | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/rust/Makefile b/rust/Makefile
index 1f10f92737f2..bb21e74abd87 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`.
@@ -254,6 +254,16 @@ bindgen_extra_c_flags += -enable-trivial-auto-var-init-zero-knowing-it-will-be-r
 endif
 endif
 
+# Starting from GCC 14, the root Makefile uses -fmin-function-alignment to replace
+# -falign-functions, which libclang doesn't support. We skipped
+# -fmin-function-alignment in bindgen_skip_c_flags, resulting in missing
+# compilation flags. Therefore we add -falign-functions in bindgen_extra_c_flags
+# to fall back and complete the alignment settings.
+# https://lore.kernel.org/linux-kbuild/20240222133500.16991-1-petr.pavlu@suse.com/
+ifdef CONFIG_CC_HAS_MIN_FUNCTION_ALIGNMENT
+bindgen_extra_c_flags += -falign-functions=$(CONFIG_FUNCTION_ALIGNMENT)
+endif
+
 bindgen_c_flags = $(filter-out $(bindgen_skip_c_flags), $(c_flags)) \
 	$(bindgen_extra_c_flags)
 endif

base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
-- 
2.45.2

On Wed, Jul 31, 2024 at 5:41 AM Zehui Xu <zehuixu@whu.edu.cn> wrote:
>
> In addition, since -falign-functions is not used when
> -fmin-function-alignment is available but skipped and libclang
> supports -falign-functions, this patch adds it back to the
> bindgen_extra_c_flags to ensure the intended function alignment
> is maintained.

Does it change the ABI or could change `bindgen`'s output in a way we care?

If we do need it, then I think passing `-falign-functions` makes
sense, since it is the only one that Clang supports and has the same
behavior as `-fmin-function-alignment` in GCC (i.e. applies it to all
functions).

If not, then it may be best to avoid unneeded complexity.

Thanks!

Cheers,
Miguel

> Does it change the ABI or could change `bindgen`'s output in a way we care?
>
This should not change the calling ABI in a way bindgen would care about;
Those flags only control how functions are defined, not how they're used.

On Wed, Jul 31, 2024 at 9:57 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> This should not change the calling ABI in a way bindgen would care about;
> Those flags only control how functions are defined, not how they're used.

Thanks Matthew for confirming. Let's go with Zehui's v3 then.

Cheers,
Miguel

On 7/31/24 13:18, Miguel Ojeda wrote:

> On Wed, Jul 31, 2024 at 5:41 AM Zehui Xu <zehuixu@whu.edu.cn> wrote:
>>
>> In addition, since -falign-functions is not used when
>> -fmin-function-alignment is available but skipped and libclang
>> supports -falign-functions, this patch adds it back to the
>> bindgen_extra_c_flags to ensure the intended function alignment
>> is maintained.
>
> Does it change the ABI or could change `bindgen`'s output in a way we care?

You are right, it should not change the ABI or the bindgen output in
a way that affects us, as bindgen only generates .rs Rust bindings and,
after comparison, the generated results are exactly the same with or
without -falign-functions.

> If we do need it, then I think passing `-falign-functions` makes
> sense, since it is the only one that Clang supports and has the same
> behavior as `-fmin-function-alignment` in GCC (i.e. applies it to all
> functions).
>
> If not, then it may be best to avoid unneeded complexity.

Considering this, I will send a patch v3. Thanks!

> Thanks!
>
> Cheers,
> Miguel

Cheers,
Zehui