x86: Fix location of '-plugin-opt=' flags

PATCH Applied Nathan Chancellor 4 participants 6 messages May 18, 2021 – May 21, 2021

Subsystems (2)

Stable branch x86

lore

Share & tools

lore

Nathan Chancellor

author

2021-05-18 19:01 UTC #0 lore raw

PATCH 1/1 x86: Fix location of '-plugin-opt=' flags

Commit b33fff07e3e3 ("x86, build: allow LTO to be selected") added a couple of '-plugin-opt=' flags to KBUILD_LDFLAGS because the code model and stack alignment are not stored in LLVM bitcode. However, these flags were added to KBUILD_LDFLAGS prior to the emulation flag assignment, which uses ':=', so they were overwritten and never added to $(LD) invocations. The absence of these flags caused misalignment issues in the AMDGPU driver when compiling with CONFIG_LTO_CLANG, resulting in

general protection faults.

Shuffle the assignment below the initial one so that the flags are properly passed along and all of the linker flags stay together.

At the same time, avoid any future issues with clobbering flags by changing the emulation flag assignment to '+=' since KBUILD_LDFLAGS is already defined with ':=' in the main Makefile before being exported for modification here as a result of commit ce99d0bf312d ("kbuild: clear LDFLAGS in the top Makefile").

Cc [email protected]

Fixes b33fff07e3e3 ("x86, build: allow LTO to be selected")

Link https://github.com/ClangBuiltLinux/linux/issues/1374

Reported-by Anthony Ruhier <[email protected]>

Tested-by Anthony Ruhier <[email protected]>

Signed-off-by Nathan Chancellor <[email protected]>

---
 arch/x86/Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/x86/Makefile +6 -6

index c77c5d8a7b3e..307529417021 100644
@@ -178,11 +178,6 @@ ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
 	KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
 endif
 
-ifdef CONFIG_LTO_CLANG
-KBUILD_LDFLAGS	+= -plugin-opt=-code-model=kernel \
-		   -plugin-opt=-stack-alignment=$(if $(CONFIG_X86_32),4,8)
-endif
-
 # Workaround for a gcc prelease that unfortunately was shipped in a suse release
 KBUILD_CFLAGS += -Wno-sign-compare
 #

@@ -202,7 +197,12 @@ ifdef CONFIG_RETPOLINE
   endif
 endif
 
-KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
+KBUILD_LDFLAGS += -m elf_$(UTS_MACHINE)
+
+ifdef CONFIG_LTO_CLANG
+KBUILD_LDFLAGS	+= -plugin-opt=-code-model=kernel \
+		   -plugin-opt=-stack-alignment=$(if $(CONFIG_X86_32),4,8)
+endif
 
 ifdef CONFIG_X86_NEED_RELOCS
 LDFLAGS_vmlinux := --emit-relocs --discard-none

base-commit: d07f6ca923ea0927a1024dfccafc5b53b61cfecc

Signature · 1 line

2.32.0.rc0

Nick Desaulniers

Reviewed-by on v1 1/1

2021-05-18 20:24 UTC #1 lore raw

Quoted 10 lines · Nathan Chancellor

Commit b33fff07e3e3 ("x86, build: allow LTO to be selected") added a couple of '-plugin-opt=' flags to KBUILD_LDFLAGS because the code model and stack alignment are not stored in LLVM bitcode. However, these flags were added to KBUILD_LDFLAGS prior to the emulation flag assignment, which uses ':=', so they were overwritten and never added to $(LD) invocations. The absence of these flags caused misalignment issues in the AMDGPU driver when compiling with CONFIG_LTO_CLANG, resulting in
general protection faults.

Quoted 8 lines

Shuffle the assignment below the initial one so that the flags are properly passed along and all of the linker flags stay together.
At the same time, avoid any future issues with clobbering flags by changing the emulation flag assignment to '+=' since KBUILD_LDFLAGS is already defined with ':=' in the main Makefile before being exported for modification here as a result of commit ce99d0bf312d ("kbuild: clear LDFLAGS in the top Makefile").

Thanks for the patch file. I will need to be more wary of := operator in kbuild changes in the future.

