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

[Tom Rini <trini@konsulko.com>]

On Wed, Jul 05, 2017 at 01:02:42AM +0000, Peng Fan wrote:
> 
> 
> > -----Original Message-----
> > From: Tom Rini [mailto:trini at konsulko.com]
> > Sent: Tuesday, July 04, 2017 9:35 PM
> > 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 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-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 .

So you're missing the patch I mentioned that added .got/.got.plt then :)

-- 
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/9ac0d89c/attachment.sig>