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

[Jessica Clarke]

On 10 Jul 2021, at 19:07, Xiang W <wxjstz at 126.com> wrote:
> 
> 在 2021-07-10星期六的 20:26 +0530，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
> 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.

Jess