Ideally, we should encode these two flags in LLVM's IR so that we don't need to respecify them when restarting optimizations from the linker during LTO. I've filed
https://github.com/ClangBuiltLinux/linux/issues/1377 to follow up on that idea.

Reviewed-by Nick Desaulniers <[email protected]>

Quoted 46 lines

Cc [email protected]

Fixes b33fff07e3e3 ("x86, build: allow LTO to be selected")

Link https://github.com/ClangBuiltLinux/linux/issues/1374

Reported-by Anthony Ruhier <[email protected]>

Tested-by Anthony Ruhier <[email protected]>

Signed-off-by Nathan Chancellor <[email protected]>

---
 arch/x86/Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/x86/Makefile +6 -6

index c77c5d8a7b3e..307529417021 100644
@@ -178,11 +178,6 @@ ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
        KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
 endif
 
-ifdef CONFIG_LTO_CLANG
-KBUILD_LDFLAGS += -plugin-opt=-code-model=kernel \
-                  -plugin-opt=-stack-alignment=$(if $(CONFIG_X86_32),4,8)
-endif
-
 # Workaround for a gcc prelease that unfortunately was shipped in a suse release
 KBUILD_CFLAGS += -Wno-sign-compare
 #

@@ -202,7 +197,12 @@ ifdef CONFIG_RETPOLINE
   endif
 endif
 
-KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
+KBUILD_LDFLAGS += -m elf_$(UTS_MACHINE)
+
+ifdef CONFIG_LTO_CLANG
+KBUILD_LDFLAGS += -plugin-opt=-code-model=kernel \
+                  -plugin-opt=-stack-alignment=$(if $(CONFIG_X86_32),4,8)
+endif
 
 ifdef CONFIG_X86_NEED_RELOCS
 LDFLAGS_vmlinux := --emit-relocs --discard-none

base-commit: d07f6ca923ea0927a1024dfccafc5b53b61cfecc --
2.32.0.rc0

Signature · 2 lines

Thanks,
~Nick Desaulniers

tip-bot2 for Nathan Chancellor

merge bot Applied

2021-05-19 11:24 UTC #2 lore raw

[tip: x86/urgent] x86/build: Fix location of '-plugin-opt=' flags

In reply to Nathan Chancellor

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     0024430e920f2900654ad83cd081cf52e02a3ef5
Gitweb:        https://git.kernel.org/tip/0024430e920f2900654ad83cd081cf52e02a3ef5
Author:        Nathan Chancellor <[email protected]>
AuthorDate:    Tue, 18 May 2021 12:01:06 -07:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Wed, 19 May 2021 13:05:53 +02:00

x86/build: Fix location of '-plugin-opt=' flags

Commit b33fff07e3e3 ("x86, build: allow LTO to be selected") added a couple of '-plugin-opt=' flags to KBUILD_LDFLAGS because the code model and stack alignment are not stored in LLVM bitcode.

However, these flags were added to KBUILD_LDFLAGS prior to the emulation flag assignment, which uses ':=', so they were overwritten and never added to $(LD) invocations.

The absence of these flags caused misalignment issues in the AMDGPU driver when compiling with CONFIG_LTO_CLANG, resulting in

general protection faults.

Shuffle the assignment below the initial one so that the flags are properly passed along and all of the linker flags stay together.

ce99d0bf312d ("kbuild: clear LDFLAGS in the top Makefile")

Fixes b33fff07e3e3 ("x86, build: allow LTO to be selected")

Reported-by Anthony Ruhier <[email protected]>

Signed-off-by Nathan Chancellor <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>

Tested-by Anthony Ruhier <[email protected]>

Cc [email protected]

Link https://github.com/ClangBuiltLinux/linux/issues/1374

Link https://lore.kernel.org/r/[email protected] open here

---
 arch/x86/Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/x86/Makefile +6 -6

index c77c5d8..3075294 100644
@@ -178,11 +178,6 @@ ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
 	KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
 endif
 
-ifdef CONFIG_LTO_CLANG
-KBUILD_LDFLAGS	+= -plugin-opt=-code-model=kernel \
-		   -plugin-opt=-stack-alignment=$(if $(CONFIG_X86_32),4,8)
-endif
-
 # Workaround for a gcc prelease that unfortunately was shipped in a suse release
 KBUILD_CFLAGS += -Wno-sign-compare
 #

