| Message ID | 20220112224342.958358-1-quic_eberman@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: Add environment variables for userprogs flags | expand |
On Wed, Jan 12, 2022 at 2:44 PM Elliot Berman <quic_eberman@quicinc.com> wrote: > > Allow additional arguments be passed to userprogs compilation. > Reproducible clang builds need to provide a sysroot and gcc path to > ensure same toolchain is used across hosts. KCFLAGS is not currently > used for any user programs compilation, so add new USERCFLAGS and > USERLDFLAGS which serves similar purpose as HOSTCFLAGS/HOSTLDFLAGS. > > Specifically, I'm trying to force CC_CAN_LINK to consistently fail in > an environment where a user sysroot is not specifically available. > Currently, Clang might automatically detect GCC installation on hosts > which have it installed to a default location in /. With addition of > these environment variables, our build environment can do like > "--sysroot=/dev/null" to force sysroot detection to fail. Hi Elliot, Thanks for the patch! Sorry for the delay in reviewing; I didn't quite get around to it then went on vacation for a week. Things get buried in my inbox quickly; feel free to ping me if a week goes by with no response on whichever channel works best for you. I'm happy with the intent of this patch; GNU binutils has been removed from Android, so supporting CC_CAN_LINK for Android kernel builds has been a question I've been thinking about (though, not with higher priority with some of our other issues), since we'll need to either incorporate musl or bionic libc into our kernel build. I was thinking of adding a SYSROOT command line variable for that, but I see your approach is more flexible. One minor nit below, a typo, a few questions, and in the commit message, but this generally LGTM. For the commit message, I think it would be good to expand `can do like "--sysroot=/dev/null"` fully into ``` can specify $ make USERCFLAGS=--sysroot=/dev/null USERLDFLAGS=-Wl,--sysroot=/dev/null ``` > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > --- > Documentation/kbuild/kbuild.rst | 8 ++++++++ > Makefile | 9 ++++++--- > init/Kconfig | 8 ++++---- > usr/include/Makefile | 3 +++ > 4 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst > index 2d1fc03d346e..16e90a3ae01b 100644 > --- a/Documentation/kbuild/kbuild.rst > +++ b/Documentation/kbuild/kbuild.rst > @@ -77,6 +77,14 @@ HOSTLDLIBS > ---------- > Additional libraries to link against when building host programs. > > +USERCFLAGS > +---------- > +Additional options used for $(CC) when compiling userprogs. > + > +USERLDFLAGS > +---------- > +Additional options used for $(LD) when linking userprogs. Probably should note the necessity of `-Wl,` prefixes here. Is `userprogs` cannonical? Yeah, I guess (reading Documentation/kbuild/makefiles.rst). I wonder if we should mention these in Documentation/kbuild/makefiles.rst as well? Under `5.3 Controlling compiler options for userspace programs`. > + > KBUILD_KCONFIG > -------------- > Set the top-level Kconfig file to the value of this environment > diff --git a/Makefile b/Makefile > index 45278d508d81..4a55537c8ca0 100644 > --- a/Makefile > +++ b/Makefile > @@ -431,15 +431,17 @@ HOSTCC = gcc > HOSTCXX = g++ > endif > > -export KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ > - -O2 -fomit-frame-pointer -std=gnu89 > -export KBUILD_USERLDFLAGS := > +KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ > + -O2 -fomit-frame-pointer -std=gnu89 > +KBUILD_USERLDFLAGS := $(USERLDFLAGGS) ^ I think there's an extra G in USERLDFLAGS above. > > KBUILD_HOSTCFLAGS := $(KBUILD_USERCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS) > KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) > KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS) > KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS) > > +KBUILD_USERCFLAGS += $(USERCFLAGS) Since you added USERLDFLAGS to KBUILD_USERLDFLAGS above where it's defined, why not do so for USERCFLAGS/KBUILD_USERCFLAGS as well? > + > # Make variables (CC, etc...) > CPP = $(CC) -E > ifneq ($(LLVM),) > @@ -530,6 +532,7 @@ export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AW > export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX > export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD > export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE > +export KBUILD_USERCFLAGS KBUILD_USERLDFLAGS > > export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS KBUILD_LDFLAGS > export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE > diff --git a/init/Kconfig b/init/Kconfig > index f2ae41e6717f..164706c38e8b 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -62,13 +62,13 @@ config LLD_VERSION > > config CC_CAN_LINK > bool > - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag)) if 64BIT > - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag)) > + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag)) if 64BIT > + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag)) > > config CC_CAN_LINK_STATIC > bool > - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT > - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag) -static) > + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag) -static) if 64BIT > + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag) -static) since USERLDFLAGS get passed to $(CC), they will need `-Wl`, prefixes, hence the request for expanding the example usage in the commit message. > > config CC_HAS_ASM_GOTO > def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC)) > diff --git a/usr/include/Makefile b/usr/include/Makefile > index 1c2ae1368079..6a8c7dd9ccaf 100644 > --- a/usr/include/Makefile > +++ b/usr/include/Makefile > @@ -12,6 +12,9 @@ UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration > # It is here just because CONFIG_CC_CAN_LINK is tested with -m32 or -m64. > UAPI_CFLAGS += $(filter -m32 -m64, $(KBUILD_CFLAGS)) > > +# USERCFLAGS might contain sysroot location for CC > +UAPI_CFLAGS += $(USERCFLAGS) > + Do we need to worry about USERLDFLAGS here, too? (or usr/Makefile?) > override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) -I$(objtree)/usr/include > > # The following are excluded for now because they fail to build. > -- > 2.25.1 >
On Thu, Jan 27, 2022 at 3:21 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > config CC_HAS_ASM_GOTO > > def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC)) > > diff --git a/usr/include/Makefile b/usr/include/Makefile > > index 1c2ae1368079..6a8c7dd9ccaf 100644 > > --- a/usr/include/Makefile > > +++ b/usr/include/Makefile > > @@ -12,6 +12,9 @@ UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration > > # It is here just because CONFIG_CC_CAN_LINK is tested with -m32 or -m64. > > UAPI_CFLAGS += $(filter -m32 -m64, $(KBUILD_CFLAGS)) > > > > +# USERCFLAGS might contain sysroot location for CC > > +UAPI_CFLAGS += $(USERCFLAGS) > > + > Do we need to worry about USERLDFLAGS here, too? (or usr/Makefile?) I do not think so. usr/include/Makefile does not link the objects. ( $(CC) -S stops after the compilation stage) -- Best Regards Masahiro Yamada
(+CC: Arnd) On Thu, Jan 13, 2022 at 7:44 AM Elliot Berman <quic_eberman@quicinc.com> wrote: > > Allow additional arguments be passed to userprogs compilation. > Reproducible clang builds need to provide a sysroot and gcc path to > ensure same toolchain is used across hosts. KCFLAGS is not currently > used for any user programs compilation, so add new USERCFLAGS and > USERLDFLAGS which serves similar purpose as HOSTCFLAGS/HOSTLDFLAGS. > > Specifically, I'm trying to force CC_CAN_LINK to consistently fail in > an environment where a user sysroot is not specifically available. > Currently, Clang might automatically detect GCC installation on hosts > which have it installed to a default location in /. With addition of > these environment variables, our build environment can do like > "--sysroot=/dev/null" to force sysroot detection to fail. > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > --- > --- a/usr/include/Makefile > +++ b/usr/include/Makefile > @@ -12,6 +12,9 @@ UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration > # It is here just because CONFIG_CC_CAN_LINK is tested with -m32 or -m64. > UAPI_CFLAGS += $(filter -m32 -m64, $(KBUILD_CFLAGS)) > > +# USERCFLAGS might contain sysroot location for CC > +UAPI_CFLAGS += $(USERCFLAGS) > I am OK with this patch, but I was not sure with this line. Initially, I thought exported UAPI headers should be self-contained. In other words, we should be able to compile-test them without relying on compiler or libc headers. Is this achievable or not? I think Arnd is an expert in this area. I hope some input from him. + > override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) -I$(objtree)/usr/include > > # The following are excluded for now because they fail to build. > -- > 2.25.1 >
On 2022-01-26, Nick Desaulniers wrote: >On Wed, Jan 12, 2022 at 2:44 PM Elliot Berman <quic_eberman@quicinc.com> wrote: >> >> Allow additional arguments be passed to userprogs compilation. >> Reproducible clang builds need to provide a sysroot and gcc path to >> ensure same toolchain is used across hosts. KCFLAGS is not currently >> used for any user programs compilation, so add new USERCFLAGS and >> USERLDFLAGS which serves similar purpose as HOSTCFLAGS/HOSTLDFLAGS. >> >> Specifically, I'm trying to force CC_CAN_LINK to consistently fail in >> an environment where a user sysroot is not specifically available. >> Currently, Clang might automatically detect GCC installation on hosts >> which have it installed to a default location in /. With addition of >> these environment variables, our build environment can do like >> "--sysroot=/dev/null" to force sysroot detection to fail. > >Hi Elliot, >Thanks for the patch! Sorry for the delay in reviewing; I didn't quite >get around to it then went on vacation for a week. Things get buried >in my inbox quickly; feel free to ping me if a week goes by with no >response on whichever channel works best for you. > >I'm happy with the intent of this patch; GNU binutils has been removed >from Android, so supporting CC_CAN_LINK for Android kernel builds has >been a question I've been thinking about (though, not with higher >priority with some of our other issues), since we'll need to either >incorporate musl or bionic libc into our kernel build. I was thinking >of adding a SYSROOT command line variable for that, but I see your >approach is more flexible. > >One minor nit below, a typo, a few questions, and in the commit >message, but this generally LGTM. > >For the commit message, I think it would be good to expand `can do >like "--sysroot=/dev/null"` fully into >``` >can specify >$ make USERCFLAGS=--sysroot=/dev/null USERLDFLAGS=-Wl,--sysroot=/dev/null >``` Is -Wl,--sysroot=/dev/null to override a -Wl,--sysroot specified previously on the command line? The driver option --sysroot does two things: * Decide include/library search paths (e.g. $sysroot/usr/include, $sysroot/lib64). * Pass --sysroot to ld. In ld, it means: if a linker script is in the sysroot directory, when ld opens an absolute path file (via INPUT or GROUP), add sysroot before the absolute path. In ld, --sysroot=/dev/null is not different --sysroot= (empty value). >> >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >> --- >> Documentation/kbuild/kbuild.rst | 8 ++++++++ >> Makefile | 9 ++++++--- >> init/Kconfig | 8 ++++---- >> usr/include/Makefile | 3 +++ >> 4 files changed, 21 insertions(+), 7 deletions(-) >> >> diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst >> index 2d1fc03d346e..16e90a3ae01b 100644 >> --- a/Documentation/kbuild/kbuild.rst >> +++ b/Documentation/kbuild/kbuild.rst >> @@ -77,6 +77,14 @@ HOSTLDLIBS >> ---------- >> Additional libraries to link against when building host programs. >> >> +USERCFLAGS >> +---------- >> +Additional options used for $(CC) when compiling userprogs. >> + >> +USERLDFLAGS >> +---------- >> +Additional options used for $(LD) when linking userprogs. > >Probably should note the necessity of `-Wl,` prefixes here. > >Is `userprogs` cannonical? Yeah, I guess (reading >Documentation/kbuild/makefiles.rst). I wonder if we should mention >these in Documentation/kbuild/makefiles.rst as well? Under `5.3 >Controlling compiler options for userspace programs`. > >> + >> KBUILD_KCONFIG >> -------------- >> Set the top-level Kconfig file to the value of this environment >> diff --git a/Makefile b/Makefile >> index 45278d508d81..4a55537c8ca0 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -431,15 +431,17 @@ HOSTCC = gcc >> HOSTCXX = g++ >> endif >> >> -export KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ >> - -O2 -fomit-frame-pointer -std=gnu89 >> -export KBUILD_USERLDFLAGS := >> +KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ >> + -O2 -fomit-frame-pointer -std=gnu89 >> +KBUILD_USERLDFLAGS := $(USERLDFLAGGS) > >^ I think there's an extra G in USERLDFLAGS above. > >> >> KBUILD_HOSTCFLAGS := $(KBUILD_USERCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS) >> KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) >> KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS) >> KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS) >> >> +KBUILD_USERCFLAGS += $(USERCFLAGS) > >Since you added USERLDFLAGS to KBUILD_USERLDFLAGS above where it's >defined, why not do so for USERCFLAGS/KBUILD_USERCFLAGS as well? > >> + >> # Make variables (CC, etc...) >> CPP = $(CC) -E >> ifneq ($(LLVM),) >> @@ -530,6 +532,7 @@ export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AW >> export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX >> export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD >> export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE >> +export KBUILD_USERCFLAGS KBUILD_USERLDFLAGS >> >> export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS KBUILD_LDFLAGS >> export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE >> diff --git a/init/Kconfig b/init/Kconfig >> index f2ae41e6717f..164706c38e8b 100644 >> --- a/init/Kconfig >> +++ b/init/Kconfig >> @@ -62,13 +62,13 @@ config LLD_VERSION >> >> config CC_CAN_LINK >> bool >> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag)) if 64BIT >> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag)) >> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag)) if 64BIT >> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag)) >> >> config CC_CAN_LINK_STATIC >> bool >> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT >> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag) -static) >> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag) -static) if 64BIT >> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag) -static) > >since USERLDFLAGS get passed to $(CC), they will need `-Wl`, prefixes, >hence the request for expanding the example usage in the commit >message. > >> >> config CC_HAS_ASM_GOTO >> def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC)) >> diff --git a/usr/include/Makefile b/usr/include/Makefile >> index 1c2ae1368079..6a8c7dd9ccaf 100644 >> --- a/usr/include/Makefile >> +++ b/usr/include/Makefile >> @@ -12,6 +12,9 @@ UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration >> # It is here just because CONFIG_CC_CAN_LINK is tested with -m32 or -m64. >> UAPI_CFLAGS += $(filter -m32 -m64, $(KBUILD_CFLAGS)) >> >> +# USERCFLAGS might contain sysroot location for CC >> +UAPI_CFLAGS += $(USERCFLAGS) >> + > >Do we need to worry about USERLDFLAGS here, too? (or usr/Makefile?) > >> override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) -I$(objtree)/usr/include >> >> # The following are excluded for now because they fail to build. >> -- >> 2.25.1 >> > > >-- >Thanks, >~Nick Desaulniers
Hi Fangrui, On 1/27/2022 7:15 PM, Fangrui Song wrote: > On 2022-01-26, Nick Desaulniers wrote: >> On Wed, Jan 12, 2022 at 2:44 PM Elliot Berman >> <quic_eberman@quicinc.com> wrote: >>> >>> Allow additional arguments be passed to userprogs compilation. >>> Reproducible clang builds need to provide a sysroot and gcc path to >>> ensure same toolchain is used across hosts. KCFLAGS is not currently >>> used for any user programs compilation, so add new USERCFLAGS and >>> USERLDFLAGS which serves similar purpose as HOSTCFLAGS/HOSTLDFLAGS. >>> >>> Specifically, I'm trying to force CC_CAN_LINK to consistently fail in >>> an environment where a user sysroot is not specifically available. >>> Currently, Clang might automatically detect GCC installation on hosts >>> which have it installed to a default location in /. With addition of >>> these environment variables, our build environment can do like >>> "--sysroot=/dev/null" to force sysroot detection to fail. >> >> Hi Elliot, >> Thanks for the patch! Sorry for the delay in reviewing; I didn't quite >> get around to it then went on vacation for a week. Things get buried >> in my inbox quickly; feel free to ping me if a week goes by with no >> response on whichever channel works best for you. >> >> I'm happy with the intent of this patch; GNU binutils has been removed >> from Android, so supporting CC_CAN_LINK for Android kernel builds has >> been a question I've been thinking about (though, not with higher >> priority with some of our other issues), since we'll need to either >> incorporate musl or bionic libc into our kernel build. I was thinking >> of adding a SYSROOT command line variable for that, but I see your >> approach is more flexible. >> >> One minor nit below, a typo, a few questions, and in the commit >> message, but this generally LGTM. >> >> For the commit message, I think it would be good to expand `can do >> like "--sysroot=/dev/null"` fully into >> ``` >> can specify >> $ make USERCFLAGS=--sysroot=/dev/null USERLDFLAGS=-Wl,--sysroot=/dev/null >> ``` > > Is -Wl,--sysroot=/dev/null to override a -Wl,--sysroot specified > previously on the command line? > --sysroot isn't specified previously on the commandline for Android kernel builds. Clang is falling back to sysroot detection based on common installation paths, and we want to effectively disable that for reproducible builds. I believe this was introduced in Clang 13 [1], but it might also be a change in AOSP Clang's prebuilt. I wasn't able to get clear picture of what changed in Clang. Android12-5.10 builds which use Clang 12 are unaffected. [1]: https://releases.llvm.org/13.0.0/tools/clang/docs/ReleaseNotes.html#modified-compiler-flags > The driver option --sysroot does two things: > > * Decide include/library search paths (e.g. $sysroot/usr/include, > $sysroot/lib64). > * Pass --sysroot to ld. > > In ld, it means: if a linker script is in the sysroot directory, when ld > opens an > absolute path file (via INPUT or GROUP), add sysroot before the absolute > path. > In ld, --sysroot=/dev/null is not different --sysroot= (empty value). > >>> >>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >>> --- >>> Documentation/kbuild/kbuild.rst | 8 ++++++++ >>> Makefile | 9 ++++++--- >>> init/Kconfig | 8 ++++---- >>> usr/include/Makefile | 3 +++ >>> 4 files changed, 21 insertions(+), 7 deletions(-) >>> >>> diff --git a/Documentation/kbuild/kbuild.rst >>> b/Documentation/kbuild/kbuild.rst >>> index 2d1fc03d346e..16e90a3ae01b 100644 >>> --- a/Documentation/kbuild/kbuild.rst >>> +++ b/Documentation/kbuild/kbuild.rst >>> @@ -77,6 +77,14 @@ HOSTLDLIBS >>> ---------- >>> Additional libraries to link against when building host programs. >>> >>> +USERCFLAGS >>> +---------- >>> +Additional options used for $(CC) when compiling userprogs. >>> + >>> +USERLDFLAGS >>> +---------- >>> +Additional options used for $(LD) when linking userprogs. >> >> Probably should note the necessity of `-Wl,` prefixes here. >> >> Is `userprogs` cannonical? Yeah, I guess (reading >> Documentation/kbuild/makefiles.rst). I wonder if we should mention >> these in Documentation/kbuild/makefiles.rst as well? Under `5.3 >> Controlling compiler options for userspace programs`. >> >>> + >>> KBUILD_KCONFIG >>> -------------- >>> Set the top-level Kconfig file to the value of this environment >>> diff --git a/Makefile b/Makefile >>> index 45278d508d81..4a55537c8ca0 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -431,15 +431,17 @@ HOSTCC = gcc >>> HOSTCXX = g++ >>> endif >>> >>> -export KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes >>> -Wstrict-prototypes \ >>> - -O2 -fomit-frame-pointer -std=gnu89 >>> -export KBUILD_USERLDFLAGS := >>> +KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ >>> + -O2 -fomit-frame-pointer -std=gnu89 >>> +KBUILD_USERLDFLAGS := $(USERLDFLAGGS) >> >> ^ I think there's an extra G in USERLDFLAGS above. >> >>> >>> KBUILD_HOSTCFLAGS := $(KBUILD_USERCFLAGS) $(HOST_LFS_CFLAGS) >>> $(HOSTCFLAGS) >>> KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) >>> KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS) >>> KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS) >>> >>> +KBUILD_USERCFLAGS += $(USERCFLAGS) >> >> Since you added USERLDFLAGS to KBUILD_USERLDFLAGS above where it's >> defined, why not do so for USERCFLAGS/KBUILD_USERCFLAGS as well? >> >>> + >>> # Make variables (CC, etc...) >>> CPP = $(CC) -E >>> ifneq ($(LLVM),) >>> @@ -530,6 +532,7 @@ export CPP AR NM STRIP OBJCOPY OBJDUMP READELF >>> PAHOLE RESOLVE_BTFIDS LEX YACC AW >>> export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX >>> export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD >>> export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS >>> LDFLAGS_MODULE >>> +export KBUILD_USERCFLAGS KBUILD_USERLDFLAGS >>> >>> export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS >>> KBUILD_LDFLAGS >>> export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE >>> diff --git a/init/Kconfig b/init/Kconfig >>> index f2ae41e6717f..164706c38e8b 100644 >>> --- a/init/Kconfig >>> +++ b/init/Kconfig >>> @@ -62,13 +62,13 @@ config LLD_VERSION >>> >>> config CC_CAN_LINK >>> bool >>> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) >>> $(CLANG_FLAGS) $(m64-flag)) if 64BIT >>> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) >>> $(CLANG_FLAGS) $(m32-flag)) >>> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) >>> $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag)) if 64BIT >>> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) >>> $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag)) >>> >>> config CC_CAN_LINK_STATIC >>> bool >>> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) >>> $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT >>> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) >>> $(CLANG_FLAGS) $(m32-flag) -static) >>> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) >>> $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag) -static) if >>> 64BIT >>> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) >>> $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag) -static) >> >> since USERLDFLAGS get passed to $(CC), they will need `-Wl`, prefixes, >> hence the request for expanding the example usage in the commit >> message. >> >>> >>> config CC_HAS_ASM_GOTO >>> def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC)) >>> diff --git a/usr/include/Makefile b/usr/include/Makefile >>> index 1c2ae1368079..6a8c7dd9ccaf 100644 >>> --- a/usr/include/Makefile >>> +++ b/usr/include/Makefile >>> @@ -12,6 +12,9 @@ UAPI_CFLAGS := -std=c90 -Wall >>> -Werror=implicit-function-declaration >>> # It is here just because CONFIG_CC_CAN_LINK is tested with -m32 or >>> -m64. >>> UAPI_CFLAGS += $(filter -m32 -m64, $(KBUILD_CFLAGS)) >>> >>> +# USERCFLAGS might contain sysroot location for CC >>> +UAPI_CFLAGS += $(USERCFLAGS) >>> + >> >> Do we need to worry about USERLDFLAGS here, too? (or usr/Makefile?) >> >>> override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) >>> -I$(objtree)/usr/include >>> >>> # The following are excluded for now because they fail to build. >>> -- >>> 2.25.1 >>> >> >> >> -- >> Thanks, >> ~Nick Desaulniers
On 1/26/2022 10:21 AM, Nick Desaulniers wrote: > On Wed, Jan 12, 2022 at 2:44 PM Elliot Berman <quic_eberman@quicinc.com> wrote: >> >> Allow additional arguments be passed to userprogs compilation. >> Reproducible clang builds need to provide a sysroot and gcc path to >> ensure same toolchain is used across hosts. KCFLAGS is not currently >> used for any user programs compilation, so add new USERCFLAGS and >> USERLDFLAGS which serves similar purpose as HOSTCFLAGS/HOSTLDFLAGS. >> >> Specifically, I'm trying to force CC_CAN_LINK to consistently fail in >> an environment where a user sysroot is not specifically available. >> Currently, Clang might automatically detect GCC installation on hosts >> which have it installed to a default location in /. With addition of >> these environment variables, our build environment can do like >> "--sysroot=/dev/null" to force sysroot detection to fail. > > Hi Elliot, > Thanks for the patch! Sorry for the delay in reviewing; I didn't quite > get around to it then went on vacation for a week. Things get buried > in my inbox quickly; feel free to ping me if a week goes by with no > response on whichever channel works best for you. No worries :) Thanks for the review. > > I'm happy with the intent of this patch; GNU binutils has been removed > from Android, so supporting CC_CAN_LINK for Android kernel builds has > been a question I've been thinking about (though, not with higher > priority with some of our other issues), since we'll need to either > incorporate musl or bionic libc into our kernel build. I was thinking > of adding a SYSROOT command line variable for that, but I see your > approach is more flexible. > > One minor nit below, a typo, a few questions, and in the commit > message, but this generally LGTM. > > For the commit message, I think it would be good to expand `can do > like "--sysroot=/dev/null"` fully into > ``` > can specify > $ make USERCFLAGS=--sysroot=/dev/null USERLDFLAGS=-Wl,--sysroot=/dev/null > ``` > Will do. >> >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >> --- >> Documentation/kbuild/kbuild.rst | 8 ++++++++ >> Makefile | 9 ++++++--- >> init/Kconfig | 8 ++++---- >> usr/include/Makefile | 3 +++ >> 4 files changed, 21 insertions(+), 7 deletions(-) >> >> diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst >> index 2d1fc03d346e..16e90a3ae01b 100644 >> --- a/Documentation/kbuild/kbuild.rst >> +++ b/Documentation/kbuild/kbuild.rst >> @@ -77,6 +77,14 @@ HOSTLDLIBS >> ---------- >> Additional libraries to link against when building host programs. >> >> +USERCFLAGS >> +---------- >> +Additional options used for $(CC) when compiling userprogs. >> + >> +USERLDFLAGS >> +---------- >> +Additional options used for $(LD) when linking userprogs. > > Probably should note the necessity of `-Wl,` prefixes here. > > Is `userprogs` cannonical? Yeah, I guess (reading > Documentation/kbuild/makefiles.rst). I wonder if we should mention > these in Documentation/kbuild/makefiles.rst as well? Under `5.3 > Controlling compiler options for userspace programs`. > Will do. >> + >> KBUILD_KCONFIG >> -------------- >> Set the top-level Kconfig file to the value of this environment >> diff --git a/Makefile b/Makefile >> index 45278d508d81..4a55537c8ca0 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -431,15 +431,17 @@ HOSTCC = gcc >> HOSTCXX = g++ >> endif >> >> -export KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ >> - -O2 -fomit-frame-pointer -std=gnu89 >> -export KBUILD_USERLDFLAGS := >> +KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ >> + -O2 -fomit-frame-pointer -std=gnu89 >> +KBUILD_USERLDFLAGS := $(USERLDFLAGGS) > > ^ I think there's an extra G in USERLDFLAGS above. > Oops, thanks! >> >> KBUILD_HOSTCFLAGS := $(KBUILD_USERCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS) >> KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) >> KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS) >> KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS) >> >> +KBUILD_USERCFLAGS += $(USERCFLAGS) > > Since you added USERLDFLAGS to KBUILD_USERLDFLAGS above where it's > defined, why not do so for USERCFLAGS/KBUILD_USERCFLAGS as well? > The initial KBUILD_USERCFLAGS above is also used in KBUILD_HOSTCFLAGS. I didn't think it would be wise to have USERCFLAGS from command line affect HOSTCFLAGS as well, since a use case might be to add cross-compilation flags in USERCFLAGS but not in HOSTCFLAGS. When Android kernel builds do get arm64 musl/bionic libc available, we will want it only for userprogs and not hostprogs. >> + >> # Make variables (CC, etc...) >> CPP = $(CC) -E >> ifneq ($(LLVM),) >> @@ -530,6 +532,7 @@ export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AW >> export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX >> export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD >> export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE >> +export KBUILD_USERCFLAGS KBUILD_USERLDFLAGS >> >> export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS KBUILD_LDFLAGS >> export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE >> diff --git a/init/Kconfig b/init/Kconfig >> index f2ae41e6717f..164706c38e8b 100644 >> --- a/init/Kconfig >> +++ b/init/Kconfig >> @@ -62,13 +62,13 @@ config LLD_VERSION >> >> config CC_CAN_LINK >> bool >> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag)) if 64BIT >> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag)) >> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag)) if 64BIT >> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag)) >> >> config CC_CAN_LINK_STATIC >> bool >> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT >> - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag) -static) >> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag) -static) if 64BIT >> + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag) -static) > > since USERLDFLAGS get passed to $(CC), they will need `-Wl`, prefixes, > hence the request for expanding the example usage in the commit > message. > >> >> config CC_HAS_ASM_GOTO >> def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC)) >> diff --git a/usr/include/Makefile b/usr/include/Makefile >> index 1c2ae1368079..6a8c7dd9ccaf 100644 >> --- a/usr/include/Makefile >> +++ b/usr/include/Makefile >> @@ -12,6 +12,9 @@ UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration >> # It is here just because CONFIG_CC_CAN_LINK is tested with -m32 or -m64. >> UAPI_CFLAGS += $(filter -m32 -m64, $(KBUILD_CFLAGS)) >> >> +# USERCFLAGS might contain sysroot location for CC >> +UAPI_CFLAGS += $(USERCFLAGS) >> + > > Do we need to worry about USERLDFLAGS here, too? (or usr/Makefile?) > >> override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) -I$(objtree)/usr/include >> >> # The following are excluded for now because they fail to build. >> -- >> 2.25.1 >> > >
On 2022-01-28, Elliot Berman wrote: >Hi Fangrui, > >On 1/27/2022 7:15 PM, Fangrui Song wrote: >>On 2022-01-26, Nick Desaulniers wrote: >>>On Wed, Jan 12, 2022 at 2:44 PM Elliot Berman >>><quic_eberman@quicinc.com> wrote: >>>> >>>>Allow additional arguments be passed to userprogs compilation. >>>>Reproducible clang builds need to provide a sysroot and gcc path to >>>>ensure same toolchain is used across hosts. KCFLAGS is not currently >>>>used for any user programs compilation, so add new USERCFLAGS and >>>>USERLDFLAGS which serves similar purpose as HOSTCFLAGS/HOSTLDFLAGS. >>>> >>>>Specifically, I'm trying to force CC_CAN_LINK to consistently fail in >>>>an environment where a user sysroot is not specifically available. >>>>Currently, Clang might automatically detect GCC installation on hosts >>>>which have it installed to a default location in /. With addition of >>>>these environment variables, our build environment can do like >>>>"--sysroot=/dev/null" to force sysroot detection to fail. >>> >>>Hi Elliot, >>>Thanks for the patch! Sorry for the delay in reviewing; I didn't quite >>>get around to it then went on vacation for a week. Things get buried >>>in my inbox quickly; feel free to ping me if a week goes by with no >>>response on whichever channel works best for you. >>> >>>I'm happy with the intent of this patch; GNU binutils has been removed >>>from Android, so supporting CC_CAN_LINK for Android kernel builds has >>>been a question I've been thinking about (though, not with higher >>>priority with some of our other issues), since we'll need to either >>>incorporate musl or bionic libc into our kernel build. I was thinking >>>of adding a SYSROOT command line variable for that, but I see your >>>approach is more flexible. >>> >>>One minor nit below, a typo, a few questions, and in the commit >>>message, but this generally LGTM. >>> >>>For the commit message, I think it would be good to expand `can do >>>like "--sysroot=/dev/null"` fully into >>>``` >>>can specify >>>$ make USERCFLAGS=--sysroot=/dev/null USERLDFLAGS=-Wl,--sysroot=/dev/null >>>``` >> >>Is -Wl,--sysroot=/dev/null to override a -Wl,--sysroot specified >>previously on the command line? >> > >--sysroot isn't specified previously on the commandline for Android >kernel builds. Clang is falling back to sysroot detection based on >common installation paths, and we want to effectively disable that for >reproducible builds. > OK, the default sysroot in Clang is empty (can be overridden if one configures llvm-project with -DDEFAULT_SYSROOT=/some/path) Clang detects GCC installations in this order (https://maskray.me/blog/2021-03-28-compiler-driver-and-cross-compilation#clang) if (OPT_gcc_toolchain) prefixes = {OPT_gcc_toolchain}; else prefixes = {OPT_sysroot/usr, OPT_sysroot}; for prefix in prefixes if "$prefix/lib/gcc" exists ... If --gcc-toolchain is unspecified and --sysroot=/dev/null, there will be no selected GCC installation, and we will not get -L search paths like -L/usr/lib/x86_64-linux-gnu, -L/lib, etc. (-nostdlib does not suppress -L) So perhaps the change is to update Nick's suggestion $ make USERCFLAGS=--sysroot=/dev/null USERLDFLAGS=-Wl,--sysroot=/dev/null with $ make USERCFLAGS=--sysroot=/dev/null USERLDFLAGS=--sysroot=/dev/null We can compare the differences of clang --sysroot=/dev/null -xc /dev/null '-###' |& sed -E 's/ "?-[iIL]/\n&/g' and clang -Wl,--sysroot=/dev/null -xc /dev/null '-###' |& sed -E 's/ "?-[iIL]/\n&/g' >I believe this was introduced in Clang 13 [1], but it might also be a >change in AOSP Clang's prebuilt. I wasn't able to get clear picture of >what changed in Clang. Android12-5.10 builds which use Clang 12 are >unaffected. > >[1]: https://releases.llvm.org/13.0.0/tools/clang/docs/ReleaseNotes.html#modified-compiler-flags Yeah, I added this note in https://reviews.llvm.org/D97993 Looks like the note isn't exactly clear/correct. The exact semantics of -B (--prefix) are: * Search $prefix$file for executables, libraries, and data files. If $prefix is a directory, search $prefix/$file . These files include as (if -fno-integrated-as), ld, Scrt1.o, etc. Clang used to search $prefix/$triple-$file as well but I removed the rule in 2020. Hope that I did not deviate the topic too much. Just want to add some details which may be useful for your patch :) >>The driver option --sysroot does two things: >> >>* Decide include/library search paths (e.g. $sysroot/usr/include, >>$sysroot/lib64). >>* Pass --sysroot to ld. >> >>In ld, it means: if a linker script is in the sysroot directory, >>when ld opens an >>absolute path file (via INPUT or GROUP), add sysroot before the >>absolute path. >>In ld, --sysroot=/dev/null is not different --sysroot= (empty value). >>>>> >>>>Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >>>>--- >>>> Documentation/kbuild/kbuild.rst | 8 ++++++++ >>>> Makefile | 9 ++++++--- >>>> init/Kconfig | 8 ++++---- >>>> usr/include/Makefile | 3 +++ >>>> 4 files changed, 21 insertions(+), 7 deletions(-) >>>> >>>>diff --git a/Documentation/kbuild/kbuild.rst >>>>b/Documentation/kbuild/kbuild.rst >>>>index 2d1fc03d346e..16e90a3ae01b 100644 >>>>--- a/Documentation/kbuild/kbuild.rst >>>>+++ b/Documentation/kbuild/kbuild.rst >>>>@@ -77,6 +77,14 @@ HOSTLDLIBS >>>> ---------- >>>> Additional libraries to link against when building host programs. >>>> >>>>+USERCFLAGS >>>>+---------- >>>>+Additional options used for $(CC) when compiling userprogs. >>>>+ >>>>+USERLDFLAGS >>>>+---------- >>>>+Additional options used for $(LD) when linking userprogs. >>> >>>Probably should note the necessity of `-Wl,` prefixes here. >>> >>>Is `userprogs` cannonical? Yeah, I guess (reading >>>Documentation/kbuild/makefiles.rst). I wonder if we should mention >>>these in Documentation/kbuild/makefiles.rst as well? Under `5.3 >>>Controlling compiler options for userspace programs`. >>> >>>>+ >>>> KBUILD_KCONFIG >>>> -------------- >>>> Set the top-level Kconfig file to the value of this environment >>>>diff --git a/Makefile b/Makefile >>>>index 45278d508d81..4a55537c8ca0 100644 >>>>--- a/Makefile >>>>+++ b/Makefile >>>>@@ -431,15 +431,17 @@ HOSTCC = gcc >>>> HOSTCXX = g++ >>>> endif >>>> >>>>-export KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes >>>>-Wstrict-prototypes \ >>>>- -O2 -fomit-frame-pointer -std=gnu89 >>>>-export KBUILD_USERLDFLAGS := >>>>+KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ >>>>+ -O2 -fomit-frame-pointer -std=gnu89 >>>>+KBUILD_USERLDFLAGS := $(USERLDFLAGGS) >>> >>>^ I think there's an extra G in USERLDFLAGS above. >>> >>>> >>>> KBUILD_HOSTCFLAGS := $(KBUILD_USERCFLAGS) $(HOST_LFS_CFLAGS) >>>>$(HOSTCFLAGS) >>>> KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) >>>> KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS) >>>> KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS) >>>> >>>>+KBUILD_USERCFLAGS += $(USERCFLAGS) >>> >>>Since you added USERLDFLAGS to KBUILD_USERLDFLAGS above where it's >>>defined, why not do so for USERCFLAGS/KBUILD_USERCFLAGS as well? >>> >>>>+ >>>> # Make variables (CC, etc...) >>>> CPP = $(CC) -E >>>> ifneq ($(LLVM),) >>>>@@ -530,6 +532,7 @@ export CPP AR NM STRIP OBJCOPY OBJDUMP >>>>READELF PAHOLE RESOLVE_BTFIDS LEX YACC AW >>>> export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX >>>> export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD >>>> export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS >>>>LDFLAGS_MODULE >>>>+export KBUILD_USERCFLAGS KBUILD_USERLDFLAGS >>>> >>>> export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS >>>>KBUILD_LDFLAGS >>>> export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE >>>>diff --git a/init/Kconfig b/init/Kconfig >>>>index f2ae41e6717f..164706c38e8b 100644 >>>>--- a/init/Kconfig >>>>+++ b/init/Kconfig >>>>@@ -62,13 +62,13 @@ config LLD_VERSION >>>> >>>> config CC_CAN_LINK >>>> bool >>>>- default $(success,$(srctree)/scripts/cc-can-link.sh >>>>$(CC) $(CLANG_FLAGS) $(m64-flag)) if 64BIT >>>>- default $(success,$(srctree)/scripts/cc-can-link.sh >>>>$(CC) $(CLANG_FLAGS) $(m32-flag)) >>>>+ default $(success,$(srctree)/scripts/cc-can-link.sh >>>>$(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag)) >>>>if 64BIT >>>>+ default $(success,$(srctree)/scripts/cc-can-link.sh >>>>$(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag)) >>>> >>>> config CC_CAN_LINK_STATIC >>>> bool >>>>- default $(success,$(srctree)/scripts/cc-can-link.sh >>>>$(CC) $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT >>>>- default $(success,$(srctree)/scripts/cc-can-link.sh >>>>$(CC) $(CLANG_FLAGS) $(m32-flag) -static) >>>>+ default $(success,$(srctree)/scripts/cc-can-link.sh >>>>$(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag) >>>>-static) if 64BIT >>>>+ default $(success,$(srctree)/scripts/cc-can-link.sh >>>>$(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag) >>>>-static) >>> >>>since USERLDFLAGS get passed to $(CC), they will need `-Wl`, prefixes, >>>hence the request for expanding the example usage in the commit >>>message. >>> >>>> >>>> config CC_HAS_ASM_GOTO >>>> def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC)) >>>>diff --git a/usr/include/Makefile b/usr/include/Makefile >>>>index 1c2ae1368079..6a8c7dd9ccaf 100644 >>>>--- a/usr/include/Makefile >>>>+++ b/usr/include/Makefile >>>>@@ -12,6 +12,9 @@ UAPI_CFLAGS := -std=c90 -Wall >>>>-Werror=implicit-function-declaration >>>> # It is here just because CONFIG_CC_CAN_LINK is tested with >>>>-m32 or -m64. >>>> UAPI_CFLAGS += $(filter -m32 -m64, $(KBUILD_CFLAGS)) >>>> >>>>+# USERCFLAGS might contain sysroot location for CC >>>>+UAPI_CFLAGS += $(USERCFLAGS) >>>>+ >>> >>>Do we need to worry about USERLDFLAGS here, too? (or usr/Makefile?) >>> >>>> override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) >>>>-I$(objtree)/usr/include >>>> >>>> # The following are excluded for now because they fail to build. >>>>-- >>>>2.25.1 >>>> >>> >>> >>>-- >>>Thanks, >>>~Nick Desaulniers
diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst index 2d1fc03d346e..16e90a3ae01b 100644 --- a/Documentation/kbuild/kbuild.rst +++ b/Documentation/kbuild/kbuild.rst @@ -77,6 +77,14 @@ HOSTLDLIBS ---------- Additional libraries to link against when building host programs. +USERCFLAGS +---------- +Additional options used for $(CC) when compiling userprogs. + +USERLDFLAGS +---------- +Additional options used for $(LD) when linking userprogs. + KBUILD_KCONFIG -------------- Set the top-level Kconfig file to the value of this environment diff --git a/Makefile b/Makefile index 45278d508d81..4a55537c8ca0 100644 --- a/Makefile +++ b/Makefile @@ -431,15 +431,17 @@ HOSTCC = gcc HOSTCXX = g++ endif -export KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ - -O2 -fomit-frame-pointer -std=gnu89 -export KBUILD_USERLDFLAGS := +KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ + -O2 -fomit-frame-pointer -std=gnu89 +KBUILD_USERLDFLAGS := $(USERLDFLAGGS) KBUILD_HOSTCFLAGS := $(KBUILD_USERCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS) KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS) KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS) +KBUILD_USERCFLAGS += $(USERCFLAGS) + # Make variables (CC, etc...) CPP = $(CC) -E ifneq ($(LLVM),) @@ -530,6 +532,7 @@ export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AW export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE +export KBUILD_USERCFLAGS KBUILD_USERLDFLAGS export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS KBUILD_LDFLAGS export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE diff --git a/init/Kconfig b/init/Kconfig index f2ae41e6717f..164706c38e8b 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -62,13 +62,13 @@ config LLD_VERSION config CC_CAN_LINK bool - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag)) if 64BIT - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag)) + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag)) if 64BIT + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag)) config CC_CAN_LINK_STATIC bool - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag) -static) + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag) -static) if 64BIT + default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag) -static) config CC_HAS_ASM_GOTO def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC)) diff --git a/usr/include/Makefile b/usr/include/Makefile index 1c2ae1368079..6a8c7dd9ccaf 100644 --- a/usr/include/Makefile +++ b/usr/include/Makefile @@ -12,6 +12,9 @@ UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration # It is here just because CONFIG_CC_CAN_LINK is tested with -m32 or -m64. UAPI_CFLAGS += $(filter -m32 -m64, $(KBUILD_CFLAGS)) +# USERCFLAGS might contain sysroot location for CC +UAPI_CFLAGS += $(USERCFLAGS) + override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) -I$(objtree)/usr/include # The following are excluded for now because they fail to build.
Allow additional arguments be passed to userprogs compilation. Reproducible clang builds need to provide a sysroot and gcc path to ensure same toolchain is used across hosts. KCFLAGS is not currently used for any user programs compilation, so add new USERCFLAGS and USERLDFLAGS which serves similar purpose as HOSTCFLAGS/HOSTLDFLAGS. Specifically, I'm trying to force CC_CAN_LINK to consistently fail in an environment where a user sysroot is not specifically available. Currently, Clang might automatically detect GCC installation on hosts which have it installed to a default location in /. With addition of these environment variables, our build environment can do like "--sysroot=/dev/null" to force sysroot detection to fail. Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> --- Documentation/kbuild/kbuild.rst | 8 ++++++++ Makefile | 9 ++++++--- init/Kconfig | 8 ++++---- usr/include/Makefile | 3 +++ 4 files changed, 21 insertions(+), 7 deletions(-)