From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752582AbdFLSeb (ORCPT ); Mon, 12 Jun 2017 14:34:31 -0400 Received: from mail-pg0-f47.google.com ([74.125.83.47]:35702 "EHLO mail-pg0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751594AbdFLSea (ORCPT ); Mon, 12 Jun 2017 14:34:30 -0400 Date: Mon, 12 Jun 2017 11:34:27 -0700 From: Matthias Kaehlcke To: Ingo Molnar Cc: Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , "H . J . Lu" , David Woodhouse , x86@kernel.org, linux-kernel@vger.kernel.org, Michael Davidson , Greg Hackmann , Nick Desaulniers , Stephen Hines , Kees Cook , Arnd Bergmann , Bernhard.Rosenkranzer@linaro.org, Peter Foley , Behan Webster , Douglas Anderson , Linus Torvalds , Peter Zijlstra Subject: Re: [PATCH] x86/build: Specify stack alignment for clang Message-ID: <20170612183427.GJ141096@google.com> References: <20170609173134.181976-1-mka@chromium.org> <20170610070359.7pci2anwe3pvvcea@gmail.com> <20170612161714.GI141096@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170612161714.GI141096@google.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org El Mon, Jun 12, 2017 at 09:17:14AM -0700 Matthias Kaehlcke ha dit: > El Sat, Jun 10, 2017 at 09:03:59AM +0200 Ingo Molnar ha dit: > > > > > * Matthias Kaehlcke wrote: > > > > > For gcc stack alignment is configured with -mpreferred-stack-boundary=N, > > > clang has the option -mstack-alignment=N for that purpose. Use the same > > > alignment as for gcc. > > > > > > If the alignment is not specified clang assumes an alignment of 16 bytes, > > > as required by the standard ABI. However as mentioned in d9b0cde91c60 > > > ("x86-64, gcc: Use -mpreferred-stack-boundary=3 if supported") the > > > standard kernel entry on x86-64 leaves the stack on an 8-byte > > > boundary, as a consequence clang will keep the stack misaligned. > > > > > > Signed-off-by: Matthias Kaehlcke > > > --- > > > arch/x86/Makefile | 9 ++++++--- > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile > > > index 5851411e60fb..a32badbe87ad 100644 > > > --- a/arch/x86/Makefile > > > +++ b/arch/x86/Makefile > > > @@ -27,7 +27,8 @@ REALMODE_CFLAGS := $(M16_CFLAGS) -g -Os -D__KERNEL__ \ > > > -mno-mmx -mno-sse \ > > > $(call cc-option, -ffreestanding) \ > > > $(call cc-option, -fno-stack-protector) \ > > > - $(call cc-option, -mpreferred-stack-boundary=2) > > > + $(call cc-option, -mpreferred-stack-boundary=2) \ > > > + $(call cc-option, -mstack-alignment=2) > > > export REALMODE_CFLAGS > > > > > > # BITS is used as extension for files which are available in a 32 bit > > > @@ -64,8 +65,9 @@ ifeq ($(CONFIG_X86_32),y) > > > # with nonstandard options > > > KBUILD_CFLAGS += -fno-pic > > > > > > - # prevent gcc from keeping the stack 16 byte aligned > > > + # prevent the compiler from keeping the stack 16 byte aligned > > > KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=2) > > > + KBUILD_CFLAGS += $(call cc-option,-mstack-alignment=2) > > > > > > # Disable unit-at-a-time mode on pre-gcc-4.0 compilers, it makes gcc use > > > # a lot more stack due to the lack of sharing of stacklots: > > > @@ -97,8 +99,9 @@ else > > > KBUILD_CFLAGS += $(call cc-option,-mno-80387) > > > KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387) > > > > > > - # Use -mpreferred-stack-boundary=3 if supported. > > > + # Align the stack to 8 bytes if supported. > > > KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3) > > > + KBUILD_CFLAGS += $(call cc-option,-mstack-alignment=3) > > > > > > # Use -mskip-rax-setup if supported. > > > KBUILD_CFLAGS += $(call cc-option,-mskip-rax-setup) > > > > That's really ugly and the duplicated options are repeated. A cleaner solution > > would be introduce a variable that stores the option, and use that later on. Also > > describe the variable, pointing out that -mpreferred-stack-boundary is a GCC > > option, while -mstack-alignment is a Clang one. > > Thanks for your comments, I will send out an updated patch shortly. While testing I found that with the current Makefile the stack alignment option is not set in REALMODE_CFLAGS. The reason is that 'cc-option' uses KBUILD_CPPFLAGS and CC_OPTION_CFLAGS while checking if an option is valid, and not REALMODE_CFLAGS or CODE16GCC_CFLAGS. As a result -m32 is not set (not even for i386 builds, it is set further down in the Makefile) and without it gcc expects an alignment value >= 4: gcc -Werror -mpreferred-stack-boundary=2 -c -x c /dev/null -o /dev/null /dev/null:1:0: error: -mpreferred-stack-boundary=2 is not between 4 and 12 REALMODE_CFLAGS was introduced by 1c678da3bd13 ("x86: Remove duplication of 16-bit CFLAGS"), previously KBUILD_CFLAGS were used for the boot code. However I think the issue already existed before this change since adding the '-m32' and the check for '-mpreferred-stack-boundary=2' were done in a single assignment: KBUILD_CFLAGS := $(USERINCLUDE) -m32 -g -Os -D_SETUP -D__KERNEL__ \ ... $(call cc-option, -mpreferred-stack-boundary=2)