From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress To: Ard Biesheuvel ,devel@edk2.groups.io From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= X-Originating-Location: Berlin, Land Berlin, DE (104.28.62.40) X-Originating-Platform: Mac Safari 16.4 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Thu, 30 Mar 2023 14:54:02 -0700 References: <20230330212101.1566931-2-ardb@kernel.org> In-Reply-To: <20230330212101.1566931-2-ardb@kernel.org> Message-ID: <13330.1680213242478608984@groups.io> Content-Type: multipart/alternative; boundary="TCsgUk7FpRwDlmUyNRxK" --TCsgUk7FpRwDlmUyNRxK Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Ard, Sorry, I cannot preserve the CC list as the groups.io interface doesn't see= m to allow it. Can you please CC me on future revisions? This patch will badly corrupt binaries. I cannot cite a source right now (i= f you want me to, please remind me in your response, so I can look it up to= morrow), but for X64 (but not IA32, which is why this is enabled there), re= locs are relative to the first *writable* segment. In other words, any relo= cation to __TEXT will badly corrupt binaries this way. In AUDK, we support this with two essential changes. The first is that we a= lways generate a writable dummy segment at the beginning of the address spa= ce [1], making the relocs relative to the image base. The second is that in= ocmtoc, our fork of the abandoned (and pretty badly-bugged) Apple mtoc, we= explicitly require this segment to be present and verify its virtual addre= ss is the minimum virtual address [2]. It is then omitted from the conversi= on process [3]. I suggest you replicate these changes and fully switch to o= cmtoc for XCODE5 builds. Best regards, Marvin [1] https://github.com/acidanthera/audk/blob/c382e9c571c7d5f39ba53b46a0c723c794= 3f33c5/BaseTools/Conf/tools_def.template#L2976-L2988 [2] https://github.com/acidanthera/ocmtoc/blob/b0152c51beae264770c3faf0d213f959= 4ee043be/efitools/mtoc.c#L1097-L1123 https://github.com/acidanthera/ocmtoc/blob/b0152c51beae264770c3faf0d213f959= 4ee043be/efitools/mtoc.c#L1204-L1214 [3] https://github.com/acidanthera/ocmtoc/blob/b0152c51beae264770c3faf0d213f959= 4ee043be/efitools/mtoc.c#L1307-L1311 --TCsgUk7FpRwDlmUyNRxK Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Ard,

Sorry, I cannot preserve the CC list as the groups.io in= terface doesn't seem to allow it. Can you please CC me on future revisions?=

This patch will badly corrupt binaries. I cannot cite a source = right now (if you want me to, please remind me in your response, so I can l= ook it up tomorrow), but for X64 (but not IA32, which is why this is enable= d there), relocs are relative to the first *writable* segment. In other wor= ds, any relocation to __TEXT will badly corrupt binaries this way.
In AUDK, we support this with two essential changes. The first is that w= e always generate a writable dummy segment at the beginning of the address = space [1], making the relocs relative to the image base. The second is that= in ocmtoc, our fork of the abandoned (and pretty badly-bugged) Apple mtoc,= we explicitly require this segment to be present and verify its virtual ad= dress is the minimum virtual address [2]. It is then omitted from the conve= rsion process [3]. I suggest you replicate these changes and fully switch t= o ocmtoc for XCODE5 builds.

Best regards,
Marvin

[1]
= https://github.com/acidanthera/audk/blob/c382e9c571c7d5f39ba53b46a0c723c794= 3f33c5/BaseTools/Conf/tools_def.template#L2976-L2988

[2]
https://github.com/acida= nthera/ocmtoc/blob/b0152c51beae264770c3faf0d213f9594ee043be/efitools/mtoc.c= #L1097-L1123
http= s://github.com/acidanthera/ocmtoc/blob/b0152c51beae264770c3faf0d213f9594ee0= 43be/efitools/mtoc.c#L1204-L1214

[3]
https://github.com/acidanthera/ocmtoc/blob/b= 0152c51beae264770c3faf0d213f9594ee043be/efitools/mtoc.c#L1307-L1311 --TCsgUk7FpRwDlmUyNRxK--