* [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references @ 2022-08-21 14:16 Ard Biesheuvel 2022-08-21 14:33 ` [edk2-devel] " Rebecca Cran 0 siblings, 1 reply; 6+ messages in thread From: Ard Biesheuvel @ 2022-08-21 14:16 UTC (permalink / raw) To: devel Cc: Ard Biesheuvel, Rebecca Cran, Bob Feng, Liming Gao, Kinney, Michael D, Leif Lindholm 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 <rebecca@bsdio.com> Cc: Bob Feng <bob.c.feng@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com> Cc: Leif Lindholm <quic_llindhol@quicinc.com> Signed-off-by: Ard Biesheuvel <ardb@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 -- 2.35.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references 2022-08-21 14:16 [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references Ard Biesheuvel @ 2022-08-21 14:33 ` Rebecca Cran 2022-09-06 10:30 ` Ard Biesheuvel 0 siblings, 1 reply; 6+ messages in thread From: Rebecca Cran @ 2022-08-21 14:33 UTC (permalink / raw) To: devel, ardb; +Cc: Bob Feng, Liming Gao, Kinney, Michael D, Leif Lindholm Tested-by: Rebecca Cran <rebecca@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 <rebecca@bsdio.com> > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: "Kinney, Michael D" <michael.d.kinney@intel.com> > Cc: Leif Lindholm <quic_llindhol@quicinc.com> > Signed-off-by: Ard Biesheuvel <ardb@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references 2022-08-21 14:33 ` [edk2-devel] " Rebecca Cran @ 2022-09-06 10:30 ` Ard Biesheuvel 2022-09-06 11:53 ` Leif Lindholm 0 siblings, 1 reply; 6+ messages in thread From: Ard Biesheuvel @ 2022-09-06 10:30 UTC (permalink / raw) To: Rebecca Cran Cc: devel, Bob Feng, Liming Gao, Kinney, Michael D, Leif Lindholm Bob, Liming, Leif: any thoughts? On Sun, 21 Aug 2022 at 16:33, Rebecca Cran <rebecca@bsdio.com> wrote: > > Tested-by: Rebecca Cran <rebecca@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 <rebecca@bsdio.com> > > Cc: Bob Feng <bob.c.feng@intel.com> > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Cc: "Kinney, Michael D" <michael.d.kinney@intel.com> > > Cc: Leif Lindholm <quic_llindhol@quicinc.com> > > Signed-off-by: Ard Biesheuvel <ardb@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references 2022-09-06 10:30 ` Ard Biesheuvel @ 2022-09-06 11:53 ` Leif Lindholm 2022-09-07 13:04 ` Bob Feng 0 siblings, 1 reply; 6+ messages in thread From: Leif Lindholm @ 2022-09-06 11:53 UTC (permalink / raw) To: Ard Biesheuvel, Rebecca Cran Cc: devel, Bob Feng, Liming Gao, Kinney, Michael D 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_llindhol@quicinc.com> > On Sun, 21 Aug 2022 at 16:33, Rebecca Cran <rebecca@bsdio.com> wrote: >> >> Tested-by: Rebecca Cran <rebecca@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 <rebecca@bsdio.com> >>> Cc: Bob Feng <bob.c.feng@intel.com> >>> Cc: Liming Gao <gaoliming@byosoft.com.cn> >>> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com> >>> Cc: Leif Lindholm <quic_llindhol@quicinc.com> >>> Signed-off-by: Ard Biesheuvel <ardb@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references 2022-09-06 11:53 ` Leif Lindholm @ 2022-09-07 13:04 ` Bob Feng 2022-09-10 6:59 ` Ard Biesheuvel 0 siblings, 1 reply; 6+ messages in thread From: Bob Feng @ 2022-09-07 13:04 UTC (permalink / raw) To: Leif Lindholm, Ard Biesheuvel, Rebecca Cran Cc: devel@edk2.groups.io, Gao, Liming, Kinney, Michael D Acked-by: Bob Feng <bob.c.feng@intel.com> -----Original Message----- From: Leif Lindholm <quic_llindhol@quicinc.com> Sent: Tuesday, September 6, 2022 7:54 PM To: Ard Biesheuvel <ardb@kernel.org>; Rebecca Cran <rebecca@bsdio.com> Cc: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@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_llindhol@quicinc.com> > On Sun, 21 Aug 2022 at 16:33, Rebecca Cran <rebecca@bsdio.com> wrote: >> >> Tested-by: Rebecca Cran <rebecca@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 <rebecca@bsdio.com> >>> Cc: Bob Feng <bob.c.feng@intel.com> >>> Cc: Liming Gao <gaoliming@byosoft.com.cn> >>> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com> >>> Cc: Leif Lindholm <quic_llindhol@quicinc.com> >>> Signed-off-by: Ard Biesheuvel <ardb@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references 2022-09-07 13:04 ` Bob Feng @ 2022-09-10 6:59 ` Ard Biesheuvel 0 siblings, 0 replies; 6+ messages in thread From: Ard Biesheuvel @ 2022-09-10 6:59 UTC (permalink / raw) To: Feng, Bob C Cc: Leif Lindholm, Rebecca Cran, devel@edk2.groups.io, Gao, Liming, Kinney, Michael D On Wed, 7 Sept 2022 at 14:04, Feng, Bob C <bob.c.feng@intel.com> wrote: > > Acked-by: Bob Feng <bob.c.feng@intel.com> > Merged as #3322 Thanks all, > -----Original Message----- > From: Leif Lindholm <quic_llindhol@quicinc.com> > Sent: Tuesday, September 6, 2022 7:54 PM > To: Ard Biesheuvel <ardb@kernel.org>; Rebecca Cran <rebecca@bsdio.com> > Cc: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@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_llindhol@quicinc.com> > > > On Sun, 21 Aug 2022 at 16:33, Rebecca Cran <rebecca@bsdio.com> wrote: > >> > >> Tested-by: Rebecca Cran <rebecca@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 <rebecca@bsdio.com> > >>> Cc: Bob Feng <bob.c.feng@intel.com> > >>> Cc: Liming Gao <gaoliming@byosoft.com.cn> > >>> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com> > >>> Cc: Leif Lindholm <quic_llindhol@quicinc.com> > >>> Signed-off-by: Ard Biesheuvel <ardb@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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-10 6:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-08-21 14:16 [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references Ard Biesheuvel 2022-08-21 14:33 ` [edk2-devel] " Rebecca Cran 2022-09-06 10:30 ` Ard Biesheuvel 2022-09-06 11:53 ` Leif Lindholm 2022-09-07 13:04 ` Bob Feng 2022-09-10 6:59 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox