[U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot

[Tom Rini <trini@konsulko.com>]

On Tue, Jul 04, 2017 at 03:12:54AM +0000, Peng Fan wrote:
> 
> 
> > -----Original Message-----
> > From: Tom Rini [mailto:trini at konsulko.com]
> > Sent: Tuesday, July 04, 2017 10:47 AM
> > To: Peng Fan <peng.fan@nxp.com>
> > Cc: Simon Glass <sjg@chromium.org>; Philipp Tomsich
> > <philipp.tomsich@theobroma-systems.com>; albert.u.boot at aribaud.net; u-
> > boot at lists.denx.de
> > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and
> > normal U-Boot
> > 
> > On Tue, Jul 04, 2017 at 01:09:36AM +0000, Peng Fan wrote:
> > > Hi Tom,
> > >
> > > > -----Original Message-----
> > > > From: Tom Rini [mailto:trini at konsulko.com]
> > > > Sent: Tuesday, July 04, 2017 12:17 AM
> > > > To: Peng Fan <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>;
> > > > Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > > > Cc: albert.u.boot at aribaud.net; u-boot at lists.denx.de
> > > > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for
> > > > SPL and normal U-Boot
> > > >
> > > > On Mon, Jul 03, 2017 at 09:14:08PM +0800, Peng Fan wrote:
> > > >
> > > > > If not pass -fno-pic to toolchains, some toolchains may generate
> > > > > .got and .got.plt sections, but when generate binaries, we did not
> > > > > take .got and .got.plt into consideration, then SPL or normal
> > > > > U-Boot boot failure because image corrupted.
> > > > >
> > > > > Need to pass -fno-pic to disable generating .got and .got.plt
> > > > > sections.
> > > > >
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > > > > Cc: Tom Rini <trini@konsulko.com>
> > > > > ---
> > > > >  arch/arm/config.mk | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm/config.mk b/arch/arm/config.mk index
> > > > > 1a77779..66ae403 100644
> > > > > --- a/arch/arm/config.mk
> > > > > +++ b/arch/arm/config.mk
> > > > > @@ -130,9 +130,10 @@ ALL-y += checkarmreloc  # instruction.
> > > > > Relocation is not supported for that case, so disable  # such
> > > > > usage by requiring word relocations.
> > > > >  PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
> > > > > -PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)  endif
> > > > >
> > > > > +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
> > > > > +
> > > > >  # limit ourselves to the sections we want in the .bin.
> > > > >  ifdef CONFIG_ARM64
> > > > >  OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j
> > > > > .rodata -j .data \
> > > >
> > > > Something is "up" here and I need you to dig harder and perhaps see
> > > > if we're missing something in the linker scripts?  The very next line of
> > context here is:
> > > >                 -j .u_boot_list -j .rela.dyn -j .got -j .got.plt
> > > >
> > > > Meaning that we intentionally copy .got / .got.plt into the
> > > > resulting binary.  And I see that we took in 397d7d5a1be1 from you
> > > > back in 2016 saying that we needed this in SPL.  But 5a942a152776
> > > > put the got/got.plt sections (for 32bit
> > > > ARM) in intentionally as some relocations do need it.  And in
> > > > 4b0d506ed3b4 Philipp seems to have seen the same problem you have,
> > > > but fixed it with adding got/got.plt to the sections list we copy in (the above
> > hunk of context).
> > >
> > > If pass -fno-pic to compiler, there will be no .got and .got.plt sections.
> > > The .got and .got.plt is usually used for dynamic link, such as linux "*.so" file.
> > > We need to pass -fno-pic to compiler to remove .got and .got.plt sections.
> > 
> > "Usually" isn't the same as "always" or "only".  And this reminded me that we
> 
> From https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#Code-Gen-Options,
> when -fpic is used for ARM, there will be GOT. The dynamic loader resolves the GOT entries
> when the program starts (the dynamic loader is not part of GCC; it is part of the
> operating system). 
> 
> > had to deal with e391b1e64b0b because yes, we're not making a shared library
> > but we do have position independent code.  So, in the case of SPL, since we
> 
> For position independent code, but not making a shared library, we could use -fpie.

Note that "just" a switch to -fno-pic -fpie results in non-booting
platforms for me, so there'd be more work to be done there.

> > can get away with -fno-pic (and get some space
> > savings) that's just not true of U-Boot itself.  We're enforcing -fpic on other
> > architectures, so what exactly is going on with what you're seeing?  Where
> 
> If not passing -fno-pic to gcc, the android toolchain will generate
> .got and .got.plt section. I think these sections are for dynamic
> link, not for static link.

Right, but that's why we include these sections in the images now.  Have
you confirmed that top of tree doesn't work with that android toolchain,
without your second patch here?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170704/1cdb84d7/attachment.sig>