Re: [Buildroot] [PATCH] package/elfutils: ensure we have 5.0 kernel headers

[Julien Olivain <ju.o@free.fr>]

Hi Alex,

Thanks for the patch!  I have few comments on it.

Could refine the patch title with:
"package/elfutils: ensure we have 5.0 kernel headers for aarch64"
to better reflect what it is changing.

On 13/02/2025 19:15, Alex Bennée wrote:
> It seems some of the external toolchains have fairly old headers. This
> can cause a build failure of elfutils on Aarch64 due to a missing
> definition of struct user_pac_mask.
> 
> Prevent the user selecting elfutils unless the kernel headers will
> allow it to build.

Could you add more information in the commit log:
- when the requirement was introduced upstream,
- when it started to fail in Buildroot,
- detail of the error,
- links to resolved autobuild failures if any.

For example:

"""
Since Buildroot commit [1] "package/elfutils: bump to version 0.192"
elfutils fails to build on Aarch64 with toolchain including Kernel older
than version 5.0. Error shows:

     aarch64_initreg.c: In function 'aarch64_set_initial_registers_tid':
     aarch64_initreg.c:61:24: error: storage size of 'pac_mask' isn't 
known
        61 |   struct user_pac_mask pac_mask;
           |                        ^~~~~~~~

elfutils 0.192 introduced a Aarch64 pointer authentication support in
upstream commit [2].

For reference, the user_pac_mask structure was introduced in Kernel
upstream commit [3], included in Kernel v5.0.

Fixes:
- 
https://autobuild.buildroot.org/results/5156901a73be52ce8ffbf10729e6852499a300be/
- and many others...

[1] 
https://gitlab.com/buildroot.org/buildroot/-/commit/5eb734766b24de5fce6e586897cd4827f8855442
[2] 
https://sourceware.org/git/?p=elfutils.git;a=commitdiff;h=64e3b451ad2cec8d45661b1816e3d2dc4431f3ca
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=ec6e822d1a22d0eef1d1fa260dff751dba9a4258
"""

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  package/elfutils/Config.in | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/package/elfutils/Config.in b/package/elfutils/Config.in
> index c355048c6d..4995b4f0d2 100644
> --- a/package/elfutils/Config.in
> +++ b/package/elfutils/Config.in
> @@ -1,12 +1,14 @@
> -comment "elfutils needs a toolchain w/ wchar, dynamic library, 
> threads"
> +comment "elfutils needs a toolchain w/ wchar, dynamic library, 
> threads, (kernel headers >5.0 for AArch64)"

Here, the shown condition should be:
"kernel headers >= 5.0 for AArch64".

>  	depends on !BR2_USE_WCHAR || BR2_STATIC_LIBS \
> -		|| !BR2_TOOLCHAIN_HAS_THREADS
> +		|| !BR2_TOOLCHAIN_HAS_THREADS \
> +		&& (BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_0 || !BR2_aarch64)

We want to display the comment when the condition is not met. So
the condition should be reversed to:

     || (!BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_0 && BR2_aarch64)

> 
>  config BR2_PACKAGE_ELFUTILS
>  	bool "elfutils"
>  	depends on BR2_USE_WCHAR
>  	depends on !BR2_STATIC_LIBS
>  	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_0 || !BR2_aarch64 # 
> user_pac_mask

There is several packages selecting elfutils. So this patch should
propagate this condition on them. See the output of the command:

     git grep 'select BR2_PACKAGE_ELFUTILS' package/*/Config.in

This means those package should have a line:

     depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_0 || !BR2_aarch64 # 
elfutils

in their Config.in.

>  	select BR2_PACKAGE_ZLIB
>  	select BR2_PACKAGE_ARGP_STANDALONE if !BR2_TOOLCHAIN_USES_GLIBC
>  	select BR2_PACKAGE_MUSL_FTS if !BR2_TOOLCHAIN_USES_GLIBC
> --
> 2.39.5

Could you send a v2 addressing those comments, please?

Best regards,

Julien.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot