On 22. Jun 2022, at 20:18, Andrew Fish <af...@apple.com> wrote:
> 
> 
> 
>> On Jun 22, 2022, at 10:43 AM, Marvin Häuser <mhaeu...@posteo.de> wrote:
>> 
>> Hey Andrew,
>> 
>> thanks for verifying! If my last mail was not clear, this is already done 
>> for IA32 builds, by the way. I suspect more trouble appeared there due to 
>> the lack of IP-relative addressing.
>> 
> 
> I though the issue was on x86_64 since by default that does not support 
> relocations in .text sections.

I don't think any architecture does with -pie, but I might be wrong. The issue 
was X64-only because IA32 had the flag already.

> The maintainer mentioned this should work for x86_64 but is not really 
> tested. If it works in our case we should be good to go.

Many thanks to you and the maintainer! :)

Actually, I re-checked my old changes because I remembered some funky things 
around the relocation reference address [1]. It turns out, this may not be safe 
as-is (may or may not depend on the Xcode version). Not due to the relocations 
themselves, that part is just fine, but due to the relocation base address. The 
last sentence of my linked comment [1] mentions this very use case, it seems I 
have tested this before. It seems like my fix (the __RELOC_FIX segment) does 
not require changes to mtoc and may not be a terrible idea to implement. The 
other changes are not required for this use case. I'm not positive how this 
impacts IA32, it might not at all.

Theo, please try to verify this before V2.

> The compiler always generates RIP relative addressing in the text section. It 
> is humans that generate absolution addressing by trying to access labels 
> directly.

Yes, my linked example is a NASM file that is duplicated for XCODE5 purposes to 
basically perform the relocation during runtime. Pretty nasty.

> 
> I tested the change the flags on some some existing code bases and booted on 
> real hardware and in a VM. I did not force a relocation into a text section, 
> I just tested existing platforms as is.

V2 should have a .text relocation by resolving the CpuExceptionHandlerLib 
situation. I hope we can also craft minimal examples that are easy to verify 
whether or not the .text relocations actually work.

Best regards,
Marvin

[1] 
https://github.com/mhaeuser/edk2/commit/f80af004945fd7a01cbd9fda7917275cb0ecf4e6#diff-1076da6462ee674323da1943762adf983e446819071ecff44ac68524bbab4ac3R2960-R2970

