lkml.org 
[lkml]   [2017]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86/build: Specify stack alignment for clang
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)

\
 
 \ /
  Last update: 2017-06-12 20:58    [W:1.017 / U:0.336 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site