Acked-by: Bob Feng <bob.c.f...@intel.com>
-----Original Message-----
From: Leif Lindholm <quic_llind...@quicinc.com>
Sent: Tuesday, September 6, 2022 7:54 PM
To: Ard Biesheuvel <a...@kernel.org>; Rebecca Cran <rebe...@bsdio.com>
Cc: devel@edk2.groups.io; Feng, Bob C <bob.c.f...@intel.com>; Gao, Liming
<gaolim...@byosoft.com.cn>; Kinney, Michael D <michael.d.kin...@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: Convert more types
of explicit GOT references
On 2022-09-06 11:30, Ard Biesheuvel wrote:
> Bob, Liming, Leif: any thoughts?
I'm happy with this.
(Spotted one typo - "ofset".)
Acked-by: Leif Lindholm <quic_llind...@quicinc.com>
> On Sun, 21 Aug 2022 at 16:33, Rebecca Cran <rebe...@bsdio.com> wrote:
>>
>> Tested-by: Rebecca Cran <rebe...@bsdio.com>
>>
>>
>> On 8/21/22 08:16, Ard Biesheuvel wrote:
>>> Rebecca reports that builds of AArch64 DSCs that involve PIE linking
>>> when using ELF based toolchains are failing in some cases, resulting
>>> in an error message like
>>>
>>> bad definition for symbol '_GLOBAL_OFFSET_TABLE_'@0x72d8 or
>>> unsupported symbol type. For example, absolute and undefined symbols
>>> are not supported.
>>>
>>> The reason turns out to be that, while GenFw does carry some logic
>>> to convert GOT based symbol references into direct ones (which is
>>> always possible given that our ELF to PE/COFF conversion only
>>> supports fully linked executables), it does not support all possible
>>> combinations of relocations that the linker may emit to load symbol
>>> addresses from the GOT.
>>>
>>> In particular, when performing a non-LTO link on object code built
>>> with GCC using -fpie, we may end up with GOT based references such
>>> as the one below, where the address of the GOT itself is taken, and
>>> the ofset of the symbol in the GOT is reflected in the immediate
>>> offset of the subsequent LDR instruction.
>>>
>>> 838: adrp x0, 16000
>>> 838: R_AARCH64_ADR_PREL_PG_HI21 _GLOBAL_OFFSET_TABLE_
>>> 83c: ldr x0, [x0, #2536]
>>> 83c: R_AARCH64_LD64_GOTPAGE_LO15
>>> _gPcd_BinaryPatch_PcdFdBaseAddress
>>>
>>> The reason that we omit GOT based symbol references when performing
>>> ELF to PE/COFF conversion is that the GOT is not described by static
>>> ELF relocations, which means that the ELF file lacks the metadata to
>>> generate the PE/COFF relocations covering the GOT table in the
>>> PE/COFF executable. Given that none of the usual motivations for
>>> using a GOT (copy on write footprint, shared libraries) apply to EFI
>>> executables in the first place, the easiest way around this is to
>>> convert all GOT based symbol address loads to PC relative ADR/ADRP
>>> instructions.
>>>
>>> So implement this handling for R_AARCH64_LD64_GOTPAGE_LO15 and
>>> R_AARCH64_LD64_GOTOFF_LO15 relocations as well, and turn the LDR
>>> instructions in question into ADR instructions that generate the
>>> address immediately.
>>>
>>> This leaves the reference to _GLOBAL_OFFSET_TABLE_ itself, which is
>>> what generated the error to begin with. Considering that this symbol
>>> is never referenced (i.e., it doesn't appear anywhere in the code)
>>> and is only meaningful in combination with R_*_GOT_* based
>>> relocations that follow it, we can just disregard any references to
>>> it entirely, given that we convert all of those followup relocations into
>>> direct references.
>>>
>>> Cc: Rebecca Cran <rebe...@bsdio.com>
>>> Cc: Bob Feng <bob.c.f...@intel.com>
>>> Cc: Liming Gao <gaolim...@byosoft.com.cn>
>>> Cc: "Kinney, Michael D" <michael.d.kin...@intel.com>
>>> Cc: Leif Lindholm <quic_llind...@quicinc.com>
>>> Signed-off-by: Ard Biesheuvel <a...@kernel.org>
>>> ---
>>>
>>> This patch can be tested using the following method:
>>>
>>> - add the following lines to
>>> ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
>>>
>>> [BuildOptions]
>>> GCC:*_*_AARCH64_CC_FLAGS = -fpie
>>> GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,-Bsymbolic,-pie
>>>
>>> - build ArmVirtPkg/ArmVirtQemuKernel.dsc in DEBUG mode using GCC49
>>> (other combos might work as well) and observe the build failure
>>>
>>> - apply this patch and observe that the build failure is gone
>>>
>>> - boot the resulting image in QEMU using the -kernel ../QEMU_EFI.fd
>>> command line option and observe that the image boots as usual
>>>
>>> BaseTools/Source/C/GenFw/Elf64Convert.c | 35 ++++++++++++++++++++
>>> 1 file changed, 35 insertions(+)
>>>
>>> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
>>> b/BaseTools/Source/C/GenFw/Elf64Convert.c
>>> index 35e96dd05bc2..3173ca9280f4 100644
>>> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
>>> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
>>> @@ -1305,6 +1305,22 @@ WriteSections64 (
>>> Elf_Shdr *SymShdr;
>>> UINT8 *Targ;
>>>
>>> + //
>>> + // The _GLOBAL_OFFSET_TABLE_ symbol is not actually an absolute
>>> symbol,
>>> + // but carries the SHN_ABS section index for historical reasons.
>>> + // It must be accompanied by a R_*_GOT_* type relocation on a
>>> + // subsequent instruction, which we handle below, specifically to
>>> avoid
>>> + // the GOT indirection, and to refer to the symbol directly. This
>>> means
>>> + // we can simply disregard direct references to the GOT symbol
>>> itself,
>>> + // as the resulting value will never be used.
>>> + //
>>> + if (Sym->st_shndx == SHN_ABS) {
>>> + const UINT8 *SymName = GetSymName (Sym);
>>> + if (strcmp ((CHAR8 *)SymName, "_GLOBAL_OFFSET_TABLE_") == 0) {
>>> + continue;
>>> + }
>>> + }
>>> +
>>> //
>>> // Check section header index found in symbol table and get the
>>> section
>>> // header location.
>>> @@ -1448,6 +1464,23 @@ WriteSections64 (
>>> switch (ELF_R_TYPE(Rel->r_info)) {
>>> INT64 Offset;
>>>
>>> + case R_AARCH64_LD64_GOTOFF_LO15:
>>> + case R_AARCH64_LD64_GOTPAGE_LO15:
>>> + //
>>> + // Convert into an ADR instruction that refers to the symbol
>>> directly.
>>> + //
>>> + Offset = Sym->st_value - Rel->r_offset;
>>> +
>>> + *(UINT32 *)Targ &= 0x1000001f;
>>> + *(UINT32 *)Targ |= ((Offset & 0x1ffffc) << (5 - 2)) |
>>> + ((Offset & 0x3) << 29);
>>> +
>>> + if (Offset < -0x100000 || Offset > 0xfffff) {
>>> + Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s
>>> failed to relax GOT based symbol reference - image is too big (>1 MiB).",
>>> + mInImageName);
>>> + break;
>>> + }
>>> + break;
>>> +
>>> case R_AARCH64_LD64_GOT_LO12_NC:
>>> //
>>> // Convert into an ADD instruction - see
>>> R_AARCH64_ADR_GOT_PAGE below.
>>> @@ -1686,6 +1719,8 @@ WriteRelocations64 (
>>> case R_AARCH64_LDST128_ABS_LO12_NC:
>>> case R_AARCH64_ADR_GOT_PAGE:
>>> case R_AARCH64_LD64_GOT_LO12_NC:
>>> + case R_AARCH64_LD64_GOTOFF_LO15:
>>> + case R_AARCH64_LD64_GOTPAGE_LO15:
>>> //
>>> // No fixups are required for relative relocations,
>>> provided that
>>> // the relative offsets between sections have been
>>> preserved in
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93418): https://edk2.groups.io/g/devel/message/93418
Mute This Topic: https://groups.io/mt/93162057/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