> 
> Historically what would happen is any assembler that generated a relocation 
> in the .text section would fail to link with Xcode and we would have to 
> convert it to RIP relative addressing after the fact. 
> 
> Thanks,
> 
> Andrew Fish
> 
>> Theo wanted to prepare a V2 with an enhanced commit message and a 2nd patch 
>> to remove the related hacks, I believe.
>> 
>> Best regards,
>> Marvin
>> 
>>> On 22. Jun 2022, at 19:22, Andrew Fish <af...@apple.com> wrote:
>>> 
>>> I reached out to the Xcode ld64 maintainer to make sure it is safe to use 
>>> -read_only_relocs suppress this way. 
>>> 
>>> Thanks,
>>> 
>>> Andrew Fish
>>> 
>>>> On Jun 18, 2022, at 8:19 PM, Andrew Fish via groups.io 
>>>> <afish=apple....@groups.io> wrote:
>>>> 
>>>> Marvin,
>>>> 
>>>> I’ll look into this.
>>>> 
>>>> The history here is the original ld64 flags are what was required for 
>>>> proper function. I got them
>>>> directly from the main ld64 maintainer. 
>>>> 
>>>> Big picture ld64 is the macOS and iOS linker, and it does not have 
>>>> official support for other targets, especially embedded. So the 
>>>> combination of flags are what was required for correctness as not all 
>>>> combination of possible ld64 flags are actually supported.
>>>> 
>>>> Back in the day we made clang open source contributions to get EFIABI 
>>>> supported in clang, but we have always used the stock ld64, with help from 
>>>> the author.
>>>>>> On Jun 18, 2022, at 8:03 PM, Marvin Häuser <mhaeu...@posteo.de> wrote:
>>>>> 
>>>>> CC Andrew, Rebecca, mentors
>>>>> 
>>>>> Hey all,
>>>>> 
>>>>> The patch itself looks good to me. The description doesn't really capture 
>>>>> the issue, nor why this is an adequate solution. This should also remove 
>>>>> the mentioned XCODE5-specific code as part of a single series [1] to 
>>>>> confirm the issue has been resolved without regressions.
>>>>> 
>>>>> TL;dr for the rest: Apple ld64 complains because there are relocations to 
>>>>> read-only segments in a PIE executable. As Mach-O allows mapping 
>>>>> read-only segments of PIEs to multiple virtual addresses (in different 
>>>>> processes), this is prohibited right at link-time for obvious reasons. 
>>>>> PE/COFF doesn't really have such a feature (I think Windows used to use 
>>>>> static addresses and now uses CoW with traditional relocs?) and UEFI has 
>>>>> no concept of page sharing anyway. Hence, it is safe to allow such relocs 
>>>>> and silence the warning. All other toolchains should already work this 
>>>>> way.
>>>>> 
>>>>> Andrew, Rebecca, if I remember correctly, you pretty much maintain 
>>>>> XCODE5. I had a conversation with Andrew about related topics before, 
>>>>> too. Are you fine with this approach? It seems like it has previously 
>>>>> been applied to IA32 builds already anyway (right from import).
>>>>> 
>>>>> Maybe PIE could be dropped as a whole somehow in the future? For UEFI, it 
>>>>> basically only adds overhead (or are there blockers to this?).
>>>>> 
>>>>> Best regards,
>>>>> Marvin
>>>>> 
>>>>> [1] 
>>>>> https://github.com/tianocore/edk2/tree/cc2db6ebfb6d9d85ba4c7b35fba1fa37fffc0bc2/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
>>>>> 
>>>>>> On 18. Jun 2022, at 15:21, Théo Jehl <theojeh...@gmail.com> wrote:
>>>>>> 
>>>>>> From: Theo Jehl <theojeh...@gmail.com>
>>>>>> 
>>>>>> Added -read_only_relocs suppress for XCODE5 X64 toolchain
>>>>>> This remove the needs for XCODE5 specific source with relocation fixes
>>>>>> 
>>>>>> Cc: Bob Feng <bob.c.f...@intel.com>
>>>>>> Cc: Liming Gao <gaolim...@byosoft.com.cn>
>>>>>> Cc: Yuwei Chen <yuwei.c...@intel.com>
>>>>>> Cc: Marvin Häuser <mhaeu...@posteo.de>
>>>>>> Cc: Vitaly Cheptsov <vit9...@protonmail.com>
>>>>>> 
>>>>>> Signed-off-by: Theo Jehl <theojeh...@gmail.com>
>>>>>> ---
>>>>>> BaseTools/Conf/tools_def.template | 6 +++---
>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>> 
>>>>>> diff --git a/BaseTools/Conf/tools_def.template 
>>>>>> b/BaseTools/Conf/tools_def.template
>>>>>> index 5ed19810b727..be35094cecc3 100755
>>>>>> --- a/BaseTools/Conf/tools_def.template
>>>>>> +++ b/BaseTools/Conf/tools_def.template
>>>>>> @@ -2977,9 +2977,9 @@ RELEASE_XCODE5_IA32_CC_FLAGS   = -arch i386 -c    
>>>>>> -Os       -Wall -Werror -inclu
>>>>>> ##################
>>>>>> # X64 definitions
>>>>>> ##################
>>>>>> -  DEBUG_XCODE5_X64_DLINK_FLAGS      = -arch x86_64 -u 
>>>>>> _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20  
>>>>>> -pie -all_load -dead_strip -seg1addr 0x240 -map 
>>>>>> $(DEST_DIR_DEBUG)/$(BASE_NAME).map
>>>>>> -  NOOPT_XCODE5_X64_DLINK_FLAGS      = -arch x86_64 -u 
>>>>>> _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20  
>>>>>> -pie -all_load -dead_strip -seg1addr 0x240 -map 
>>>>>> $(DEST_DIR_DEBUG)/$(BASE_NAME).map
>>>>>> -RELEASE_XCODE5_X64_DLINK_FLAGS      = -arch x86_64 -u 
>>>>>> _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20  
>>>>>> -pie -all_load -dead_strip -seg1addr 0x240 -map 
>>>>>> $(DEST_DIR_DEBUG)/$(BASE_NAME).map
>>>>>> +  DEBUG_XCODE5_X64_DLINK_FLAGS      = -arch x86_64 -u 
>>>>>> _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20  
>>>>>> -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress 
>>>>>> -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map
>>>>>> +  NOOPT_XCODE5_X64_DLINK_FLAGS      = -arch x86_64 -u 
>>>>>> _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20  
>>>>>> -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress 
>>>>>> -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map
>>>>>> +RELEASE_XCODE5_X64_DLINK_FLAGS      = -arch x86_64 -u 
>>>>>> _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20  
>>>>>> -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress 
>>>>>> -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map
>>>>>> 
>>>>>> *_XCODE5_X64_SLINK_FLAGS      = -static -o
>>>>>> DEBUG_XCODE5_X64_ASM_FLAGS  = -arch x86_64 -g
>>>>>> -- 
>>>>>> 2.32.1 (Apple Git-133)
>>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>> 
>> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90719): https://edk2.groups.io/g/devel/message/90719
Mute This Topic: https://groups.io/mt/91840126/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to