@@ -202,7 +197,12 @@ ifdef CONFIG_RETPOLINE
   endif
 endif
 
-KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
+KBUILD_LDFLAGS += -m elf_$(UTS_MACHINE)
+
+ifdef CONFIG_LTO_CLANG
+KBUILD_LDFLAGS	+= -plugin-opt=-code-model=kernel \
+		   -plugin-opt=-stack-alignment=$(if $(CONFIG_X86_32),4,8)
+endif
 
 ifdef CONFIG_X86_NEED_RELOCS
 LDFLAGS_vmlinux := --emit-relocs --discard-none

Kees Cook

Applied

2021-05-21 17:59 UTC #3 lore raw

In reply to Nathan Chancellor

Quoted 10 lines · Nathan Chancellor

Commit b33fff07e3e3 ("x86, build: allow LTO to be selected") added a couple of '-plugin-opt=' flags to KBUILD_LDFLAGS because the code model and stack alignment are not stored in LLVM bitcode. However, these flags were added to KBUILD_LDFLAGS prior to the emulation flag assignment, which uses ':=', so they were overwritten and never added to $(LD) invocations. The absence of these flags caused misalignment issues in the AMDGPU driver when compiling with CONFIG_LTO_CLANG, resulting in
general protection faults.
[...]

(I've slightly adjusted the title.)

Applied to for-next/clang/features, thanks!

[1/1] x86: lto: Fix location of '-plugin-opt=' flags

      https://git.kernel.org/kees/c/5d6c8592ee5f

Signature · 1 line

Kees Cook

Nathan Chancellor

author

2021-05-21 18:32 UTC #4 lore raw

Quoted 10 lines · Kees Cook

Quoted 9 lines · Nathan Chancellor
Commit b33fff07e3e3 ("x86, build: allow LTO to be selected") added a couple of '-plugin-opt=' flags to KBUILD_LDFLAGS because the code model and stack alignment are not stored in LLVM bitcode. However, these flags were added to KBUILD_LDFLAGS prior to the emulation flag assignment, which uses ':=', so they were overwritten and never added to $(LD) invocations. The absence of these flags caused misalignment issues in the AMDGPU driver when compiling with CONFIG_LTO_CLANG, resulting in
general protection faults.

Quoted 9 lines

Quoted 1 line
[...]
(I've slightly adjusted the title.)
Applied to for-next/clang/features, thanks!
[1/1] x86: lto: Fix location of '-plugin-opt=' flags
      https://git.kernel.org/kees/c/5d6c8592ee5f

Ingo picked this up in x86/urgent so you should not need to carry it.

https://git.kernel.org/tip/0024430e920f2900654ad83cd081cf52e02a3ef5

Cheers,
Nathan

Kees Cook

2021-05-21 21:04 UTC #5 lore raw

Quoted 17 lines · Nathan Chancellor

Quoted 16 lines · Kees Cook
Quoted 10 lines · Nathan Chancellor
Commit b33fff07e3e3 ("x86, build: allow LTO to be selected") added a couple of '-plugin-opt=' flags to KBUILD_LDFLAGS because the code model and stack alignment are not stored in LLVM bitcode. However, these flags were added to KBUILD_LDFLAGS prior to the emulation flag assignment, which uses ':=', so they were overwritten and never added to $(LD) invocations. The absence of these flags caused misalignment issues in the AMDGPU driver when compiling with CONFIG_LTO_CLANG, resulting in
general protection faults.
[...]
(I've slightly adjusted the title.)
Applied to for-next/clang/features, thanks!

Quoted 7 lines

Quoted 3 lines
[1/1] x86: lto: Fix location of '-plugin-opt=' flags
      https://git.kernel.org/kees/c/5d6c8592ee5f
Ingo picked this up in x86/urgent so you should not need to carry it.
https://git.kernel.org/tip/0024430e920f2900654ad83cd081cf52e02a3ef5

Ah-ha, thanks! I didn't see a reply on the thread.

Signature · 1 line

Kees Cook