From: "Andrew Fish" <afish@apple.com>
To: devel@edk2.groups.io, Andrew Fish <afish@apple.com>
Cc: "Marvin Häuser" <mhaeuser@posteo.de>,
"Théo Jehl" <theojehl76@gmail.com>,
"Bob Feng" <bob.c.feng@intel.com>,
"Liming Gao" <gaoliming@byosoft.com.cn>,
"Yuwei Chen" <yuwei.chen@intel.com>,
"Vitaly Cheptsov" <vit9696@protonmail.com>,
"Rebecca Cran" <quic_rcran@quicinc.com>,
"Pedro Falcato" <pedro.falcato@gmail.com>,
"Isaac W Oram" <isaac.w.oram@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Suppress read only relocs errors on XCODE5 X64 toolchain
Date: Wed, 22 Jun 2022 10:22:26 -0700 [thread overview]
Message-ID: <5C4BDE52-3FA5-4683-ABF2-FEEB447A6DCE@apple.com> (raw)
In-Reply-To: <16F9E770868D27D7.32537@groups.io>
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.com@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 <mhaeuser@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 <theojehl76@gmail.com> wrote:
>>>
>>> From: Theo Jehl <theojehl76@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.feng@intel.com>
>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>> Cc: Yuwei Chen <yuwei.chen@intel.com>
>>> Cc: Marvin Häuser <mhaeuser@posteo.de>
>>> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
>>>
>>> Signed-off-by: Theo Jehl <theojehl76@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)
>>>
>>
>
>
>
>
>
next prev parent reply other threads:[~2022-06-22 17:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-18 13:21 [PATCH v1 1/1] BaseTools: Suppress read only relocs errors on XCODE5 X64 toolchain Théo Jehl
2022-06-18 14:08 ` Marvin Häuser
2022-06-19 3:19 ` [edk2-devel] " Andrew Fish
[not found] ` <16F9E770868D27D7.32537@groups.io>
2022-06-22 17:22 ` Andrew Fish [this message]
2022-06-22 17:43 ` Marvin Häuser
2022-06-22 18:18 ` Andrew Fish
2022-06-22 18:43 ` Marvin Häuser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5C4BDE52-3FA5-4683-ABF2-FEEB447A6DCE@apple.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox