[PATCH v3] Makefile: Tell compiler to generate bare-metal code

[Andre Przywara]

On Mon, 17 Jan 2022 16:47:20 +0000
Mark Rutland <mark.rutland at arm.com> wrote:

> Hi Andre,
> 
> On Mon, Jan 17, 2022 at 03:33:48PM +0000, Andre Przywara wrote:
> > Our GCC invocation does not provide many parameters, which lets the
> > toolchain fill in its own default setup.
> > In case of a native build or when using a full-featured cross-compiler,
> > this probably means Linux userland, which is not what we want for a
> > bare-metal application like boot-wrapper.
> > 
> > Tell the compiler to forget about those standard settings, and only use
> > what we explicitly ask for. In particular that means to not use toolchain
> > provided libraries, since they might pull in more code than we want, and
> > might not run well in the boot-wrapper environment.  
> 
> Thanks for splitting this out; I'd like to apply this with two minor
> fixups (which I'll apply locally if you agree).

Yes, I am fine with those changes, please go ahead!

> > Disable the stack protector, as this adds code that relies on userland:
> > "If a guard check fails, an error message is printed and the program
> > exits." (from the gcc manpage).  
> 
> I think we should say:
> 
> | Disable the stack protector, as this relies on support code, e.g.
> | a __stack_chk_guard variable and __stack_chk_fail function, which the
> | boot-wrapper does not implement.
> 
> > Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > ---
> > Hi,
> > 
> > compared to [PATCH v2 3/9] this drops the more contentious options (for
> > now, at least), and focuses on what's really needed. Including
> > -fno-stack-protector, as the need for this showed up in the cleanup series
> > already.
> > 
> > Cheers,
> > Andre
> > 
> >  Makefile.am | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index f941b07..581840e 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -125,6 +125,8 @@ CHOSEN_NODE	:= chosen {						\
> >  CPPFLAGS	+= $(INITRD_FLAGS)
> >  CFLAGS		+= -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/
> >  CFLAGS		+= -Wall -fomit-frame-pointer
> > +CFLAGS		+= -ffreestanding -nostdlib	# this is not for userland
> > +CFLAGS		+= -fno-stack-protector		# no terminal to print  
> 
> For now, I'd like to drop these comments, since I think the "no terminal
> to print" comment is a bit confusing, and the "this is not for userland"
> comment is arguably the most obvious.
> 
> Then as a future follow-up we can reoganise this for clarity:
> 
> | # Build code suitable for a bare-metal environment which does not
> | # depend on any support libraries
> | CFLAGS	+= ffreestanding -nostdlib
> | 
> | # Disable unnecessary features which require runtime support
> | CFLAGS	+= -fno-stack-protector
> | CFLAGS	+= ...
> |
> | # Disable unnecessary features which impact code-size
> | CFLAGS	+= -fomit-frame-pointer
> | CFLAGS	+= ...
> |
> | # Allow the linker to remove unused code and data to shrink the
> | # resulting binary
> | CFLAGS	+= -ffunction-sections -fdata-sections
> | LDFLAGS	+= --gc-sections

Yes, that looks good to me as a plan. I agree that trying to fit
meaningful comment to the rest of the line was not a great idea.

Cheers,
Andre

> 
> 
> >  CFLAGS		+= -ffunction-sections -fdata-sections
> >  CFLAGS		+= -fno-pic -fno-pie
> >  LDFLAGS		+= --gc-sections
> > -- 
> > 2.25.1
> >