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

[Peng Fan]

> -----Original Message-----
> From: Tom Rini [mailto:trini at konsulko.com]
> Sent: Tuesday, July 04, 2017 9:35 PM
> To: Peng Fan <peng.fan at nxp.com>
> Cc: Simon Glass <sjg at chromium.org>; Philipp Tomsich
> <philipp.tomsich at 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 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 at nxp.com>
> > > Cc: Simon Glass <sjg at chromium.org>; Philipp Tomsich
> > > <philipp.tomsich at 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 at nxp.com>; Simon Glass <sjg at chromium.org>;
> > > > > Philipp Tomsich <philipp.tomsich at 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 at nxp.com>
> > > > > > Cc: Albert Aribaud <albert.u.boot at aribaud.net>
> > > > > > Cc: Tom Rini <trini at 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-Opti
> > ons, 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?

I am using 2017.03 release.  Without the second patch, gcc will generate instructions
like "str q0, [x2]", and uboot runs into sync abort when booting .

Regards,
Peng.

> 
> --
> Tom