2023-01-08 10:05:15

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2] kbuild: do not export LDFLAGS_vmlinux to decompressor Makefiles

Nathan Chancellor reports an error message from $(NM) if GNU Make 4.4
is used to build the ARM decompressor.

$ make-4.4 ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- O=build defconfig zImage
[snip]
LD vmlinux
NM System.map
SORTTAB vmlinux
OBJCOPY arch/arm/boot/Image
Kernel: arch/arm/boot/Image is ready
arm-linux-gnueabi-nm: 'arch/arm/boot/compressed/../../../../vmlinux': No such file
/bin/sh: 1: arithmetic expression: expecting primary: " "
LDS arch/arm/boot/compressed/vmlinux.lds
AS arch/arm/boot/compressed/head.o
GZIP arch/arm/boot/compressed/piggy_data
AS arch/arm/boot/compressed/piggy.o
CC arch/arm/boot/compressed/misc.o

This occurs since GNU Make commit 98da874c4303 ("[SV 10593] Export
variables to $(shell ...) commands"), and the O= option is needed to
reproduce it. The generated zImage is correct despite the error message.

As the commit description of 98da874c4303 [1] says, exported variables
are passed down to $(shell ) functions, which means exported recursive
variables might be expanded earlier than before, in the parse stage.

The following test code demonstrates the change for GNU Make 4.4.

[Test Makefile]

$(shell echo hello > foo)
export foo = $(shell cat bar/../foo)
$(shell mkdir bar)

all:
@echo $(foo)

[GNU Make 4.3]

$ rm -rf bar; make-4.3
hello

[GNU Make 4.4]

$ rm -rf bar; make-4.4
cat: bar/../foo: No such file or directory
hello

The 'foo' is a resursively expanded (i.e. lazily expanded) variable.

GNU Make 4.3 expands 'foo' just before running the recipe '@echo $(foo)',
at this point, the directory 'bar' exists.

GNU Make 4.4 expands 'foo' to evaluate $(shell mkdir bar) because it is
exported. At this point, the directory 'bar' does not exit yet. The cat
command cannot resolve the bar/../foo path, hence the error message.

Let's get back to the kernel Makefile.

In arch/arm/boot/compressed/Makefile, KBSS_SZ is referenced by
LDFLAGS_vmlinux, which is recursive and also exported by the top
Makefile.

GNU Make 4.3 expands KBSS_SZ just before running the recipes, so it
works fine.

GNU Make 4.4 expands KBSS_SZ in the parse stage, where the directory
arm/arm/boot/compressed does not exit yet. When compiled O= option,
the output directory is created by $(shell mkdir -p $(obj-dirs))
in scripts/Makefile.build.

There are two ways to fix this particular issue:

- change "$(obj)/../../../../vmlinux" in KBSS_SZ to "vmlinux"
- unexport LDFLAGS_vmlinux

This commit takes the latter course because it is what I originally
intended.

Commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
unexported LDFLAGS_vmlinux.

Commit 5d4aeffbf709 ("kbuild: rebuild .vmlinux.export.o when its
prerequisite is updated") accidentally exported it again.

We can clean up arch/arm/boot/compressed/Makefile later.

[1]: https://git.savannah.gnu.org/cgit/make.git/commit/?id=98da874c43035a490cdca81331724f233a3d0c9a

Link: https://lore.kernel.org/all/[email protected]/
Fixes: 5d4aeffbf709 ("kbuild: rebuild .vmlinux.export.o when its prerequisite is updated")
Reported-by: Nathan Chancellor <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v2:
- Postpone the change for arch/arm/boot/compressed/Makefile
- Update the commit description

Makefile | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index dfba294ae790..658fc0e602c9 100644
--- a/Makefile
+++ b/Makefile
@@ -549,7 +549,7 @@ LDFLAGS_MODULE =
CFLAGS_KERNEL =
RUSTFLAGS_KERNEL =
AFLAGS_KERNEL =
-export LDFLAGS_vmlinux =
+LDFLAGS_vmlinux =

# Use USERINCLUDE when you must reference the UAPI directories only.
USERINCLUDE := \
@@ -1248,6 +1248,11 @@ vmlinux.o modules.builtin.modinfo modules.builtin: vmlinux_o
@:

PHONY += vmlinux
+# LDFLAGS_vmlinux flags from the top Makefile and arch/*/Makefile should be
+# exported for building the top vmlinux, but not for building decompressors.
+# They happen to have the same base name (arch/*/boot/compressed/vmlinux).
+# The decompressor Makefiles should not inherit LDFLAGS_vmlinux.
+vmlinux: export LDFLAGS_vmlinux := $(LDFLAGS_vmlinux)
vmlinux: vmlinux.o $(KBUILD_LDS) modpost
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vmlinux

--
2.34.1


2023-01-08 13:50:54

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCH v2] kbuild: do not export LDFLAGS_vmlinux to decompressor Makefiles

On Sun, Jan 08, 2023 at 06:53:42PM +0900 Masahiro Yamada wrote:
> Date: Sun, 8 Jan 2023 18:53:42 +0900
> From: Masahiro Yamada <[email protected]>
> To: [email protected]
> Cc: [email protected], Nathan Chancellor <[email protected]>,
> Masahiro Yamada <[email protected]>, Nick Desaulniers
> <[email protected]>, Nicolas Schier <[email protected]>
> Subject: [PATCH v2] kbuild: do not export LDFLAGS_vmlinux to decompressor
> Makefiles
> Message-Id: <[email protected]>
> X-Mailer: git-send-email 2.34.1
>
> Nathan Chancellor reports an error message from $(NM) if GNU Make 4.4
> is used to build the ARM decompressor.
>
> $ make-4.4 ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- O=build defconfig zImage
> [snip]
> LD vmlinux
> NM System.map
> SORTTAB vmlinux
> OBJCOPY arch/arm/boot/Image
> Kernel: arch/arm/boot/Image is ready
> arm-linux-gnueabi-nm: 'arch/arm/boot/compressed/../../../../vmlinux': No such file
> /bin/sh: 1: arithmetic expression: expecting primary: " "
> LDS arch/arm/boot/compressed/vmlinux.lds
> AS arch/arm/boot/compressed/head.o
> GZIP arch/arm/boot/compressed/piggy_data
> AS arch/arm/boot/compressed/piggy.o
> CC arch/arm/boot/compressed/misc.o
>
> This occurs since GNU Make commit 98da874c4303 ("[SV 10593] Export
> variables to $(shell ...) commands"), and the O= option is needed to
> reproduce it. The generated zImage is correct despite the error message.
>
> As the commit description of 98da874c4303 [1] says, exported variables
> are passed down to $(shell ) functions, which means exported recursive
> variables might be expanded earlier than before, in the parse stage.
>
> The following test code demonstrates the change for GNU Make 4.4.
>
> [Test Makefile]
>
> $(shell echo hello > foo)
> export foo = $(shell cat bar/../foo)
> $(shell mkdir bar)
>
> all:
> @echo $(foo)
>
> [GNU Make 4.3]
>
> $ rm -rf bar; make-4.3
> hello
>
> [GNU Make 4.4]
>
> $ rm -rf bar; make-4.4
> cat: bar/../foo: No such file or directory
> hello
>
> The 'foo' is a resursively expanded (i.e. lazily expanded) variable.
>
> GNU Make 4.3 expands 'foo' just before running the recipe '@echo $(foo)',
> at this point, the directory 'bar' exists.
>
> GNU Make 4.4 expands 'foo' to evaluate $(shell mkdir bar) because it is
> exported. At this point, the directory 'bar' does not exit yet. The cat
> command cannot resolve the bar/../foo path, hence the error message.
>
> Let's get back to the kernel Makefile.
>
> In arch/arm/boot/compressed/Makefile, KBSS_SZ is referenced by
> LDFLAGS_vmlinux, which is recursive and also exported by the top
> Makefile.
>
> GNU Make 4.3 expands KBSS_SZ just before running the recipes, so it
> works fine.
>
> GNU Make 4.4 expands KBSS_SZ in the parse stage, where the directory
> arm/arm/boot/compressed does not exit yet. When compiled O= option,
> the output directory is created by $(shell mkdir -p $(obj-dirs))
> in scripts/Makefile.build.
>
> There are two ways to fix this particular issue:
>
> - change "$(obj)/../../../../vmlinux" in KBSS_SZ to "vmlinux"
> - unexport LDFLAGS_vmlinux
>
> This commit takes the latter course because it is what I originally
> intended.
>
> Commit 3ec8a5b33dea ("kbuild: do not export LDFLAGS_vmlinux")
> unexported LDFLAGS_vmlinux.
>
> Commit 5d4aeffbf709 ("kbuild: rebuild .vmlinux.export.o when its
> prerequisite is updated") accidentally exported it again.
>
> We can clean up arch/arm/boot/compressed/Makefile later.
>
> [1]: https://git.savannah.gnu.org/cgit/make.git/commit/?id=98da874c43035a490cdca81331724f233a3d0c9a
>
> Link: https://lore.kernel.org/all/[email protected]/
> Fixes: 5d4aeffbf709 ("kbuild: rebuild .vmlinux.export.o when its prerequisite is updated")
> Reported-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Changes in v2:
> - Postpone the change for arch/arm/boot/compressed/Makefile
> - Update the commit description
>
> Makefile | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)

Once again thanks for the interesting details!

Reviewed-by: Nicolas Schier <[email protected]>


Attachments:
(No filename) (4.31 kB)
signature.asc (849.00 B)
Download all attachments