[PATCH v4 3/5] firmware: Explicitly pass -pie to the linker, not just the driver

[Anup Patel]

On Sat, Jul 10, 2021 at 7:13 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> On Sat, Jul 10, 2021 at 9:38 PM Anup Patel <anup at brainfault.org> wrote:
> >
> > On Sat, Jul 10, 2021 at 6:13 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Anup,
> > >
> > > On Sat, Jul 10, 2021 at 6:42 PM Anup Patel <anup at brainfault.org> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Sat, Jul 10, 2021 at 2:23 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > >
> > > > > On Sat, Jul 10, 2021 at 10:56 AM Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > >
> > > > > > On Sat, Jul 10, 2021 at 3:35 AM Jessica Clarke <jrtc27 at jrtc27.com> wrote:
> > > > > > >
> > > > > > > When using Clang with a bare-metal triple, -pie does not get passed to
> > > > > > > the linker as it's not normally a thing that makes sense. However, in
> > > > > > > our case it is, and manually forwarding it on works as desired, so do so
> > > > > > > to fully support FW_PIC with Clang, including when linking with LLD.
> > > > > > > ---
> > > > > > >  firmware/objects.mk | 2 +-
> > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/firmware/objects.mk b/firmware/objects.mk
> > > > > > > index ce91c2f..3bc83cd 100644
> > > > > > > --- a/firmware/objects.mk
> > > > > > > +++ b/firmware/objects.mk
> > > > > > > @@ -21,7 +21,7 @@ ifeq ($(FW_PIC),y)
> > > > > > >  firmware-genflags-y += -DFW_PIC
> > > > > > >  firmware-asflags-y  += -fpic
> > > > > > >  firmware-cflags-y   += -fPIE -pie
> > > > > > > -firmware-ldflags-y  +=  -Wl,--no-dynamic-linker
> > > > > > > +firmware-ldflags-y  +=  -Wl,--no-dynamic-linker -Wl,-pie
> > > > > > >  endif
> > > > > >
> > > > > > Does this manual forwarding also work for GNU ld? If so, I think we
> > > > > > don't need to detect bare-metal triple and turn off FW_PIC in
> > > > > > Makefile?
> > > > >
> > > > > I just built a riscv64-unknown-elf-gcc toolchain and used it to build
> > > > > the current HEAD of opensbi/master. Indeed it's broken that pie is not
> > > > > supported with the bare-metal triple.
> > > > >
> > > > > The GNU ld simply complains:
> > > > >
> > > > >  ELF       platform/generic/firmware/payloads/test.elf
> > > > > /opt/riscv-unknown-elf/lib/gcc/riscv64-unknown-elf/10.1.0/../../../../riscv64-unknown-elf/bin/ld.bfd:
> > > > > -pie not supported
> > > >
> > > > The firmware/objects.mk is doing the following:
> > > >
> > > > ifndef FW_PIC
> > > > FW_PIC := y
> > > > endif
> > > >
> > > > Instead of above, we should set FW_PIC=y only when the underlying
> > > > toolchain supports pie.
> > > >
> > >
> > > Agree.
> > >
> > > > We need a patch for this to be merged before we can merge this
> > > > series. Can you send such a patch ? If not then I can send it.
> > >
> > > Do you have some reliable ways to check whether a toolchain supports PIE?
> >
> > How about checking "-linux-" in CROSS_COMPILE prefix ? If it's available then
> > we set FW_PIC=y else we set FW_PIC=n.
>
> That works for cross-compile toolchains. But how about native
> toolchains (building OpenSBI on a RISC-V machine)?

Yes, it will not work for native compilation of OpenSBI.

>
> > I did not find any info in the "${CROSS_COMPILE}gcc -v" output.
>
> Me neither.
>
> >
> > Maybe also add some comment in objects.mk that GCC bare-metal toolchain
> > does not have PIE enabled.
>
> Or we can just document if using bare-metal toolchain FW_PIC has to be
> set to n in the build.

Sounds good.

We should document it under section "Required Toolchain" of top-level
README.md. Basically, we prefer toolchains with PIE support and for
toolchains not having PIE users have to pass the "FW_PIC=n" option.

Also, we should replace references of bare-metal toolchain prefix with
linux toolchain prefix everywhere in documentation.

Regards,
Anup

>
> Regards,
> Bin