Bob, Liming, Leif: any thoughts?
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 (#93211): https://edk2.groups.io/g/devel/message/93211
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]
-=-=-=-=-=-=-=-=-=-=-=-
