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

[Jessica Clarke]

On 10 Jul 2021, at 19:27, Xiang W <wxjstz at 126.com> wrote:
> 
> 在 2021-07-10星期六的 19:10 +0100，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.
> I have tested it, so that the following commands no longer report
> errors
> 
> CROSS_COMPILE=riscv64-unknown-elf- make PLATFORM=generic

It doesn’t work with Clang. It will always give an error as it regards
/dev/null as being an object file. You need to force it to parse
/dev/null as a C file with -x c. The same thing also happens with
riscv64-unknown-freebsd12.1-gcc. I don’t know what’s special about
riscv64-unknown-elf-gcc, though that *does* seem to do the right thing
currently, but only that. Even my native x86_64-linux-gnu-gcc treats
/dev/null as an object file. So something weird is going on in GCC
land, but -x c should ensure you always get what you want.

Jess