From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web09.11830.1655923395751336503 for ; Wed, 22 Jun 2022 11:43:16 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=XSRSjIww; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id EE1BD24010B for ; Wed, 22 Jun 2022 20:43:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1655923394; bh=VQBU2TScnqZjW97ej3PtHbhQHldqz9Wbm9hcaTn7s3g=; h=Subject:From:Date:Cc:To:From; b=XSRSjIwwZJqOOiSU+6fcwqNcOezP2fRG9eIKQIFvnqXmSQf3qRG6u+yX967uU5rlh gpdNfVcPUMHQ6gA0KKjebZ9VHCLA8zhKEq8Z3+QpRLsOpdJgQSG8hlqrBuifNh/dem AxC8uJD0BQXEQvwwwvOt5iVsai/YEAEyaGVNVGHZAxVncqAJIeLqPk6zaPurmJYog+ yu+QZ9RlRZEMRUXgoM9tdJskx1CBzWufQ1I58NHyj14salctok8bZK0dk//Nzfg+hk +NxIrC7ih9VNz3s9v4I0SjTW83k0jWLTlD0jt3iePDR0Lwizf1Gr3xVKyGaQMoatxA hl7wvr3jE3nKQ== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4LSshZ227Mz6tm6; Wed, 22 Jun 2022 20:43:10 +0200 (CEST) Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.100.31\)) Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Suppress read only relocs errors on XCODE5 X64 toolchain From: =?utf-8?Q?Marvin_H=C3=A4user?= In-Reply-To: <34AF20A4-8FF4-4971-9754-EEE36091EA9D@apple.com> Date: Wed, 22 Jun 2022 18:43:09 +0000 Cc: edk2-devel-groups-io , =?utf-8?Q?Th=C3=A9o_Jehl?= , Bob Feng , Liming Gao , Yuwei Chen , Vitaly Cheptsov , Rebecca Cran , Pedro Falcato , Isaac W Oram Message-Id: <8A807270-ED55-4139-8244-D1F622DDAA48@posteo.de> References: <5C4BDE52-3FA5-4683-ABF2-FEEB447A6DCE@apple.com> <34AF20A4-8FF4-4971-9754-EEE36091EA9D@apple.com> To: Andrew Fish Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 22. Jun 2022, at 20:18, Andrew Fish wrote: >=20 >=20 >=20 >> On Jun 22, 2022, at 10:43 AM, Marvin H=C3=A4user wr= ote: >>=20 >> Hey Andrew, >>=20 >> thanks for verifying! If my last mail was not clear, this is already don= e for IA32 builds, by the way. I suspect more trouble appeared there due to= the lack of IP-relative addressing. >>=20 >=20 > I though the issue was on x86_64 since by default that does not support r= elocations in .text sections. I don't think any architecture does with -pie, but I might be wrong. The is= sue was X64-only because IA32 had the flag already. > The maintainer mentioned this should work for x86_64 but is not really te= sted. 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 thing= s around the relocation reference address [1]. It turns out, this may not b= e safe as-is (may or may not depend on the Xcode version). Not due to the r= elocations themselves, that part is just fine, but due to the relocation ba= se address. The last sentence of my linked comment [1] mentions this very u= se case, it seems I have tested this before. It seems like my fix (the __RE= LOC_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 labe= ls directly. Yes, my linked example is a NASM file that is duplicated for XCODE5 purpose= s to basically perform the relocation during runtime. Pretty nasty. >=20 > 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 sec= tion, I just tested existing platforms as is. V2 should have a .text relocation by resolving the CpuExceptionHandlerLib s= ituation. 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/f80af004945fd7a01cbd9fda7917275= cb0ecf4e6#diff-1076da6462ee674323da1943762adf983e446819071ecff44ac68524bbab= 4ac3R2960-R2970 >=20 > Historically what would happen is any assembler that generated a relocati= on in the .text section would fail to link with Xcode and we would have to = convert it to RIP relative addressing after the fact.=20 >=20 > Thanks, >=20 > Andrew Fish >=20 >> Theo wanted to prepare a V2 with an enhanced commit message and a 2nd pa= tch to remove the related hacks, I believe. >>=20 >> Best regards, >> Marvin >>=20 >>> On 22. Jun 2022, at 19:22, Andrew Fish wrote: >>>=20 >>> =EF=BB=BFI reached out to the Xcode ld64 maintainer to make sure it is = safe to use -read_only_relocs suppress this way.=20 >>>=20 >>> Thanks, >>>=20 >>> Andrew Fish >>>=20 >>>> On Jun 18, 2022, at 8:19 PM, Andrew Fish via groups.io wrote: >>>>=20 >>>> Marvin, >>>>=20 >>>> I=E2=80=99ll look into this. >>>>=20 >>>> The history here is the original ld64 flags are what was required for = proper function. I got them >>>> directly from the main ld64 maintainer.=20 >>>>=20 >>>> Big picture ld64 is the macOS and iOS linker, and it does not have off= icial support for other targets, especially embedded. So the combination of= flags are what was required for correctness as not all combination of poss= ible ld64 flags are actually supported. >>>>=20 >>>> 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=C3=A4user = wrote: >>>>>=20 >>>>> =EF=BB=BFCC Andrew, Rebecca, mentors >>>>>=20 >>>>> Hey all, >>>>>=20 >>>>> The patch itself looks good to me. The description doesn't really cap= ture the issue, nor why this is an adequate solution. This should also remo= ve the mentioned XCODE5-specific code as part of a single series [1] to con= firm the issue has been resolved without regressions. >>>>>=20 >>>>> TL;dr for the rest: Apple ld64 complains because there are relocation= s 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 war= ning. All other toolchains should already work this way. >>>>>=20 >>>>> Andrew, Rebecca, if I remember correctly, you pretty much maintain XC= ODE5. I had a conversation with Andrew about related topics before, too. Ar= e you fine with this approach? It seems like it has previously been applied= to IA32 builds already anyway (right from import). >>>>>=20 >>>>> 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?). >>>>>=20 >>>>> Best regards, >>>>> Marvin >>>>>=20 >>>>> [1] https://github.com/tianocore/edk2/tree/cc2db6ebfb6d9d85ba4c7b35fb= a1fa37fffc0bc2/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5Exceptio= nHandlerAsm.nasm >>>>>=20 >>>>>> On 18. Jun 2022, at 15:21, Th=C3=A9o Jehl wro= te: >>>>>>=20 >>>>>> From: Theo Jehl >>>>>>=20 >>>>>> Added -read_only_relocs suppress for XCODE5 X64 toolchain >>>>>> This remove the needs for XCODE5 specific source with relocation fix= es >>>>>>=20 >>>>>> Cc: Bob Feng >>>>>> Cc: Liming Gao >>>>>> Cc: Yuwei Chen >>>>>> Cc: Marvin H=C3=A4user >>>>>> Cc: Vitaly Cheptsov >>>>>>=20 >>>>>> Signed-off-by: Theo Jehl >>>>>> --- >>>>>> BaseTools/Conf/tools_def.template | 6 +++--- >>>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>>>=20 >>>>>> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tool= s_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 =3D -arch i386 = -c -Os -Wall -Werror -inclu >>>>>> ################## >>>>>> # X64 definitions >>>>>> ################## >>>>>> - DEBUG_XCODE5_X64_DLINK_FLAGS =3D -arch x86_64 -u _$(IMAGE_EN= TRY_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 =3D -arch x86_64 -u _$(IMAGE_EN= TRY_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 =3D -arch x86_64 -u _$(IMAGE_EN= TRY_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 =3D -arch x86_64 -u _$(IMAGE_EN= TRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load= -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEB= UG)/$(BASE_NAME).map >>>>>> + NOOPT_XCODE5_X64_DLINK_FLAGS =3D -arch x86_64 -u _$(IMAGE_EN= TRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load= -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEB= UG)/$(BASE_NAME).map >>>>>> +RELEASE_XCODE5_X64_DLINK_FLAGS =3D -arch x86_64 -u _$(IMAGE_EN= TRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load= -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEB= UG)/$(BASE_NAME).map >>>>>>=20 >>>>>> *_XCODE5_X64_SLINK_FLAGS =3D -static -o >>>>>> DEBUG_XCODE5_X64_ASM_FLAGS =3D -arch x86_64 -g >>>>>> --=20 >>>>>> 2.32.1 (Apple Git-133) >>>>>>=20 >>>>>=20 >>>>=20 >>>>=20 >>>>=20 >>>>=20 >>>>=20 >>>=20 >>=20 >=20