Re: [PATCH v2] kbuild: add --target to correctly cross-compile UAPI headers with Clang

[Masahiro Yamada]

On Fri, Mar 11, 2022 at 4:16 AM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> On Sat, Mar 5, 2022 at 4:56 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
> >
> > When you compile-test UAPI headers (CONFIG_UAPI_HEADER_TEST=y) with
> > Clang, they are currently compiled for the host target (likely x86_64)
> > regardless of the given ARCH=.
> >
> > In fact, some exported headers include libc headers. For example,
> > include/uapi/linux/agpgart.h includes <stdlib.h> after being exported.
> > The header search paths should match to the target we are compiling
> > them for.
>
> Isn't that a bug in that header though? Why does it inconsistently use
> size_t vs. __kernel_size_t. Shouldn't it be consistently using
> __kernel_size_t? (Seeing TRUE/FALSE defined in such a low level header
> is also *yikes*.) Are there platforms where sizeof(size_t) !=
> sizeof(__kernel_size_t)?

size_t and __kernel_size_t are always the same.

For uapi headers  (include/uapi, arch/*/include/uapi), __kernel_size_t
should be used.


See this series, which is already available in linux-next.

https://lore.kernel.org/all/20220210021129.3386083-1-masahiroy@xxxxxxxxxx/


>
> Usually to bootstrap a toolchain you need to start with kernel headers
> to bootstrap the libc.  It seems like some kind of circular dependency
> to me if kernel headers are dependent on libc headers.

Right, ideally, UAPI headers should be self-contained.
Hence, CONFIG_UAPI_HEADER_TEST exists.
But, I am not sure how far we can obey this rule.

Arnd Bergmann is an expert in this area.

The number of "no-header-test" in usr/include/Makefile
is gradually decreasing, but there are still more than 30.
They must be checked one by one.





> Hence my
> previous comment about -ffreestanding.

To ensure that we are not including any header from the system,
-nostdinc is a more correct flag.


-ffreestanding still allows system headers to be included.


I am skeptical about adding -ffreestanding here.
Please note we are compiling the user-space on the operating
system (this is what is called "hosted environment", which is
opposed to the "freestanding environment").




>
> >
> > Pick up the --target triple from KBUILD_CFLAGS in the same ways as
> > commit 7f58b487e9ff ("kbuild: make Clang build userprogs for target
> > architecture").
>
> Oh boy thanks for finding+fixing this! I still suspect we shouldn't
> need a cross-libc for UAPI header testing, and that the kernel headers
> simply need to be cleaned up. But regardless of that it doesn't make
> sense to use the wrong target when checking headers generated for the
> target. Thanks for the patch (and sorry I've been falling behind on
> code review lately).
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
>
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > ---
> >
> > Changes in v2:
> >   - Reword the commit description to mention agpgart.h instead of
> >     asound.h because the latter is in the no-header-test list.
> >
> >  usr/include/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/usr/include/Makefile b/usr/include/Makefile
> > index ac206fb27c65..4215801e1110 100644
> > --- a/usr/include/Makefile
> > +++ b/usr/include/Makefile
> > @@ -10,7 +10,7 @@ UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration
> >
> >  # In theory, we do not care -m32 or -m64 for header compile tests.
> >  # It is here just because CONFIG_CC_CAN_LINK is tested with -m32 or -m64.
> > -UAPI_CFLAGS += $(filter -m32 -m64, $(KBUILD_CFLAGS))
> > +UAPI_CFLAGS += $(filter -m32 -m64 --target=%, $(KBUILD_CFLAGS))
> >
> >  # USERCFLAGS might contain sysroot location for CC.
> >  UAPI_CFLAGS += $(USERCFLAGS)
> > --
> > 2.32.0
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Best Regards
Masahiro Yamada