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

[Xiang W <wxjstz@126.com>]

? 2021-07-10???? 19:10 +0100?Jessica Clarke???
> On 10 Jul 2021, at 19:07, Xiang W <wxjstz@126.com> wrote:
> > 
> > ? 2021-07-10???? 20:26 +0530?Anup Patel???
> > > On Sat, Jul 10, 2021 at 7:13 PM Bin Meng <bmeng.cn@gmail.com>
> > > wrote:
> > > > 
> > > > On Sat, Jul 10, 2021 at 9:38 PM Anup Patel
> > > > <anup@brainfault.org>
> > > > wrote:
> > > > > 
> > > > > On Sat, Jul 10, 2021 at 6:13 PM Bin Meng <bmeng.cn@gmail.com>
> > > > > wrote:
> > > > > > 
> > > > > > Hi Anup,
> > > > > > 
> > > > > > On Sat, Jul 10, 2021 at 6:42 PM Anup Patel
> > > > > > <anup@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
> > We can add the following code to the makefile to detect pie support
> > 
> > pie_support=$(shell $(CC) -nostdlib -fPIE -Wl,-pie /dev/null
> > 2>/dev/null && echo y || echo n)
> 
> That doesn?t work, /dev/null is not a valid object file so this will
> always fail with any toolchain.
I have tested it, so that the following commands no longer report
errors

CROSS_COMPILE=riscv64-unknown-elf- make PLATFORM=generic

Xiang W
> 
> Jess