celelibi at gmail.com
2015-Nov-27 22:41 UTC
[syslinux] [PATCH 0/2] Do not use the "red zone" on EFI
From: Sylvain Gault <sylvain.gault at gmail.com> The System V ABI for x86-64 specify that a "red zone" is an area of 128 bytes above the current stack frame. This area can be used by a called function in order to avoid the overhead of modifying the stack pointer. The direct effect is that interrupt/event/signal handlers must not write to this area. In the UEFI calling convention, there is no such thing and the event handlers do write their data just above the current stack frame. However, gcc generate by default code that uses the red zone. This has to be disabled explicitely with the option -mno-red-zone. Not doing so lead to some functions behaving randomly once in a while. Fixing this revealed that some Makefiles out of the efi/ directory have some specific options when building for BIOS or for EFI. These Makefiles do this by testing the EFI_BUILD variable. However, this variable wasn't passed down the Makefiles making these specific options never used. Best regards, Sylvain Gault Sylvain Gault (2): Makefile: Pass down the variable EFI_BUILD Makefile: Always use -mno-red-zone for EFI Makefile | 17 +++++++++-------- com32/Makefile | 2 +- diag/Makefile | 2 +- mk/com32.mk | 4 +++- mk/elf.mk | 6 ++++-- mk/embedded.mk | 5 ++--- mk/lib.mk | 4 +++- tests/Makefile | 6 +++--- 8 files changed, 26 insertions(+), 20 deletions(-) -- 2.6.2
celelibi at gmail.com
2015-Nov-27 22:41 UTC
[syslinux] [PATCH 1/2] Makefile: Pass down the variable EFI_BUILD
From: Sylvain Gault <sylvain.gault at gmail.com> This variable indicates whether or nor the files are compiled for EFI. The lack of it lead the Makefiles to forget to add some compilation options specific to EFI. Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com> --- Makefile | 17 +++++++++-------- com32/Makefile | 2 +- diag/Makefile | 2 +- tests/Makefile | 6 +++--- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 615ccc4..79cfd63 100644 --- a/Makefile +++ b/Makefile @@ -102,7 +102,8 @@ endif .PHONY: $(filter-out $(private-targets), $(MAKECMDGOALS)) $(filter-out $(private-targets), $(MAKECMDGOALS)): $(MAKE) -C $(OBJDIR) -f $(CURDIR)/Makefile SRC="$(topdir)" \ - OBJ=$(OBJDIR) objdir=$(OBJDIR) $(MAKECMDGOALS) + OBJ=$(OBJDIR) objdir=$(OBJDIR) EFI_BUILD=$(EFI_BUILD) \ + $(MAKECMDGOALS) unittest: printf "Executing unit tests\n" @@ -111,7 +112,7 @@ unittest: regression: $(MAKE) -C tests SRC="$(topdir)/tests" OBJ="$(topdir)/tests" \ - objdir=$(OBJDIR) \ + objdir=$(OBJDIR) EFI_BUILD=$(EFI_BUILD) \ -f $(topdir)/tests/Makefile all test: unittest regression @@ -284,17 +285,17 @@ subdirs: $(BSUBDIRS) $(ISUBDIRS) $(sort $(ISUBDIRS) $(BSUBDIRS)): @mkdir -p $@ - $(MAKE) -C $@ SRC="$(SRC)/$@" OBJ="$(OBJ)/$@" \ + $(MAKE) -C $@ SRC="$(SRC)/$@" OBJ="$(OBJ)/$@" EFI_BUILD=$(EFI_BUILD) \ -f $(SRC)/$@/Makefile $(MAKECMDGOALS) $(ITARGET): @mkdir -p $@ - $(MAKE) -C $@ SRC="$(SRC)/$@" OBJ="$(OBJ)/$@" \ + $(MAKE) -C $@ SRC="$(SRC)/$@" OBJ="$(OBJ)/$@" EFI_BUILD=$(EFI_BUILD) \ -f $(SRC)/$@/Makefile $(MAKECMDGOALS) $(BINFILES): @mkdir -p $@ - $(MAKE) -C $@ SRC="$(SRC)/$@" OBJ="$(OBJ)/$@" \ + $(MAKE) -C $@ SRC="$(SRC)/$@" OBJ="$(OBJ)/$@" EFI_BUILD=$(EFI_BUILD) \ -f $(SRC)/$@/Makefile $(MAKECMDGOALS) # @@ -309,7 +310,7 @@ gpxe: core installer: installer-local set -e; for i in $(ISUBDIRS); \ do $(MAKE) -C $$i SRC="$(SRC)/$$i" OBJ="$(OBJ)/$$i" \ - -f $(SRC)/$$i/Makefile all; done + EFI_BUILD=$(EFI_BUILD) -f $(SRC)/$$i/Makefile all; done installer-local: $(ITARGET) $(BINFILES) @@ -317,7 +318,7 @@ installer-local: $(ITARGET) $(BINFILES) strip: strip-local set -e; for i in $(ISUBDIRS); \ do $(MAKE) -C $$i SRC="$(SRC)/$$i" OBJ="$(OBJ)/$$i" \ - -f $(SRC)/$$i/Makefile strip; done + EFI_BUILD=$(EFI_BUILD) -f $(SRC)/$$i/Makefile strip; done -ls -l $(BOBJECTS) $(IOBJECTS) strip-local: @@ -355,7 +356,7 @@ install: set -e ; for i in $(INSTALLSUBDIRS) ; \ do $(MAKE) -C $$i SRC="$(SRC)/$$i" OBJ="$(OBJ)/$$i" \ BITS="$(BITS)" AUXDIR="$(AUXDIR)/efi$(BITS)" \ - -f $(SRC)/$$i/Makefile $@; done + EFI_BUILD=$(EFI_BUILD) -f $(SRC)/$$i/Makefile $@; done -install -m 644 $(INSTALLABLE_MODULES) $(INSTALLROOT)$(AUXDIR)/efi$(BITS) install -m 644 com32/elflink/ldlinux/$(LDLINUX) $(INSTALLROOT)$(AUXDIR)/efi$(BITS) endif diff --git a/com32/Makefile b/com32/Makefile index 5efda1c..3e7a770 100644 --- a/com32/Makefile +++ b/com32/Makefile @@ -6,7 +6,7 @@ subdirs: $(SUBDIRS) $(SUBDIRS): @mkdir -p $(OBJ)/$@ $(MAKE) -C $(OBJ)/$@ SRC="$(SRC)"/$@ OBJ="$(OBJ)"/$@/ \ - -f $(SRC)/$@/Makefile $(MAKECMDGOALS) + EFI_BUILD=$(EFI_BUILD) -f $(SRC)/$@/Makefile $(MAKECMDGOALS) all tidy dist clean spotless install: subdirs diff --git a/diag/Makefile b/diag/Makefile index e335375..619c163 100644 --- a/diag/Makefile +++ b/diag/Makefile @@ -4,4 +4,4 @@ all tidy dist clean spotless install: @mkdir -p $(addprefix $(OBJ)/,$(SUBDIRS)) set -e; for d in $(SUBDIRS); \ do $(MAKE) -C $(OBJ)/$$d -f $(SRC)/$$d/Makefile \ - SRC="$(SRC)"/$$d OBJ="$(OBJ)"/$$d $@; done + SRC="$(SRC)"/$$d OBJ="$(OBJ)"/$$d EFI_BUILD=$(EFI_BUILD) $@; done diff --git a/tests/Makefile b/tests/Makefile index 99b1618..7c0f979 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -39,7 +39,7 @@ QEMU = qemu-system-i386 all: syslinux-tests pxelinux-tests syslinux-tests: - $(MAKE) SRC="$(SRC)" OBJ="$(OBJ)" objdir="$(objdir)" \ + $(MAKE) SRC="$(SRC)" OBJ="$(OBJ)" objdir="$(objdir)" EFI_BUILD=$(EFI_BUILD) \ INITIAL="$(INITIAL)" INSTALL_DIR="$(SYSLINUX_INSTALL_DIR)" \ CONFIG_FILE="$(SYSLINUX_CONFIG_FILE)" \ DERIVATIVE=SYSLINUX @@ -47,7 +47,7 @@ syslinux-tests: $(SYSLINUX_LOOPDEV) test.cfg pxelinux-tests: - $(MAKE) SRC="$(SRC)" OBJ="$(OBJ)" objdir="$(objdir)" \ + $(MAKE) SRC="$(SRC)" OBJ="$(OBJ)" objdir="$(objdir)" EFI_BUILD=$(EFI_BUILD) \ INITIAL="$(INITIAL)" INSTALL_DIR="$(PXELINUX_INSTALL_DIR)" \ CONFIG_FILE="$(PXELINUX_CONFIG_FILE)" \ DERIVATIVE=PXELINUX @@ -71,7 +71,7 @@ SUBDIRS = linux com32 subdirs: $(SUBDIRS) $(SUBDIRS): mkdir -p $(OBJ)/$@ - $(MAKE) -C $(OBJ)/$@ SRC="$(SRC)"/$@ OBJ="$(OBJ)"/$@/ \ + $(MAKE) -C $(OBJ)/$@ SRC="$(SRC)"/$@ OBJ="$(OBJ)"/$@/ EFI_BUILD=$(EFI_BUILD) \ -f $(SRC)/$@/Makefile objdir="$(objdir)/tests" \ INSTALL_DIR="$(INSTALL_DIR)" CONFIG_FILE="$(CONFIG_FILE)" \ DERIVATIVE="$(DERIVATIVE)" -- 2.6.2
celelibi at gmail.com
2015-Nov-27 22:41 UTC
[syslinux] [PATCH 2/2] Makefile: Always use -mno-red-zone for EFI
From: Sylvain Gault <sylvain.gault at gmail.com> This option is mandatory when compiling for EFI as some event handlers may interupt the running code and use the space just above the reserved stack space. Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com> --- mk/com32.mk | 4 +++- mk/elf.mk | 6 ++++-- mk/embedded.mk | 5 ++--- mk/lib.mk | 4 +++- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/mk/com32.mk b/mk/com32.mk index 90eb7ed..2e8591a 100644 --- a/mk/com32.mk +++ b/mk/com32.mk @@ -48,7 +48,9 @@ GCCOPT += $(call gcc_ok,-falign-jumps=0,-malign-jumps=0) GCCOPT += $(call gcc_ok,-falign-labels=0,-malign-labels=0) GCCOPT += $(call gcc_ok,-falign-loops=0,-malign-loops=0) -ifndef EFI_BUILD +ifdef EFI_BUILD +GCCOPT += -mno-red-zone +else GCCOPT += -mregparm=3 -DREGPARM=3 endif diff --git a/mk/elf.mk b/mk/elf.mk index 12d1077..54fd4ce 100644 --- a/mk/elf.mk +++ b/mk/elf.mk @@ -60,8 +60,10 @@ CFLAGS = $(GCCOPT) $(GCCWARN) -W -Wall \ -I$(com32)/libutil/include -I$(com32)/include \ -I$(com32)/include/sys $(GPLINCLUDE) -I$(core)/include \ -I$(objdir) -DLDLINUX=\"$(LDLINUX)\" -ifndef EFI_BUILD -CFLAGS += -mregparm=3 -DREGPARM=3 +ifdef EFI_BUILD +GCCOPT += -mno-red-zone +else +GCCOPT += -mregparm=3 -DREGPARM=3 endif SFLAGS = $(GCCOPT) -D__COM32__ -D__FIRMWARE_$(FIRMWARE)__ diff --git a/mk/embedded.mk b/mk/embedded.mk index 1614d8bc..df8e85f 100644 --- a/mk/embedded.mk +++ b/mk/embedded.mk @@ -39,10 +39,9 @@ GCCOPT += $(call gcc_ok,-fno-stack-protector,) GCCOPT += $(call gcc_ok,-fwrapv,) GCCOPT += $(call gcc_ok,-freg-struct-return,) ifdef EFI_BUILD -GCCOPT += -Os -fomit-frame-pointer -msoft-float +GCCOPT += -Os -fomit-frame-pointer -msoft-float -mno-red-zone else -GCCOPT += -Os -fomit-frame-pointer -mregparm=3 -DREGPARM=3 \ - -msoft-float +GCCOPT += -Os -fomit-frame-pointer -mregparm=3 -DREGPARM=3 -msoft-float endif GCCOPT += $(call gcc_ok,-fno-exceptions,) GCCOPT += $(call gcc_ok,-fno-asynchronous-unwind-tables,) diff --git a/mk/lib.mk b/mk/lib.mk index ceb95bd..7a48756 100644 --- a/mk/lib.mk +++ b/mk/lib.mk @@ -54,7 +54,9 @@ WARNFLAGS = $(GCCWARN) -Wpointer-arith -Wwrite-strings -Wstrict-prototypes -Winl CFLAGS = $(OPTFLAGS) $(REQFLAGS) $(WARNFLAGS) $(LIBFLAGS) -ifndef EFI_BUILD +ifdef EFI_BUILD +CFLAGS += -mno-red-zone +else CFLAGS += -mregparm=3 -DREGPARM=3 endif -- 2.6.2
Geert Stappers
2015-Nov-28 06:42 UTC
[syslinux] [PATCH 0/2] Do not use the "red zone" on EFI
On Fri, Nov 27, 2015 at 11:41:47PM +0100, celelibi--- via Syslinux wrote:> From: Sylvain Gault <sylvain.gault at gmail.com> > > The System V ABI for x86-64 specify that a "red zone" is an area of 128 bytes > above the current stack frame. This area can be used by a called function in > order to avoid the overhead of modifying the stack pointer. The direct effect > is that interrupt/event/signal handlers must not write to this area. In the > UEFI calling convention, there is no such thing and the event handlers do write > their data just above the current stack frame. > > However, gcc generate by default code that uses the red zone. This has to be > disabled explicitely with the option -mno-red-zone. Not doing so lead to some > functions behaving randomly once in a while. > > Fixing this revealed that some Makefiles out of the efi/ directory have some > specific options when building for BIOS or for EFI. These Makefiles do this by > testing the EFI_BUILD variable. However, this variable wasn't passed down the > Makefiles making these specific options never used.YEE, that is some _serious_ research work. Respect!> Best regards, > Sylvain GaultGroeten Geert Stappers -- Leven en laten leven ------------- volgend deel ------------ Een niet-tekst bijlage is gescrubt... Naam: signature.asc Type: application/pgp-signature Grootte: 836 bytes Omschrijving: Digital signature URL : <http://www.zytor.com/pipermail/syslinux/attachments/20151128/4a5ca303/attachment.sig>
> From: Sylvain Gault <sylvain.gault at gmail.com> > > The System V ABI for x86-64 specify that a "red zone" is an area of 128 bytes > above the current stack frame. This area can be used by a called function in > order to avoid the overhead of modifying the stack pointer. The direct effect > is that interrupt/event/signal handlers must not write to this area. In the > UEFI calling convention, there is no such thing and the event handlers do write > their data just above the current stack frame. > > However, gcc generate by default code that uses the red zone. This has to be > disabled explicitely with the option -mno-red-zone. Not doing so lead to some > functions behaving randomly once in a while. > > Fixing this revealed that some Makefiles out of the efi/ directory have some > specific options when building for BIOS or for EFI. These Makefiles do this by > testing the EFI_BUILD variable. However, this variable wasn't passed down the > Makefiles making these specific options never used. >The addition of the EFI_BUILD variable inside Makefiles could potentially affect scripts such as package builders, perhaps even the way the official (pre)release archives are built(?). For further details and explanation, see the commit "The make files have undergone changes to support both i386 and x86_64 platforms" (2012Jun): repo.or.cz/syslinux.git/commit/38e58635d3868c23537fc5dce87b152a52df34ad Perhaps we should consider how to improve the Makefiles without potentially breaking backwards compatibility? Regards, Ady. PS: Shouldn't this type of things (as described in the aforementioned commit from 2012Jun) be included in "./doc/building.txt" and in the corresponding wiki page?
2015-11-28 8:47 UTC+01:00, Ady via Syslinux <syslinux at zytor.com>:> >> From: Sylvain Gault <sylvain.gault at gmail.com> >> >> The System V ABI for x86-64 specify that a "red zone" is an area of 128 >> bytes >> above the current stack frame. This area can be used by a called function >> in >> order to avoid the overhead of modifying the stack pointer. The direct >> effect >> is that interrupt/event/signal handlers must not write to this area. In >> the >> UEFI calling convention, there is no such thing and the event handlers do >> write >> their data just above the current stack frame. >> >> However, gcc generate by default code that uses the red zone. This has to >> be >> disabled explicitely with the option -mno-red-zone. Not doing so lead to >> some >> functions behaving randomly once in a while. >> >> Fixing this revealed that some Makefiles out of the efi/ directory have >> some >> specific options when building for BIOS or for EFI. These Makefiles do >> this by >> testing the EFI_BUILD variable. However, this variable wasn't passed down >> the >> Makefiles making these specific options never used. >> > > The addition of the EFI_BUILD variable inside Makefiles could potentially > affect scripts such as package builders, perhaps even the way the > official (pre)release archives are built(?).This variable is not something you're supposed to see or use when building Syslinux. I kinda have the feeling that according to you, no commit shall ever be made again because it may have an interaction with the user / maintainer. Maybe some users rely on the presence of the bugs. Who knows? The difference between a bug and a feature is pretty subjective after all.> > For further details and explanation, see the commit "The make files have > undergone changes to support both i386 and x86_64 platforms" (2012Jun): > repo.or.cz/syslinux.git/commit/38e58635d3868c23537fc5dce87b152a52df34ad > > Perhaps we should consider how to improve the Makefiles without > potentially breaking backwards compatibility?The way I think the Makefiles could be improved is by rewriting them from scratch in a non-recursive way.> PS: Shouldn't this type of things (as described in the aforementioned > commit from 2012Jun) be included in "./doc/building.txt" and in the > corresponding wiki page?Why should they? Celelibi
On Nov 28, 2015 2:51 AM, "Ady via Syslinux" <syslinux at zytor.com> wrote:> > From: Sylvain Gault <sylvain.gault at gmail.com> > > > > The System V ABI for x86-64 specify that a "red zone" is an area of 128bytes> > above the current stack frame. This area can be used by a calledfunction in> > order to avoid the overhead of modifying the stack pointer. The directeffect> > is that interrupt/event/signal handlers must not write to this area. Inthe> > UEFI calling convention, there is no such thing and the event handlersdo write> > their data just above the current stack frame. > > > > However, gcc generate by default code that uses the red zone. This hasto be> > disabled explicitely with the option -mno-red-zone. Not doing so leadto some> > functions behaving randomly once in a while. > > > > Fixing this revealed that some Makefiles out of the efi/ directory havesome> > specific options when building for BIOS or for EFI. These Makefiles dothis by> > testing the EFI_BUILD variable. However, this variable wasn't passeddown the> > Makefiles making these specific options never used.> The addition of the EFI_BUILD variable inside Makefiles could potentially > affect scripts such as package builders, perhaps even the way the > official (pre)release archives are built(?).Feels like a merge/design oversight.> For further details and explanation, see the commit "The make files have > undergone changes to support both i386 and x86_64 platforms" (2012Jun): > repo.or.cz/syslinux.git/commit/38e58635d3868c23537fc5dce87b152a52df34adThis looks superseded by the current design that can simultaneously build all three architectures at once.> Perhaps we should consider how to improve the Makefiles without > potentially breaking backwards compatibility? > > Regards, > Ady. > > PS: Shouldn't this type of things (as described in the aforementioned > commit from 2012Jun) be included in "./doc/building.txt" and in the > corresponding wiki page?See above. --Gene
On Nov 27, 2015 5:44 PM, "celelibi--- via Syslinux" <syslinux at zytor.com> wrote:> > From: Sylvain Gault <sylvain.gault at gmail.com> > > The System V ABI for x86-64 specify that a "red zone" is an area of 128bytes> above the current stack frame. This area can be used by a called functionin> order to avoid the overhead of modifying the stack pointer. The directeffect> is that interrupt/event/signal handlers must not write to this area. Inthe> UEFI calling convention, there is no such thing and the event handlers dowrite> their data just above the current stack frame. > > However, gcc generate by default code that uses the red zone. This has tobe> disabled explicitely with the option -mno-red-zone. Not doing so lead tosome> functions behaving randomly once in a while. > > Fixing this revealed that some Makefiles out of the efi/ directory havesome> specific options when building for BIOS or for EFI. These Makefiles dothis by> testing the EFI_BUILD variable. However, this variable wasn't passed downthe> Makefiles making these specific options never used. > > Best regards, > Sylvain GaultAlmost done with a round of testing and I hope to push the patch tonight or tomorrow. --Gene
On Fri, Dec 4, 2015 at 7:37 PM, Gene Cumm <gene.cumm at gmail.com> wrote:> On Nov 27, 2015 5:44 PM, "celelibi--- via Syslinux" <syslinux at zytor.com> > wrote: >> >> From: Sylvain Gault <sylvain.gault at gmail.com> >> >> The System V ABI for x86-64 specify that a "red zone" is an area of 128 >> bytes >> above the current stack frame. This area can be used by a called function >> in >> order to avoid the overhead of modifying the stack pointer. The direct >> effect >> is that interrupt/event/signal handlers must not write to this area. In >> the >> UEFI calling convention, there is no such thing and the event handlers do >> write >> their data just above the current stack frame. >> >> However, gcc generate by default code that uses the red zone. This has to >> be >> disabled explicitely with the option -mno-red-zone. Not doing so lead to >> some >> functions behaving randomly once in a while. >> >> Fixing this revealed that some Makefiles out of the efi/ directory have >> some >> specific options when building for BIOS or for EFI. These Makefiles do >> this by >> testing the EFI_BUILD variable. However, this variable wasn't passed down >> the >> Makefiles making these specific options never used. >> >> Best regards, >> Sylvain Gault > > Almost done with a round of testing and I hope to push the patch tonight or > tomorrow. > > --GenePushed. -- -Gene