2023-04-09 15:02:44

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.

As a result, the linker script is preprocessed with predefined macros
for the build host instead of the target.

Assuming you use an x86 build machine, compare the following:

$ clang -dM -E -x c /dev/null
$ clang -dM -E -x c /dev/null -target aarch64-linux-gnu

There is no actual problem presumably because our linker scripts do not
rely on such predefined macros, but it is better to define correct ones.

Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
will be processed with the proper target triple.

Reported-by: Tom Rini <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---

scripts/Makefile.clang | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
index 70b354fa1cb4..93ca059cc3b8 100644
--- a/scripts/Makefile.clang
+++ b/scripts/Makefile.clang
@@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
CLANG_FLAGS += -Werror=ignored-optimization-argument
CLANG_FLAGS += -Werror=option-ignored
CLANG_FLAGS += -Werror=unused-command-line-argument
-KBUILD_CFLAGS += $(CLANG_FLAGS)
-KBUILD_AFLAGS += $(CLANG_FLAGS)
+KBUILD_CPPFLAGS += $(CLANG_FLAGS)
export CLANG_FLAGS
--
2.37.2


2023-04-09 20:53:18

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

Hi Masahiro,

kernel test robot noticed the following build warnings:

[auto build test WARNING on masahiroy-kbuild/for-next]
[also build test WARNING on masahiroy-kbuild/fixes linus/master v6.3-rc6 next-20230406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/kbuild-add-CLANG_CFLAGS-to-KBUILD_CPPFLAGS/20230409-225441
base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
patch link: https://lore.kernel.org/r/20230409145358.2538266-1-masahiroy%40kernel.org
patch subject: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS
config: mips-ip22_defconfig (https://download.01.org/0day-ci/archive/20230410/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 2c57868e2e877f73c339796c3374ae660bb77f0d)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/4a97e93c7f8c2aaf7bcca67a061264a1126d0e25
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Masahiro-Yamada/kbuild-add-CLANG_CFLAGS-to-KBUILD_CPPFLAGS/20230409-225441
git checkout 4a97e93c7f8c2aaf7bcca67a061264a1126d0e25
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips prepare

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> clang: warning: argument unused during compilation: '-mno-check-zero-division' [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: '-mabi=32' [-Wunused-command-line-argument]
>> clang: warning: argument unused during compilation: '-G 0' [-Wunused-command-line-argument]
>> clang: warning: argument unused during compilation: '-mno-abicalls' [-Wunused-command-line-argument]
error: unknown target CPU 'r5000'
note: valid target CPU values are: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, goldmont-plus, tremont, nehalem, corei7, westmere, sandybridge, corei7-avx, ivybridge, core-avx-i, haswell, core-avx2, broadwell, skylake, skylake-avx512, skx, cascadelake, cooperlake, cannonlake, icelake-client, rocketlake, icelake-server, tigerlake, sapphirerapids, alderlake, raptorlake, meteorlake, sierraforest, grandridge, graniterapids, emeraldrapids, knl, knm, k8, athlon64, athlon-fx, opteron, k8-sse3, athlon64-sse3, opteron-sse3, amdfam10, barcelona, btver1, btver2, bdver1, bdver2, bdver3, bdver4, znver1, znver2, znver3, znver4, x86-64, x86-64-v2, x86-64-v3, x86-64-v4
scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
error: unknown target CPU 'r5000'
note: valid target CPU values are: mips1, mips2, mips3, mips4, mips5, mips32, mips32r2, mips32r3, mips32r5, mips32r6, mips64, mips64r2, mips64r3, mips64r5, mips64r6, octeon, octeon+, p5600
make[2]: *** [scripts/Makefile.build:252: scripts/mod/empty.o] Error 1
error: unknown target CPU 'r5000'
note: valid target CPU values are: mips1, mips2, mips3, mips4, mips5, mips32, mips32r2, mips32r3, mips32r5, mips32r6, mips64, mips64r2, mips64r3, mips64r5, mips64r6, octeon, octeon+, p5600
make[2]: *** [scripts/Makefile.build:114: scripts/mod/devicetable-offsets.s] Error 1
make[2]: Target 'scripts/mod/' not remade because of errors.
make[1]: *** [Makefile:1285: prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:226: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-04-10 17:22:57

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

On Mon, Apr 10, 2023 at 04:09:37AM +0800, kernel test robot wrote:
> Hi Masahiro,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on masahiroy-kbuild/for-next]
> [also build test WARNING on masahiroy-kbuild/fixes linus/master v6.3-rc6 next-20230406]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/kbuild-add-CLANG_CFLAGS-to-KBUILD_CPPFLAGS/20230409-225441
> base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> patch link: https://lore.kernel.org/r/20230409145358.2538266-1-masahiroy%40kernel.org
> patch subject: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS
> config: mips-ip22_defconfig (https://download.01.org/0day-ci/archive/20230410/[email protected]/config)
> compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 2c57868e2e877f73c339796c3374ae660bb77f0d)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install mips cross compiling tool for clang build
> # apt-get install binutils-mips-linux-gnu
> # https://github.com/intel-lab-lkp/linux/commit/4a97e93c7f8c2aaf7bcca67a061264a1126d0e25
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Masahiro-Yamada/kbuild-add-CLANG_CFLAGS-to-KBUILD_CPPFLAGS/20230409-225441
> git checkout 4a97e93c7f8c2aaf7bcca67a061264a1126d0e25
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips prepare
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
> | Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
> >> clang: warning: argument unused during compilation: '-mno-check-zero-division' [-Wunused-command-line-argument]
> clang: warning: argument unused during compilation: '-mabi=32' [-Wunused-command-line-argument]
> >> clang: warning: argument unused during compilation: '-G 0' [-Wunused-command-line-argument]
> >> clang: warning: argument unused during compilation: '-mno-abicalls' [-Wunused-command-line-argument]
> error: unknown target CPU 'r5000'
> note: valid target CPU values are: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, goldmont-plus, tremont, nehalem, corei7, westmere, sandybridge, corei7-avx, ivybridge, core-avx-i, haswell, core-avx2, broadwell, skylake, skylake-avx512, skx, cascadelake, cooperlake, cannonlake, icelake-client, rocketlake, icelake-server, tigerlake, sapphirerapids, alderlake, raptorlake, meteorlake, sierraforest, grandridge, graniterapids, emeraldrapids, knl, knm, k8, athlon64, athlon-fx, opteron, k8-sse3, athlon64-sse3, opteron-sse3, amdfam10, barcelona, btver1, btver2, bdver1, bdver2, bdver3, bdver4, znver1, znver2, znver3, znver4, x86-64, x86-64-v2, x86-64-v3, x86-64-v4

While r5000 is not a valid MIPS CPU for LLVM (as you can see from the
values below), the note here seems to imply that there is a place where
KBUILD_{A,C}FLAGS is used without KBUILD_CPPFLAGS because we are
dropping '--target'. V=1 does not make it obvious where that is
happening though, as I see the error right after syncing the
configuration (is it coming from Kconfig internally?). I can see the
same issue with 32r2_defconfig, which uses an LLVM supported CPU value.

> scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
> scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
> scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
> error: unknown target CPU 'r5000'
> note: valid target CPU values are: mips1, mips2, mips3, mips4, mips5, mips32, mips32r2, mips32r3, mips32r5, mips32r6, mips64, mips64r2, mips64r3, mips64r5, mips64r6, octeon, octeon+, p5600
> make[2]: *** [scripts/Makefile.build:252: scripts/mod/empty.o] Error 1
> error: unknown target CPU 'r5000'
> note: valid target CPU values are: mips1, mips2, mips3, mips4, mips5, mips32, mips32r2, mips32r3, mips32r5, mips32r6, mips64, mips64r2, mips64r3, mips64r5, mips64r6, octeon, octeon+, p5600
> make[2]: *** [scripts/Makefile.build:114: scripts/mod/devicetable-offsets.s] Error 1
> make[2]: Target 'scripts/mod/' not remade because of errors.
> make[1]: *** [Makefile:1285: prepare0] Error 2
> make[1]: Target 'prepare' not remade because of errors.
> make: *** [Makefile:226: __sub-make] Error 2
> make: Target 'prepare' not remade because of errors.
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests

2023-04-10 17:32:13

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

Hi Masahiro,

Nit: s/CLANG_CFLAGS/CLANG_FLAGS/ in the title and a small typo below.

On Sun, Apr 09, 2023 at 11:53:57PM +0900, Masahiro Yamada wrote:
> When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
> not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.
>
> As a result, the linker script is preprocessed with predefined macros
> for the build host instead of the target.
>
> Assuming you use an x86 build machine, compare the following:
>
> $ clang -dM -E -x c /dev/null
> $ clang -dM -E -x c /dev/null -target aarch64-linux-gnu
>
> There is no actual problem presumably because our linker scripts do not
> rely on such predefined macros, but it is better to define correct ones.
>
> Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
^ CLANG_FLAGS
> will be processed with the proper target triple.
>
> Reported-by: Tom Rini <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>

We will need to sort out the report from the kbuild test robot before we
can do this. Otherwise, I think this should be fine, although I am
curious if -Wunused-command-line-argument might fire more because there
may be flags are unused while preprocessing. I will run this through my
test matrix once the other report has been addressed to make sure there
is nothing else obviously wrong with this change.

Cheers,
Nathan

> ---
>
> scripts/Makefile.clang | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> index 70b354fa1cb4..93ca059cc3b8 100644
> --- a/scripts/Makefile.clang
> +++ b/scripts/Makefile.clang
> @@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
> CLANG_FLAGS += -Werror=ignored-optimization-argument
> CLANG_FLAGS += -Werror=option-ignored
> CLANG_FLAGS += -Werror=unused-command-line-argument
> -KBUILD_CFLAGS += $(CLANG_FLAGS)
> -KBUILD_AFLAGS += $(CLANG_FLAGS)
> +KBUILD_CPPFLAGS += $(CLANG_FLAGS)
> export CLANG_FLAGS
> --
> 2.37.2
>

2023-05-31 21:38:41

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

Hi Masahiro,

On Sun, Apr 09, 2023 at 11:53:57PM +0900, Masahiro Yamada wrote:
> When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
> not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.
>
> As a result, the linker script is preprocessed with predefined macros
> for the build host instead of the target.
>
> Assuming you use an x86 build machine, compare the following:
>
> $ clang -dM -E -x c /dev/null
> $ clang -dM -E -x c /dev/null -target aarch64-linux-gnu
>
> There is no actual problem presumably because our linker scripts do not
> rely on such predefined macros, but it is better to define correct ones.
>
> Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
> will be processed with the proper target triple.
>
> Reported-by: Tom Rini <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> scripts/Makefile.clang | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> index 70b354fa1cb4..93ca059cc3b8 100644
> --- a/scripts/Makefile.clang
> +++ b/scripts/Makefile.clang
> @@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
> CLANG_FLAGS += -Werror=ignored-optimization-argument
> CLANG_FLAGS += -Werror=option-ignored
> CLANG_FLAGS += -Werror=unused-command-line-argument
> -KBUILD_CFLAGS += $(CLANG_FLAGS)
> -KBUILD_AFLAGS += $(CLANG_FLAGS)
> +KBUILD_CPPFLAGS += $(CLANG_FLAGS)
> export CLANG_FLAGS
> --
> 2.37.2
>

I am doubling back to this change, as the lack of '--target' in
KBUILD_CPPFLAGS is now an active bug with clang-17 due to a new change
that rejects '-mbig-endian' and '-mlittle-endian' when not supported by
the target, which breaks the arm64 vDSO build when preprocessing its
linker script:

# Turn on CONFIG_CPU_BIG_ENDIAN in menuconfig
$ make -skj"$(nproc)" ARCH=arm64 LLVM=1 O=build mrproper virtconfig menuconfig arch/arm64/kernel/vdso/
...
clang: error: unsupported option '-mbig-endian' for target 'x86_64-pc-linux-gnu'
make[3]: *** [.../scripts/Makefile.build:387: arch/arm64/kernel/vdso/vdso.lds] Error 1
...

https://github.com/llvm/llvm-project/commit/d81ce04587c006b6731198956c522c93d0df1050
https://github.com/ClangBuiltLinux/linux/issues/1859

This change resolves that issue. I was able to figure out why those new
warnings appeared for ARCH=mips, it is the shell invocation for
CHECKFLAGS. The following diff resolves it for me:

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index a7a4ee66a9d3..ef7b05ae92ce 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -346,7 +346,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
KBUILD_LDFLAGS += -m $(ld-emul)

ifdef CONFIG_MIPS
-CHECKFLAGS += $(shell $(CC) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
+CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')
endif

I will run this change plus that diff through my build matrix to see if
any other issues pop up. If not, I will respond with some tags and
perhaps this could be taken as a fix for 6.4 so that it could
potentially be backported?

Cheers,
Nathan

2023-06-02 15:36:47

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

On Wed, May 31, 2023 at 02:33:23PM -0700, Nathan Chancellor wrote:
> Hi Masahiro,
>
> On Sun, Apr 09, 2023 at 11:53:57PM +0900, Masahiro Yamada wrote:
> > When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
> > not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.
> >
> > As a result, the linker script is preprocessed with predefined macros
> > for the build host instead of the target.
> >
> > Assuming you use an x86 build machine, compare the following:
> >
> > $ clang -dM -E -x c /dev/null
> > $ clang -dM -E -x c /dev/null -target aarch64-linux-gnu
> >
> > There is no actual problem presumably because our linker scripts do not
> > rely on such predefined macros, but it is better to define correct ones.
> >
> > Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
> > will be processed with the proper target triple.
> >
> > Reported-by: Tom Rini <[email protected]>
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > scripts/Makefile.clang | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> > index 70b354fa1cb4..93ca059cc3b8 100644
> > --- a/scripts/Makefile.clang
> > +++ b/scripts/Makefile.clang
> > @@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
> > CLANG_FLAGS += -Werror=ignored-optimization-argument
> > CLANG_FLAGS += -Werror=option-ignored
> > CLANG_FLAGS += -Werror=unused-command-line-argument
> > -KBUILD_CFLAGS += $(CLANG_FLAGS)
> > -KBUILD_AFLAGS += $(CLANG_FLAGS)
> > +KBUILD_CPPFLAGS += $(CLANG_FLAGS)
> > export CLANG_FLAGS
> > --
> > 2.37.2
> >
>
> I am doubling back to this change, as the lack of '--target' in
> KBUILD_CPPFLAGS is now an active bug with clang-17 due to a new change
> that rejects '-mbig-endian' and '-mlittle-endian' when not supported by
> the target, which breaks the arm64 vDSO build when preprocessing its
> linker script:
>
> # Turn on CONFIG_CPU_BIG_ENDIAN in menuconfig
> $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 O=build mrproper virtconfig menuconfig arch/arm64/kernel/vdso/
> ...
> clang: error: unsupported option '-mbig-endian' for target 'x86_64-pc-linux-gnu'
> make[3]: *** [.../scripts/Makefile.build:387: arch/arm64/kernel/vdso/vdso.lds] Error 1
> ...
>
> https://github.com/llvm/llvm-project/commit/d81ce04587c006b6731198956c522c93d0df1050
> https://github.com/ClangBuiltLinux/linux/issues/1859
>
> This change resolves that issue. I was able to figure out why those new
> warnings appeared for ARCH=mips, it is the shell invocation for
> CHECKFLAGS. The following diff resolves it for me:
>
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index a7a4ee66a9d3..ef7b05ae92ce 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -346,7 +346,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> KBUILD_LDFLAGS += -m $(ld-emul)
>
> ifdef CONFIG_MIPS
> -CHECKFLAGS += $(shell $(CC) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> +CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
> sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')
> endif
>
> I will run this change plus that diff through my build matrix to see if
> any other issues pop up. If not, I will respond with some tags and
> perhaps this could be taken as a fix for 6.4 so that it could
> potentially be backported?

I found two more issues lurking in PowerPC. I have attached suggested
patches for all the issues I have uncovered to this email, please feel
free to use them or do something different if you feel there is a better
fix. With those issues resolved in one way or another, consider the
original change:

Reviewed-by: Nathan Chancellor <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>

If it would work better for you, I am more than happy to take over this
series as well.

Cheers,
Nathan


Attachments:
(No filename) (4.00 kB)
kbuild-move-clang_flags-to-kbuild_cppflags.mbox (4.79 kB)
Download all attachments

2023-06-03 16:40:58

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

On Thu, Jun 1, 2023 at 6:33 AM Nathan Chancellor <[email protected]> wrote:
>
> Hi Masahiro,
>
> On Sun, Apr 09, 2023 at 11:53:57PM +0900, Masahiro Yamada wrote:
> > When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
> > not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.
> >
> > As a result, the linker script is preprocessed with predefined macros
> > for the build host instead of the target.
> >
> > Assuming you use an x86 build machine, compare the following:
> >
> > $ clang -dM -E -x c /dev/null
> > $ clang -dM -E -x c /dev/null -target aarch64-linux-gnu
> >
> > There is no actual problem presumably because our linker scripts do not
> > rely on such predefined macros, but it is better to define correct ones.
> >
> > Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
> > will be processed with the proper target triple.
> >
> > Reported-by: Tom Rini <[email protected]>
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > scripts/Makefile.clang | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> > index 70b354fa1cb4..93ca059cc3b8 100644
> > --- a/scripts/Makefile.clang
> > +++ b/scripts/Makefile.clang
> > @@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
> > CLANG_FLAGS += -Werror=ignored-optimization-argument
> > CLANG_FLAGS += -Werror=option-ignored
> > CLANG_FLAGS += -Werror=unused-command-line-argument
> > -KBUILD_CFLAGS += $(CLANG_FLAGS)
> > -KBUILD_AFLAGS += $(CLANG_FLAGS)
> > +KBUILD_CPPFLAGS += $(CLANG_FLAGS)
> > export CLANG_FLAGS
> > --
> > 2.37.2
> >
>
> I am doubling back to this change, as the lack of '--target' in
> KBUILD_CPPFLAGS is now an active bug with clang-17 due to a new change
> that rejects '-mbig-endian' and '-mlittle-endian' when not supported by
> the target, which breaks the arm64 vDSO build when preprocessing its
> linker script:
>
> # Turn on CONFIG_CPU_BIG_ENDIAN in menuconfig
> $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 O=build mrproper virtconfig menuconfig arch/arm64/kernel/vdso/
> ...
> clang: error: unsupported option '-mbig-endian' for target 'x86_64-pc-linux-gnu'
> make[3]: *** [.../scripts/Makefile.build:387: arch/arm64/kernel/vdso/vdso.lds] Error 1
> ...
>
> https://github.com/llvm/llvm-project/commit/d81ce04587c006b6731198956c522c93d0df1050
> https://github.com/ClangBuiltLinux/linux/issues/1859
>
> This change resolves that issue.

Ah. Now we have a good reason to apply this patch.



> I was able to figure out why those new
> warnings appeared for ARCH=mips, it is the shell invocation for
> CHECKFLAGS. The following diff resolves it for me:
>
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index a7a4ee66a9d3..ef7b05ae92ce 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -346,7 +346,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> KBUILD_LDFLAGS += -m $(ld-emul)
>
> ifdef CONFIG_MIPS
> -CHECKFLAGS += $(shell $(CC) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> +CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
> sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')
> endif


Agree. This is the right fix because this $(shell ...) retrieves
the predefined macros of the compiler.
So, KBUILD_CPPFLAGS is correct.



> I will run this change plus that diff through my build matrix to see if
> any other issues pop up. If not, I will respond with some tags and
> perhaps this could be taken as a fix for 6.4 so that it could
> potentially be backported?
>
> Cheers,
> Nathan



--
Best Regards
Masahiro Yamada

2023-06-03 18:10:45

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

On Sat, Jun 3, 2023 at 12:25 AM Nathan Chancellor <[email protected]> wrote:
>
> On Wed, May 31, 2023 at 02:33:23PM -0700, Nathan Chancellor wrote:
> > Hi Masahiro,
> >
> > On Sun, Apr 09, 2023 at 11:53:57PM +0900, Masahiro Yamada wrote:
> > > When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
> > > not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.
> > >
> > > As a result, the linker script is preprocessed with predefined macros
> > > for the build host instead of the target.
> > >
> > > Assuming you use an x86 build machine, compare the following:
> > >
> > > $ clang -dM -E -x c /dev/null
> > > $ clang -dM -E -x c /dev/null -target aarch64-linux-gnu
> > >
> > > There is no actual problem presumably because our linker scripts do not
> > > rely on such predefined macros, but it is better to define correct ones.
> > >
> > > Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
> > > will be processed with the proper target triple.
> > >
> > > Reported-by: Tom Rini <[email protected]>
> > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > ---
> > >
> > > scripts/Makefile.clang | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> > > index 70b354fa1cb4..93ca059cc3b8 100644
> > > --- a/scripts/Makefile.clang
> > > +++ b/scripts/Makefile.clang
> > > @@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
> > > CLANG_FLAGS += -Werror=ignored-optimization-argument
> > > CLANG_FLAGS += -Werror=option-ignored
> > > CLANG_FLAGS += -Werror=unused-command-line-argument
> > > -KBUILD_CFLAGS += $(CLANG_FLAGS)
> > > -KBUILD_AFLAGS += $(CLANG_FLAGS)
> > > +KBUILD_CPPFLAGS += $(CLANG_FLAGS)
> > > export CLANG_FLAGS
> > > --
> > > 2.37.2
> > >
> >
> > I am doubling back to this change, as the lack of '--target' in
> > KBUILD_CPPFLAGS is now an active bug with clang-17 due to a new change
> > that rejects '-mbig-endian' and '-mlittle-endian' when not supported by
> > the target, which breaks the arm64 vDSO build when preprocessing its
> > linker script:
> >
> > # Turn on CONFIG_CPU_BIG_ENDIAN in menuconfig
> > $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 O=build mrproper virtconfig menuconfig arch/arm64/kernel/vdso/
> > ...
> > clang: error: unsupported option '-mbig-endian' for target 'x86_64-pc-linux-gnu'
> > make[3]: *** [.../scripts/Makefile.build:387: arch/arm64/kernel/vdso/vdso.lds] Error 1
> > ...
> >
> > https://github.com/llvm/llvm-project/commit/d81ce04587c006b6731198956c522c93d0df1050
> > https://github.com/ClangBuiltLinux/linux/issues/1859
> >
> > This change resolves that issue. I was able to figure out why those new
> > warnings appeared for ARCH=mips, it is the shell invocation for
> > CHECKFLAGS. The following diff resolves it for me:
> >
> > diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> > index a7a4ee66a9d3..ef7b05ae92ce 100644
> > --- a/arch/mips/Makefile
> > +++ b/arch/mips/Makefile
> > @@ -346,7 +346,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> > KBUILD_LDFLAGS += -m $(ld-emul)
> >
> > ifdef CONFIG_MIPS
> > -CHECKFLAGS += $(shell $(CC) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > +CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
> > sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')
> > endif
> >
> > I will run this change plus that diff through my build matrix to see if
> > any other issues pop up. If not, I will respond with some tags and
> > perhaps this could be taken as a fix for 6.4 so that it could
> > potentially be backported?
>
> I found two more issues lurking in PowerPC. I have attached suggested
> patches for all the issues I have uncovered to this email, please feel
> free to use them or do something different if you feel there is a better
> fix. With those issues resolved in one way or another, consider the
> original change:
>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Tested-by: Nathan Chancellor <[email protected]>
>
> If it would work better for you, I am more than happy to take over this
> series as well.
>
> Cheers,
> Nathan

Thanks. All the three patches look good to me.

I will apply them, then mine on top.




A nit:

The first patch for mips will introduce the warning
as $(KBUILD_CPPFLAGS) does not contain $(CLANG_FLAGS)
at this point.
Then, it will be fixed again when my patch is applied.

The two must be squashed together to avoid the regression,
but I think it is OK as-is.




The second patch for powerpc reminded me of
converting $(CC) to $(LD) for linking vDSO.
I do not remember why powerpc vDSO still uses $(CC).




--
Best Regards
Masahiro Yamada

2023-06-05 15:29:17

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

On Sun, Jun 04, 2023 at 01:32:21AM +0900, Masahiro Yamada wrote:
> On Sat, Jun 3, 2023 at 12:25 AM Nathan Chancellor <[email protected]> wrote:
> >
> > On Wed, May 31, 2023 at 02:33:23PM -0700, Nathan Chancellor wrote:
> > > Hi Masahiro,
> > >
> > > On Sun, Apr 09, 2023 at 11:53:57PM +0900, Masahiro Yamada wrote:
> > > > When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
> > > > not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.
> > > >
> > > > As a result, the linker script is preprocessed with predefined macros
> > > > for the build host instead of the target.
> > > >
> > > > Assuming you use an x86 build machine, compare the following:
> > > >
> > > > $ clang -dM -E -x c /dev/null
> > > > $ clang -dM -E -x c /dev/null -target aarch64-linux-gnu
> > > >
> > > > There is no actual problem presumably because our linker scripts do not
> > > > rely on such predefined macros, but it is better to define correct ones.
> > > >
> > > > Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
> > > > will be processed with the proper target triple.
> > > >
> > > > Reported-by: Tom Rini <[email protected]>
> > > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > > ---
> > > >
> > > > scripts/Makefile.clang | 3 +--
> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> > > > index 70b354fa1cb4..93ca059cc3b8 100644
> > > > --- a/scripts/Makefile.clang
> > > > +++ b/scripts/Makefile.clang
> > > > @@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
> > > > CLANG_FLAGS += -Werror=ignored-optimization-argument
> > > > CLANG_FLAGS += -Werror=option-ignored
> > > > CLANG_FLAGS += -Werror=unused-command-line-argument
> > > > -KBUILD_CFLAGS += $(CLANG_FLAGS)
> > > > -KBUILD_AFLAGS += $(CLANG_FLAGS)
> > > > +KBUILD_CPPFLAGS += $(CLANG_FLAGS)
> > > > export CLANG_FLAGS
> > > > --
> > > > 2.37.2
> > > >
> > >
> > > I am doubling back to this change, as the lack of '--target' in
> > > KBUILD_CPPFLAGS is now an active bug with clang-17 due to a new change
> > > that rejects '-mbig-endian' and '-mlittle-endian' when not supported by
> > > the target, which breaks the arm64 vDSO build when preprocessing its
> > > linker script:
> > >
> > > # Turn on CONFIG_CPU_BIG_ENDIAN in menuconfig
> > > $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 O=build mrproper virtconfig menuconfig arch/arm64/kernel/vdso/
> > > ...
> > > clang: error: unsupported option '-mbig-endian' for target 'x86_64-pc-linux-gnu'
> > > make[3]: *** [.../scripts/Makefile.build:387: arch/arm64/kernel/vdso/vdso.lds] Error 1
> > > ...
> > >
> > > https://github.com/llvm/llvm-project/commit/d81ce04587c006b6731198956c522c93d0df1050
> > > https://github.com/ClangBuiltLinux/linux/issues/1859
> > >
> > > This change resolves that issue. I was able to figure out why those new
> > > warnings appeared for ARCH=mips, it is the shell invocation for
> > > CHECKFLAGS. The following diff resolves it for me:
> > >
> > > diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> > > index a7a4ee66a9d3..ef7b05ae92ce 100644
> > > --- a/arch/mips/Makefile
> > > +++ b/arch/mips/Makefile
> > > @@ -346,7 +346,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> > > KBUILD_LDFLAGS += -m $(ld-emul)
> > >
> > > ifdef CONFIG_MIPS
> > > -CHECKFLAGS += $(shell $(CC) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > > +CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > > grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
> > > sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')
> > > endif
> > >
> > > I will run this change plus that diff through my build matrix to see if
> > > any other issues pop up. If not, I will respond with some tags and
> > > perhaps this could be taken as a fix for 6.4 so that it could
> > > potentially be backported?
> >
> > I found two more issues lurking in PowerPC. I have attached suggested
> > patches for all the issues I have uncovered to this email, please feel
> > free to use them or do something different if you feel there is a better
> > fix. With those issues resolved in one way or another, consider the
> > original change:
> >
> > Reviewed-by: Nathan Chancellor <[email protected]>
> > Tested-by: Nathan Chancellor <[email protected]>
> >
> > If it would work better for you, I am more than happy to take over this
> > series as well.
> >
> > Cheers,
> > Nathan
>
> Thanks. All the three patches look good to me.
>
> I will apply them, then mine on top.

Thanks, sounds like a good plan!

> A nit:
>
> The first patch for mips will introduce the warning
> as $(KBUILD_CPPFLAGS) does not contain $(CLANG_FLAGS)
> at this point.
> Then, it will be fixed again when my patch is applied.
>
> The two must be squashed together to avoid the regression,
> but I think it is OK as-is.

Hmmm, what warning would be introduced? KBUILD_CPPFLAGS is being added
to the CHECKFLAGS invocation, rather than replacing KBUILD_CFLAGS, so
'--target' should always be present (first via KBUILD_CFLAGS then via
KBUILD_CPPFLAGS), right?

> The second patch for powerpc reminded me of
> converting $(CC) to $(LD) for linking vDSO.
> I do not remember why powerpc vDSO still uses $(CC).

Nick tried to make the conversion some time ago but ran into issues with
GNU ld crashing:

https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/

It might be easier to do this now that the PowerPC vDSO has been
combined into one directory, but we have not had time to sit down and
actually work on it.

Cheers,
Nathan

2023-06-06 01:37:46

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

On Tue, Jun 6, 2023 at 12:21 AM Nathan Chancellor <[email protected]> wrote:
>
> On Sun, Jun 04, 2023 at 01:32:21AM +0900, Masahiro Yamada wrote:
> > On Sat, Jun 3, 2023 at 12:25 AM Nathan Chancellor <[email protected]> wrote:
> > >
> > > On Wed, May 31, 2023 at 02:33:23PM -0700, Nathan Chancellor wrote:
> > > > Hi Masahiro,
> > > >
> > > > On Sun, Apr 09, 2023 at 11:53:57PM +0900, Masahiro Yamada wrote:
> > > > > When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
> > > > > not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.
> > > > >
> > > > > As a result, the linker script is preprocessed with predefined macros
> > > > > for the build host instead of the target.
> > > > >
> > > > > Assuming you use an x86 build machine, compare the following:
> > > > >
> > > > > $ clang -dM -E -x c /dev/null
> > > > > $ clang -dM -E -x c /dev/null -target aarch64-linux-gnu
> > > > >
> > > > > There is no actual problem presumably because our linker scripts do not
> > > > > rely on such predefined macros, but it is better to define correct ones.
> > > > >
> > > > > Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
> > > > > will be processed with the proper target triple.
> > > > >
> > > > > Reported-by: Tom Rini <[email protected]>
> > > > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > > > ---
> > > > >
> > > > > scripts/Makefile.clang | 3 +--
> > > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> > > > > index 70b354fa1cb4..93ca059cc3b8 100644
> > > > > --- a/scripts/Makefile.clang
> > > > > +++ b/scripts/Makefile.clang
> > > > > @@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
> > > > > CLANG_FLAGS += -Werror=ignored-optimization-argument
> > > > > CLANG_FLAGS += -Werror=option-ignored
> > > > > CLANG_FLAGS += -Werror=unused-command-line-argument
> > > > > -KBUILD_CFLAGS += $(CLANG_FLAGS)
> > > > > -KBUILD_AFLAGS += $(CLANG_FLAGS)
> > > > > +KBUILD_CPPFLAGS += $(CLANG_FLAGS)
> > > > > export CLANG_FLAGS
> > > > > --
> > > > > 2.37.2
> > > > >
> > > >
> > > > I am doubling back to this change, as the lack of '--target' in
> > > > KBUILD_CPPFLAGS is now an active bug with clang-17 due to a new change
> > > > that rejects '-mbig-endian' and '-mlittle-endian' when not supported by
> > > > the target, which breaks the arm64 vDSO build when preprocessing its
> > > > linker script:
> > > >
> > > > # Turn on CONFIG_CPU_BIG_ENDIAN in menuconfig
> > > > $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 O=build mrproper virtconfig menuconfig arch/arm64/kernel/vdso/
> > > > ...
> > > > clang: error: unsupported option '-mbig-endian' for target 'x86_64-pc-linux-gnu'
> > > > make[3]: *** [.../scripts/Makefile.build:387: arch/arm64/kernel/vdso/vdso.lds] Error 1
> > > > ...
> > > >
> > > > https://github.com/llvm/llvm-project/commit/d81ce04587c006b6731198956c522c93d0df1050
> > > > https://github.com/ClangBuiltLinux/linux/issues/1859
> > > >
> > > > This change resolves that issue. I was able to figure out why those new
> > > > warnings appeared for ARCH=mips, it is the shell invocation for
> > > > CHECKFLAGS. The following diff resolves it for me:
> > > >
> > > > diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> > > > index a7a4ee66a9d3..ef7b05ae92ce 100644
> > > > --- a/arch/mips/Makefile
> > > > +++ b/arch/mips/Makefile
> > > > @@ -346,7 +346,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> > > > KBUILD_LDFLAGS += -m $(ld-emul)
> > > >
> > > > ifdef CONFIG_MIPS
> > > > -CHECKFLAGS += $(shell $(CC) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > > > +CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > > > grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
> > > > sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')
> > > > endif
> > > >
> > > > I will run this change plus that diff through my build matrix to see if
> > > > any other issues pop up. If not, I will respond with some tags and
> > > > perhaps this could be taken as a fix for 6.4 so that it could
> > > > potentially be backported?
> > >
> > > I found two more issues lurking in PowerPC. I have attached suggested
> > > patches for all the issues I have uncovered to this email, please feel
> > > free to use them or do something different if you feel there is a better
> > > fix. With those issues resolved in one way or another, consider the
> > > original change:
> > >
> > > Reviewed-by: Nathan Chancellor <[email protected]>
> > > Tested-by: Nathan Chancellor <[email protected]>
> > >
> > > If it would work better for you, I am more than happy to take over this
> > > series as well.
> > >
> > > Cheers,
> > > Nathan
> >
> > Thanks. All the three patches look good to me.
> >
> > I will apply them, then mine on top.
>
> Thanks, sounds like a good plan!
>
> > A nit:
> >
> > The first patch for mips will introduce the warning
> > as $(KBUILD_CPPFLAGS) does not contain $(CLANG_FLAGS)
> > at this point.
> > Then, it will be fixed again when my patch is applied.
> >
> > The two must be squashed together to avoid the regression,
> > but I think it is OK as-is.
>
> Hmmm, what warning would be introduced? KBUILD_CPPFLAGS is being added
> to the CHECKFLAGS invocation, rather than replacing KBUILD_CFLAGS, so
> '--target' should always be present (first via KBUILD_CFLAGS then via
> KBUILD_CPPFLAGS), right?



Sorry, I wrongly read your code.

You are right, you added $(KBUILD_CPPFLAGS)
in addition to $(KBUILD_CFLAGS).




Maybe, we can drop $(KBUILD_CFLAGS) (as a follow-up)
because this $(shell ) invocation is only interested
in preprocessor macros.


diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index ef7b05ae92ce..e563cca6b663 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -346,7 +346,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
KBUILD_LDFLAGS += -m $(ld-emul)

ifdef CONFIG_MIPS
-CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM
-E -x c /dev/null | \
+CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) -dM -E -x c /dev/null | \
grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')
endif









>
> > The second patch for powerpc reminded me of
> > converting $(CC) to $(LD) for linking vDSO.
> > I do not remember why powerpc vDSO still uses $(CC).
>
> Nick tried to make the conversion some time ago but ran into issues with
> GNU ld crashing:
>
> https://lore.kernel.org/lkml/[email protected]/
> https://lore.kernel.org/lkml/[email protected]/
>
> It might be easier to do this now that the PowerPC vDSO has been
> combined into one directory, but we have not had time to sit down and
> actually work on it.
>
> Cheers,
> Nathan



--
Best Regards
Masahiro Yamada

2023-06-22 17:24:29

by Fangrui Song

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

On Sat, Jun 3, 2023 at 9:33 AM Masahiro Yamada <[email protected]> wrote:
>
> On Sat, Jun 3, 2023 at 12:25 AM Nathan Chancellor <[email protected]> wrote:
> >
> > On Wed, May 31, 2023 at 02:33:23PM -0700, Nathan Chancellor wrote:
> > > Hi Masahiro,
> > >
> > > On Sun, Apr 09, 2023 at 11:53:57PM +0900, Masahiro Yamada wrote:
> > > > When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
> > > > not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.
> > > >
> > > > As a result, the linker script is preprocessed with predefined macros
> > > > for the build host instead of the target.
> > > >
> > > > Assuming you use an x86 build machine, compare the following:
> > > >
> > > > $ clang -dM -E -x c /dev/null
> > > > $ clang -dM -E -x c /dev/null -target aarch64-linux-gnu
> > > >
> > > > There is no actual problem presumably because our linker scripts do not
> > > > rely on such predefined macros, but it is better to define correct ones.
> > > >
> > > > Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
> > > > will be processed with the proper target triple.
> > > >
> > > > Reported-by: Tom Rini <[email protected]>
> > > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > > ---
> > > >
> > > > scripts/Makefile.clang | 3 +--
> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> > > > index 70b354fa1cb4..93ca059cc3b8 100644
> > > > --- a/scripts/Makefile.clang
> > > > +++ b/scripts/Makefile.clang
> > > > @@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
> > > > CLANG_FLAGS += -Werror=ignored-optimization-argument
> > > > CLANG_FLAGS += -Werror=option-ignored
> > > > CLANG_FLAGS += -Werror=unused-command-line-argument
> > > > -KBUILD_CFLAGS += $(CLANG_FLAGS)
> > > > -KBUILD_AFLAGS += $(CLANG_FLAGS)
> > > > +KBUILD_CPPFLAGS += $(CLANG_FLAGS)
> > > > export CLANG_FLAGS
> > > > --
> > > > 2.37.2
> > > >
> > >
> > > I am doubling back to this change, as the lack of '--target' in
> > > KBUILD_CPPFLAGS is now an active bug with clang-17 due to a new change
> > > that rejects '-mbig-endian' and '-mlittle-endian' when not supported by
> > > the target, which breaks the arm64 vDSO build when preprocessing its
> > > linker script:
> > >
> > > # Turn on CONFIG_CPU_BIG_ENDIAN in menuconfig
> > > $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 O=build mrproper virtconfig menuconfig arch/arm64/kernel/vdso/
> > > ...
> > > clang: error: unsupported option '-mbig-endian' for target 'x86_64-pc-linux-gnu'
> > > make[3]: *** [.../scripts/Makefile.build:387: arch/arm64/kernel/vdso/vdso.lds] Error 1
> > > ...
> > >
> > > https://github.com/llvm/llvm-project/commit/d81ce04587c006b6731198956c522c93d0df1050
> > > https://github.com/ClangBuiltLinux/linux/issues/1859
> > >
> > > This change resolves that issue. I was able to figure out why those new
> > > warnings appeared for ARCH=mips, it is the shell invocation for
> > > CHECKFLAGS. The following diff resolves it for me:
> > >
> > > diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> > > index a7a4ee66a9d3..ef7b05ae92ce 100644
> > > --- a/arch/mips/Makefile
> > > +++ b/arch/mips/Makefile
> > > @@ -346,7 +346,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> > > KBUILD_LDFLAGS += -m $(ld-emul)
> > >
> > > ifdef CONFIG_MIPS
> > > -CHECKFLAGS += $(shell $(CC) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > > +CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > > grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
> > > sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')
> > > endif
> > >
> > > I will run this change plus that diff through my build matrix to see if
> > > any other issues pop up. If not, I will respond with some tags and
> > > perhaps this could be taken as a fix for 6.4 so that it could
> > > potentially be backported?
> >
> > I found two more issues lurking in PowerPC. I have attached suggested
> > patches for all the issues I have uncovered to this email, please feel
> > free to use them or do something different if you feel there is a better
> > fix. With those issues resolved in one way or another, consider the
> > original change:
> >
> > Reviewed-by: Nathan Chancellor <[email protected]>
> > Tested-by: Nathan Chancellor <[email protected]>
> >
> > If it would work better for you, I am more than happy to take over this
> > series as well.
> >
> > Cheers,
> > Nathan
>
> Thanks. All the three patches look good to me.
>
> I will apply them, then mine on top.

Drive-by reply from
https://github.com/ClangBuiltLinux/continuous-integration2/pull/585

Hi Masahiro, one nit:) If the patch still has time to adjust the
commit message, perhaps consider replacing

-target aarch64-linux-gnu with --target=aarch64-linux-gnu

as the former is a deprecated driver option since Clang 3.x (long time ago).

> Assuming you use an x86 build machine, compare the following:

You may drop "Assuming you use an x86 build machine" and add
--target=x86_64-linux-gnu to the `clang -dM -E -x c /dev/null` command
:)


--
宋方睿

2023-06-23 06:13:44

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: add $(CLANG_CFLAGS) to KBUILD_CPPFLAGS

On Fri, Jun 23, 2023 at 2:07 AM Fangrui Song <[email protected]> wrote:
>
> On Sat, Jun 3, 2023 at 9:33 AM Masahiro Yamada <[email protected]> wrote:
> >
> > On Sat, Jun 3, 2023 at 12:25 AM Nathan Chancellor <[email protected]> wrote:
> > >
> > > On Wed, May 31, 2023 at 02:33:23PM -0700, Nathan Chancellor wrote:
> > > > Hi Masahiro,
> > > >
> > > > On Sun, Apr 09, 2023 at 11:53:57PM +0900, Masahiro Yamada wrote:
> > > > > When preprocessing arch/*/kernel/vmlinux.lds.S, the target triple is
> > > > > not passed to $(CPP) because we add it only to KBUILD_{C,A}FLAGS.
> > > > >
> > > > > As a result, the linker script is preprocessed with predefined macros
> > > > > for the build host instead of the target.
> > > > >
> > > > > Assuming you use an x86 build machine, compare the following:
> > > > >
> > > > > $ clang -dM -E -x c /dev/null
> > > > > $ clang -dM -E -x c /dev/null -target aarch64-linux-gnu
> > > > >
> > > > > There is no actual problem presumably because our linker scripts do not
> > > > > rely on such predefined macros, but it is better to define correct ones.
> > > > >
> > > > > Move $(CFLAGS_CFLAGS) to KBUILD_CPPFLAGS, so that all *.c, *.S, *.lds.S
> > > > > will be processed with the proper target triple.
> > > > >
> > > > > Reported-by: Tom Rini <[email protected]>
> > > > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > > > ---
> > > > >
> > > > > scripts/Makefile.clang | 3 +--
> > > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> > > > > index 70b354fa1cb4..93ca059cc3b8 100644
> > > > > --- a/scripts/Makefile.clang
> > > > > +++ b/scripts/Makefile.clang
> > > > > @@ -38,6 +38,5 @@ CLANG_FLAGS += -Werror=unknown-warning-option
> > > > > CLANG_FLAGS += -Werror=ignored-optimization-argument
> > > > > CLANG_FLAGS += -Werror=option-ignored
> > > > > CLANG_FLAGS += -Werror=unused-command-line-argument
> > > > > -KBUILD_CFLAGS += $(CLANG_FLAGS)
> > > > > -KBUILD_AFLAGS += $(CLANG_FLAGS)
> > > > > +KBUILD_CPPFLAGS += $(CLANG_FLAGS)
> > > > > export CLANG_FLAGS
> > > > > --
> > > > > 2.37.2
> > > > >
> > > >
> > > > I am doubling back to this change, as the lack of '--target' in
> > > > KBUILD_CPPFLAGS is now an active bug with clang-17 due to a new change
> > > > that rejects '-mbig-endian' and '-mlittle-endian' when not supported by
> > > > the target, which breaks the arm64 vDSO build when preprocessing its
> > > > linker script:
> > > >
> > > > # Turn on CONFIG_CPU_BIG_ENDIAN in menuconfig
> > > > $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 O=build mrproper virtconfig menuconfig arch/arm64/kernel/vdso/
> > > > ...
> > > > clang: error: unsupported option '-mbig-endian' for target 'x86_64-pc-linux-gnu'
> > > > make[3]: *** [.../scripts/Makefile.build:387: arch/arm64/kernel/vdso/vdso.lds] Error 1
> > > > ...
> > > >
> > > > https://github.com/llvm/llvm-project/commit/d81ce04587c006b6731198956c522c93d0df1050
> > > > https://github.com/ClangBuiltLinux/linux/issues/1859
> > > >
> > > > This change resolves that issue. I was able to figure out why those new
> > > > warnings appeared for ARCH=mips, it is the shell invocation for
> > > > CHECKFLAGS. The following diff resolves it for me:
> > > >
> > > > diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> > > > index a7a4ee66a9d3..ef7b05ae92ce 100644
> > > > --- a/arch/mips/Makefile
> > > > +++ b/arch/mips/Makefile
> > > > @@ -346,7 +346,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> > > > KBUILD_LDFLAGS += -m $(ld-emul)
> > > >
> > > > ifdef CONFIG_MIPS
> > > > -CHECKFLAGS += $(shell $(CC) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > > > +CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
> > > > grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
> > > > sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')
> > > > endif
> > > >
> > > > I will run this change plus that diff through my build matrix to see if
> > > > any other issues pop up. If not, I will respond with some tags and
> > > > perhaps this could be taken as a fix for 6.4 so that it could
> > > > potentially be backported?
> > >
> > > I found two more issues lurking in PowerPC. I have attached suggested
> > > patches for all the issues I have uncovered to this email, please feel
> > > free to use them or do something different if you feel there is a better
> > > fix. With those issues resolved in one way or another, consider the
> > > original change:
> > >
> > > Reviewed-by: Nathan Chancellor <[email protected]>
> > > Tested-by: Nathan Chancellor <[email protected]>
> > >
> > > If it would work better for you, I am more than happy to take over this
> > > series as well.
> > >
> > > Cheers,
> > > Nathan
> >
> > Thanks. All the three patches look good to me.
> >
> > I will apply them, then mine on top.
>
> Drive-by reply from
> https://github.com/ClangBuiltLinux/continuous-integration2/pull/585
>
> Hi Masahiro, one nit:) If the patch still has time to adjust the
> commit message, perhaps consider replacing
>
> -target aarch64-linux-gnu with --target=aarch64-linux-gnu
>
> as the former is a deprecated driver option since Clang 3.x (long time ago).
>
> > Assuming you use an x86 build machine, compare the following:
>
> You may drop "Assuming you use an x86 build machine" and add
> --target=x86_64-linux-gnu to the `clang -dM -E -x c /dev/null` command
> :)

Thanks for the suggestion, but I do not want to change the commit ID
unless there is something terribly wrong.

Nathan recorded the commit ID in the CBL bug tracker.



--
Best Regards
Masahiro Yamada