[boot-wrapper PATCH v2 3/9] Makefile: Tell compiler to generate bare-metal code

[Mark Rutland]

On Fri, Jan 07, 2022 at 02:38:11PM +0000, Andre Przywara wrote:
> On Fri, 7 Jan 2022 13:53:50 +0000
> Mark Rutland <mark.rutland at arm.com> wrote:
> 
> Hi Mark,
> 
> > On Wed, Dec 22, 2021 at 06:16:01PM +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 and headers, since they might pull in more code than
> > > we want, and might not run well in the boot-wrapper environment.
> > > 
> > > This also enables optimisation, since it produces much better code and
> > > tends to avoid problems due to missing inlining, for instance.  
> > 
> > I'd appreciate if we could add some explanation for these (just in the commit
> > message is fine), as e.g. it's not clear to me why -fno-toplevel-reorder is
> > necessary.
> 
> Yeah, I was expecting some discussion about this. I went over most options
> from the manpage a while ago (for some unrelated baremetal code project),
> and decided for each option whether it would make sense or not. TBH I
> don't remember every detail from that, but I can certainly just drop the
> less critical options from that list.

Yes please.

To be clear, I'm only suggesting dropping a few of these, and I think the
others just needs some explanation.

I'm certain we *should* add:

	-ffreestanding
	-nostdlib

... and I'd be happy to take a patch adding just those as an immediate fix.

I think the folllowing are reasonable but need some explanation:

	-Wstrict-prototypes		// Just to catch accidents
	-fno-stack-protector		// Because we dont have any init/support code
	-fno-strict-aliasing		// Do we rely on type punning today?
	-fno-unwind-tables		// Because we don't consume the result
	-fno-asynchronous-unwind-tables	// Because we don't consume the result

I don't understand why we need:

	-fno-common
	-fno-toplevel-reorder
	-Os

... and I suspect we can drop those unless we have some rationale.

I'm torn over:

	-nostdinc

... as IIUC that's just to catch accidental includes, but necessitates the
prior patch adding copies of some headers. We could arguably leave that as-is
and just be careful.

> And I would be happy to add some rationale for each of them, but wonder
> what the best place would be? I'd rather do this in a file than in a
> commit message, would you prefer comments in Makefile.am, or in README or
> a separate file, with a pointer from Makefile.am. Or in a commit message
> anyway?

I think within the Makefile would be best if you're happy to organise it, e.g.

	# The boot-wrapper is a non-hosted environment and isn't linked
	# with standard libraries.
	CFLAGS += -ffreestanding -nostdlib

	# Avoid the pointless creation og a GOT
	CFLAGS += -fno-pic -fno-pie

... does that work for you?

Thanks,
Mark.

> 
> Cheers,
> Andre
> 
> > > 
> > > Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > > ---
> > >  Makefile.am | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 3e970a3..d9ad6d1 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -124,9 +124,14 @@ CHOSEN_NODE	:= chosen {						\
> > >  
> > >  CPPFLAGS	+= $(INITRD_FLAGS)
> > >  CFLAGS		+= -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/
> > > -CFLAGS		+= -Wall -fomit-frame-pointer
> > > +CFLAGS		+= -Wall -Wstrict-prototypes -fomit-frame-pointer
> > > +CFLAGS		+= -ffreestanding -nostdinc -nostdlib
> > > +CFLAGS		+= -fno-common -fno-strict-aliasing -fno-stack-protector
> > > +CFLAGS		+= -fno-toplevel-reorder
> > > +CFLAGS		+= -fno-unwind-tables -fno-asynchronous-unwind-tables
> > >  CFLAGS		+= -ffunction-sections -fdata-sections
> > >  CFLAGS		+= -fno-pic -fno-pie
> > > +CFLAGS		+= -Os
> > >  LDFLAGS		+= --gc-sections
> > >  
> > >  OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
> > > -- 
> > > 2.25.1
> > >   
>