[PATCH] x86/build: Specify stack alignment for clang

[PATCH] x86/build: Specify stack alignment for clang

[Matthias Kaehlcke]

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 <mka@chromium.org>
---
 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)
-- 
2.13.0.506.g27d5fe0cd-goog

Re: [PATCH] x86/build: Specify stack alignment for clang

[Ingo Molnar]

* Matthias Kaehlcke <mka@chromium.org> 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 <mka@chromium.org>
> ---
>  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.

BTW., GCC also has -mstack-align (note the different spelling), which does 
something else, so Clang's incompatibility is super confusing things...

Thanks,

	Ingo

Re: [PATCH] x86/build: Specify stack alignment for clang

[Matthias Kaehlcke]

El Sat, Jun 10, 2017 at 09:03:59AM +0200 Ingo Molnar ha dit:

> 
> * Matthias Kaehlcke <mka@chromium.org> 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 <mka@chromium.org>
> > ---
> >  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.

Re: [PATCH] x86/build: Specify stack alignment for clang

[Matthias Kaehlcke]

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 <mka@chromium.org> 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 <mka@chromium.org>
> > > ---
> > >  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)