* [PATCH v2] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw
[not found] <1917777633.1686290.1528375781778.ref@mail.yahoo.com>
@ 2018-06-07 12:49 ` Zenith432
2018-06-08 4:02 ` Andrew Fish
2018-07-02 9:22 ` Shi, Steven
0 siblings, 2 replies; 4+ messages in thread
From: Zenith432 @ 2018-06-07 12:49 UTC (permalink / raw)
To: edk2-devel@lists.01.org; +Cc: LimingGao
Adds support for the following X64 ELF relocations to GenFw
R_X86_64_GOTPCREL
R_X86_64_GOTPCRELX
R_X86_64_REX_GOTPCRELX
CC: Shi Steven <steven.shi@intel.com>
CC: Yonghong Zhu <yonghong.zhu@intel.com>
CC: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zenith432 <zenith432@users.sourceforge.net>
---
BaseTools/Source/C/GenFw/Elf64Convert.c | 166 +++++++++++++++++++++++-
BaseTools/Source/C/GenFw/elf_common.h | 23 ++++
2 files changed, 188 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index c39bdff0..1518c395 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -94,6 +94,15 @@ STATIC Elf_Ehdr *mEhdr;
STATIC Elf_Shdr *mShdrBase;
STATIC Elf_Phdr *mPhdrBase;
+//
+// GOT information
+//
+STATIC Elf_Shdr *mGOTShdr = NULL;
+STATIC UINT32 mGOTShindex = 0;
+STATIC UINT32 *mGOTCoffEntries = NULL;
+STATIC UINT32 mGOTMaxCoffEntries = 0;
+STATIC UINT32 mGOTNumCoffEntries = 0;
+
//
// Coff information
//
@@ -322,6 +331,117 @@ GetSymName (
return StrtabContents + Sym->st_name;
}
+//
+// Find the ELF section hosting the GOT from an ELF Rva
+// of a single GOT entry. Normally, GOT is placed in
+// ELF .text section, so assume once we find in which
+// section the GOT is, all GOT entries are there, and
+// just verify this.
+//
+STATIC
+VOID
+FindElfGOTSectionFromGOTEntryElfRva (
+ Elf64_Addr GOTEntryElfRva
+ )
+{
+ UINT32 i;
+ if (mGOTShdr != NULL) {
+ if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
+ GOTEntryElfRva < mGOTShdr->sh_addr + mGOTShdr->sh_size)
+ return;
+ Error (NULL, 0, 3000, "Unsupported", "FindElfGOTSectionFromGOTEntryElfRva: GOT entries found in multiple sections.");
+ exit(EXIT_FAILURE);
+ }
+ for (i = 0; i < mEhdr->e_shnum; i++) {
+ Elf_Shdr *shdr = GetShdrByIndex(i);
+ if (GOTEntryElfRva >= shdr->sh_addr &&
+ GOTEntryElfRva < shdr->sh_addr + shdr->sh_size) {
+ mGOTShdr = shdr;
+ mGOTShindex = i;
+ return;
+ }
+ }
+ Error (NULL, 0, 3000, "Invalid", "FindElfGOTSectionFromGOTEntryElfRva: ElfRva 0x%016LX for GOT entry not found in any section.", GOTEntryElfRva);
+ exit(EXIT_FAILURE);
+}
+
+//
+// Stores locations of GOT entries in COFF image.
+// Returns TRUE if GOT entry is new.
+// Simple implementation as number of GOT
+// entries is expected to be low.
+//
+
+STATIC
+BOOLEAN
+AccumulateCoffGOTEntries (
+ UINT32 GOTCoffEntry
+ )
+{
+ UINT32 i;
+ if (mGOTCoffEntries != NULL) {
+ for (i = 0; i < mGOTNumCoffEntries; i++)
+ if (mGOTCoffEntries[i] == GOTCoffEntry)
+ return FALSE;
+ }
+ if (mGOTCoffEntries == NULL) {
+ mGOTCoffEntries = (UINT32*)malloc(5 * sizeof *mGOTCoffEntries);
+ if (mGOTCoffEntries == NULL) {
+ Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+ }
+ assert (mGOTCoffEntries != NULL);
+ mGOTMaxCoffEntries = 5;
+ mGOTNumCoffEntries = 0;
+ } else if (mGOTNumCoffEntries == mGOTMaxCoffEntries) {
+ mGOTCoffEntries = (UINT32*)realloc(mGOTCoffEntries, 2 * mGOTMaxCoffEntries * sizeof *mGOTCoffEntries);
+ if (mGOTCoffEntries == NULL) {
+ Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+ }
+ assert (mGOTCoffEntries != NULL);
+ mGOTMaxCoffEntries += mGOTMaxCoffEntries;
+ }
+ mGOTCoffEntries[mGOTNumCoffEntries++] = GOTCoffEntry;
+ return TRUE;
+}
+
+STATIC
+int
+__comparator (
+ const void* lhs,
+ const void* rhs
+ )
+{
+ if (*(const UINT32*)lhs < *(const UINT32*)rhs)
+ return -1;
+ return *(const UINT32*)lhs > *(const UINT32*)rhs;
+}
+
+STATIC
+VOID
+EmitGOTRelocations (
+ VOID
+ )
+{
+ UINT32 i;
+ if (mGOTCoffEntries == NULL)
+ return;
+ qsort(
+ mGOTCoffEntries,
+ mGOTNumCoffEntries,
+ sizeof *mGOTCoffEntries,
+ __comparator);
+ for (i = 0; i < mGOTNumCoffEntries; i++) {
+ VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X", mGOTCoffEntries[i]);
+ CoffAddFixup(
+ mGOTCoffEntries[i],
+ EFI_IMAGE_REL_BASED_DIR64);
+ }
+ free(mGOTCoffEntries);
+ mGOTCoffEntries = NULL;
+ mGOTMaxCoffEntries = 0;
+ mGOTNumCoffEntries = 0;
+}
+
//
// Elf functions interface implementation
//
@@ -698,7 +818,7 @@ WriteSections64 (
// section that applies to the entire binary, and which will have its section
// index set to #0 (which is a NULL section with the SHF_ALLOC bit cleared).
//
- // In the absence of GOT based relocations (which we currently don't support),
+ // In the absence of GOT based relocations,
// this RELA section will contain redundant R_xxx_RELATIVE relocations, one
// for every R_xxx_xx64 relocation appearing in the per-section RELA sections.
// (i.e., .rela.text and .rela.data)
@@ -780,6 +900,7 @@ WriteSections64 (
// Determine how to handle each relocation type based on the machine type.
//
if (mEhdr->e_machine == EM_X86_64) {
+ Elf64_Addr GOTEntryRva;
switch (ELF_R_TYPE(Rel->r_info)) {
case R_X86_64_NONE:
break;
@@ -834,6 +955,32 @@ WriteSections64 (
- (SecOffset - SecShdr->sh_addr));
VerboseMsg ("Relocation: 0x%08X", *(UINT32 *)Targ);
break;
+ case R_X86_64_GOTPCREL:
+ case R_X86_64_GOTPCRELX:
+ case R_X86_64_REX_GOTPCRELX:
+ VerboseMsg ("R_X86_64_GOTPCREL family");
+ VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
+ (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
+ *(UINT32 *)Targ);
+ GOTEntryRva = Rel->r_offset - Rel->r_addend + *(INT32 *)Targ;
+ FindElfGOTSectionFromGOTEntryElfRva(GOTEntryRva);
+ *(UINT32 *)Targ = (UINT32) (*(UINT32 *)Targ
+ + (mCoffSectionsOffset[mGOTShindex] - mGOTShdr->sh_addr)
+ - (SecOffset - SecShdr->sh_addr));
+ VerboseMsg ("Relocation: 0x%08X", *(UINT32 *)Targ);
+ GOTEntryRva += (mCoffSectionsOffset[mGOTShindex] - mGOTShdr->sh_addr); // ELF Rva -> COFF Rva
+ if (AccumulateCoffGOTEntries((UINT32)GOTEntryRva)) {
+ //
+ // Relocate GOT entry if it's the first time we run into it
+ //
+ Targ = mCoffFile + GOTEntryRva;
+ VerboseMsg ("Offset: 0x%08X, Addend: 0x%016LX",
+ (UINT32)GOTEntryRva,
+ *(UINT64 *)Targ);
+ *(UINT64 *)Targ = *(UINT64 *)Targ - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx];
+ VerboseMsg ("Relocation: 0x%016LX", *(UINT64*)Targ);
+ }
+ break;
default:
Error (NULL, 0, 3000, "Invalid", "%s unsupported ELF EM_X86_64 relocation 0x%x.", mInImageName, (unsigned) ELF_R_TYPE(Rel->r_info));
}
@@ -972,6 +1119,9 @@ WriteRelocations64 (
case R_X86_64_NONE:
case R_X86_64_PC32:
case R_X86_64_PLT32:
+ case R_X86_64_GOTPCREL:
+ case R_X86_64_GOTPCRELX:
+ case R_X86_64_REX_GOTPCRELX:
break;
case R_X86_64_64:
VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X",
@@ -1040,10 +1190,24 @@ WriteRelocations64 (
Error (NULL, 0, 3000, "Not Supported", "This tool does not support relocations for ELF with e_machine %u (processor type).", (unsigned) mEhdr->e_machine);
}
}
+ if (mEhdr->e_machine == EM_X86_64 && RelShdr->sh_info == mGOTShindex) {
+ //
+ // Tack relocations for GOT entries after other relocations for
+ // the section the GOT is in, as it's usually found at the end
+ // of the section.
+ //
+ EmitGOTRelocations();
+ }
}
}
}
+ if (mEhdr->e_machine == EM_X86_64) {
+ //
+ // Just in case GOT is in a section with no other relocations
+ //
+ EmitGOTRelocations();
+ }
//
// Pad by adding empty entries.
//
diff --git a/BaseTools/Source/C/GenFw/elf_common.h b/BaseTools/Source/C/GenFw/elf_common.h
index 766d0e42..50b4e1f2 100644
--- a/BaseTools/Source/C/GenFw/elf_common.h
+++ b/BaseTools/Source/C/GenFw/elf_common.h
@@ -544,6 +544,12 @@ typedef struct {
#define R_386_TLS_DTPMOD32 35 /* GOT entry containing TLS index */
#define R_386_TLS_DTPOFF32 36 /* GOT entry containing TLS offset */
#define R_386_TLS_TPOFF32 37 /* GOT entry of -ve static TLS offset */
+#define R_386_SIZE32 38 /* 32-bit symbol size */
+#define R_386_TLS_GOTDESC 39 /* GOT offset for TLS descriptor. */
+#define R_386_TLS_DESC_CALL 40 /* Marker of call through TLS descriptor for relaxation. */
+#define R_386_TLS_DESC 41 /* TLS descriptor containing pointer to code and to argument, returning the TLS offset for the symbol. */
+#define R_386_IRELATIVE 42 /* Adjust indirectly by program base */
+#define R_386_GOT32X 43 /* Load from 32 bit GOT entry, relaxable. */
/* Null relocation */
#define R_AARCH64_NONE 256 /* No relocation */
@@ -1052,6 +1058,23 @@ typedef struct {
#define R_X86_64_DTPOFF32 21 /* Offset in TLS block */
#define R_X86_64_GOTTPOFF 22 /* PC relative offset to IE GOT entry */
#define R_X86_64_TPOFF32 23 /* Offset in static TLS block */
+#define R_X86_64_PC64 24 /* PC relative 64 bit */
+#define R_X86_64_GOTOFF64 25 /* 64 bit offset to GOT */
+#define R_X86_64_GOTPC32 26 /* 32 bit signed pc relative offset to GOT */
+#define R_X86_64_GOT64 27 /* 64-bit GOT entry offset */
+#define R_X86_64_GOTPCREL64 28 /* 64-bit PC relative offset to GOT entry */
+#define R_X86_64_GOTPC64 29 /* 64-bit PC relative offset to GOT */
+#define R_X86_64_GOTPLT64 30 /* like GOT64, says PLT entry needed */
+#define R_X86_64_PLTOFF64 31 /* 64-bit GOT relative offset to PLT entry */
+#define R_X86_64_SIZE32 32 /* Size of symbol plus 32-bit addend */
+#define R_X86_64_SIZE64 33 /* Size of symbol plus 64-bit addend */
+#define R_X86_64_GOTPC32_TLSDESC 34 /* GOT offset for TLS descriptor. */
+#define R_X86_64_TLSDESC_CALL 35 /* Marker for call through TLS descriptor. */
+#define R_X86_64_TLSDESC 36 /* TLS descriptor. */
+#define R_X86_64_IRELATIVE 37 /* Adjust indirectly by program base */
+#define R_X86_64_RELATIVE64 38 /* 64-bit adjust by program base */
+#define R_X86_64_GOTPCRELX 41 /* Load from 32 bit signed pc relative offset to GOT entry without REX prefix, relaxable. */
+#define R_X86_64_REX_GOTPCRELX 42 /* Load from 32 bit signed pc relative offset to GOT entry with REX prefix, relaxable. */
#endif /* !_SYS_ELF_COMMON_H_ */
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw
2018-06-07 12:49 ` [PATCH v2] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw Zenith432
@ 2018-06-08 4:02 ` Andrew Fish
2018-07-02 9:22 ` Shi, Steven
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Fish @ 2018-06-08 4:02 UTC (permalink / raw)
To: Zenith432; +Cc: edk2-devel@lists.01.org, LimingGao
Zenith,
Stupid question, I'm not trying to derail the code review.....
In a static lib world like EFI, I can't figure out why we need support for GOT PC REL addressing modes? I say that as I would think they would be relocation entries in object files, and then the final link would happen and these relocation types would not be required in the final executable? Seems like the linker, at link time, would know the relative offset for all the static libraries that got linked together (edk2 build) and there is no need for a relocation.
So I want to make sure this is not an issue caused by the wrong linker flags (emitting relocation for dynamic libs that is not supported in EF)? I'll freely admit there could be some circumstances I don't understand that require this support, but I just want to make sure we understand why we need to do it. Or maybe I'm missing something fundamental?
Thanks,
Andrew Fish
> On Jun 7, 2018, at 5:49 AM, Zenith432 <zenith432@users.sourceforge.net> wrote:
>
> Adds support for the following X64 ELF relocations to GenFw
> R_X86_64_GOTPCREL
> R_X86_64_GOTPCRELX
> R_X86_64_REX_GOTPCRELX
>
> CC: Shi Steven <steven.shi@intel.com>
> CC: Yonghong Zhu <yonghong.zhu@intel.com>
> CC: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zenith432 <zenith432@users.sourceforge.net>
> ---
> BaseTools/Source/C/GenFw/Elf64Convert.c | 166 +++++++++++++++++++++++-
> BaseTools/Source/C/GenFw/elf_common.h | 23 ++++
> 2 files changed, 188 insertions(+), 1 deletion(-)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index c39bdff0..1518c395 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -94,6 +94,15 @@ STATIC Elf_Ehdr *mEhdr;
> STATIC Elf_Shdr *mShdrBase;
> STATIC Elf_Phdr *mPhdrBase;
>
> +//
> +// GOT information
> +//
> +STATIC Elf_Shdr *mGOTShdr = NULL;
> +STATIC UINT32 mGOTShindex = 0;
> +STATIC UINT32 *mGOTCoffEntries = NULL;
> +STATIC UINT32 mGOTMaxCoffEntries = 0;
> +STATIC UINT32 mGOTNumCoffEntries = 0;
> +
> //
> // Coff information
> //
> @@ -322,6 +331,117 @@ GetSymName (
> return StrtabContents + Sym->st_name;
> }
>
> +//
> +// Find the ELF section hosting the GOT from an ELF Rva
> +// of a single GOT entry. Normally, GOT is placed in
> +// ELF .text section, so assume once we find in which
> +// section the GOT is, all GOT entries are there, and
> +// just verify this.
> +//
> +STATIC
> +VOID
> +FindElfGOTSectionFromGOTEntryElfRva (
> + Elf64_Addr GOTEntryElfRva
> + )
> +{
> + UINT32 i;
> + if (mGOTShdr != NULL) {
> + if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
> + GOTEntryElfRva < mGOTShdr->sh_addr + mGOTShdr->sh_size)
> + return;
> + Error (NULL, 0, 3000, "Unsupported", "FindElfGOTSectionFromGOTEntryElfRva: GOT entries found in multiple sections.");
> + exit(EXIT_FAILURE);
> + }
> + for (i = 0; i < mEhdr->e_shnum; i++) {
> + Elf_Shdr *shdr = GetShdrByIndex(i);
> + if (GOTEntryElfRva >= shdr->sh_addr &&
> + GOTEntryElfRva < shdr->sh_addr + shdr->sh_size) {
> + mGOTShdr = shdr;
> + mGOTShindex = i;
> + return;
> + }
> + }
> + Error (NULL, 0, 3000, "Invalid", "FindElfGOTSectionFromGOTEntryElfRva: ElfRva 0x%016LX for GOT entry not found in any section.", GOTEntryElfRva);
> + exit(EXIT_FAILURE);
> +}
> +
> +//
> +// Stores locations of GOT entries in COFF image.
> +// Returns TRUE if GOT entry is new.
> +// Simple implementation as number of GOT
> +// entries is expected to be low.
> +//
> +
> +STATIC
> +BOOLEAN
> +AccumulateCoffGOTEntries (
> + UINT32 GOTCoffEntry
> + )
> +{
> + UINT32 i;
> + if (mGOTCoffEntries != NULL) {
> + for (i = 0; i < mGOTNumCoffEntries; i++)
> + if (mGOTCoffEntries[i] == GOTCoffEntry)
> + return FALSE;
> + }
> + if (mGOTCoffEntries == NULL) {
> + mGOTCoffEntries = (UINT32*)malloc(5 * sizeof *mGOTCoffEntries);
> + if (mGOTCoffEntries == NULL) {
> + Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
> + }
> + assert (mGOTCoffEntries != NULL);
> + mGOTMaxCoffEntries = 5;
> + mGOTNumCoffEntries = 0;
> + } else if (mGOTNumCoffEntries == mGOTMaxCoffEntries) {
> + mGOTCoffEntries = (UINT32*)realloc(mGOTCoffEntries, 2 * mGOTMaxCoffEntries * sizeof *mGOTCoffEntries);
> + if (mGOTCoffEntries == NULL) {
> + Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
> + }
> + assert (mGOTCoffEntries != NULL);
> + mGOTMaxCoffEntries += mGOTMaxCoffEntries;
> + }
> + mGOTCoffEntries[mGOTNumCoffEntries++] = GOTCoffEntry;
> + return TRUE;
> +}
> +
> +STATIC
> +int
> +__comparator (
> + const void* lhs,
> + const void* rhs
> + )
> +{
> + if (*(const UINT32*)lhs < *(const UINT32*)rhs)
> + return -1;
> + return *(const UINT32*)lhs > *(const UINT32*)rhs;
> +}
> +
> +STATIC
> +VOID
> +EmitGOTRelocations (
> + VOID
> + )
> +{
> + UINT32 i;
> + if (mGOTCoffEntries == NULL)
> + return;
> + qsort(
> + mGOTCoffEntries,
> + mGOTNumCoffEntries,
> + sizeof *mGOTCoffEntries,
> + __comparator);
> + for (i = 0; i < mGOTNumCoffEntries; i++) {
> + VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X", mGOTCoffEntries[i]);
> + CoffAddFixup(
> + mGOTCoffEntries[i],
> + EFI_IMAGE_REL_BASED_DIR64);
> + }
> + free(mGOTCoffEntries);
> + mGOTCoffEntries = NULL;
> + mGOTMaxCoffEntries = 0;
> + mGOTNumCoffEntries = 0;
> +}
> +
> //
> // Elf functions interface implementation
> //
> @@ -698,7 +818,7 @@ WriteSections64 (
> // section that applies to the entire binary, and which will have its section
> // index set to #0 (which is a NULL section with the SHF_ALLOC bit cleared).
> //
> - // In the absence of GOT based relocations (which we currently don't support),
> + // In the absence of GOT based relocations,
> // this RELA section will contain redundant R_xxx_RELATIVE relocations, one
> // for every R_xxx_xx64 relocation appearing in the per-section RELA sections.
> // (i.e., .rela.text and .rela.data)
> @@ -780,6 +900,7 @@ WriteSections64 (
> // Determine how to handle each relocation type based on the machine type.
> //
> if (mEhdr->e_machine == EM_X86_64) {
> + Elf64_Addr GOTEntryRva;
> switch (ELF_R_TYPE(Rel->r_info)) {
> case R_X86_64_NONE:
> break;
> @@ -834,6 +955,32 @@ WriteSections64 (
> - (SecOffset - SecShdr->sh_addr));
> VerboseMsg ("Relocation: 0x%08X", *(UINT32 *)Targ);
> break;
> + case R_X86_64_GOTPCREL:
> + case R_X86_64_GOTPCRELX:
> + case R_X86_64_REX_GOTPCRELX:
> + VerboseMsg ("R_X86_64_GOTPCREL family");
> + VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
> + (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
> + *(UINT32 *)Targ);
> + GOTEntryRva = Rel->r_offset - Rel->r_addend + *(INT32 *)Targ;
> + FindElfGOTSectionFromGOTEntryElfRva(GOTEntryRva);
> + *(UINT32 *)Targ = (UINT32) (*(UINT32 *)Targ
> + + (mCoffSectionsOffset[mGOTShindex] - mGOTShdr->sh_addr)
> + - (SecOffset - SecShdr->sh_addr));
> + VerboseMsg ("Relocation: 0x%08X", *(UINT32 *)Targ);
> + GOTEntryRva += (mCoffSectionsOffset[mGOTShindex] - mGOTShdr->sh_addr); // ELF Rva -> COFF Rva
> + if (AccumulateCoffGOTEntries((UINT32)GOTEntryRva)) {
> + //
> + // Relocate GOT entry if it's the first time we run into it
> + //
> + Targ = mCoffFile + GOTEntryRva;
> + VerboseMsg ("Offset: 0x%08X, Addend: 0x%016LX",
> + (UINT32)GOTEntryRva,
> + *(UINT64 *)Targ);
> + *(UINT64 *)Targ = *(UINT64 *)Targ - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx];
> + VerboseMsg ("Relocation: 0x%016LX", *(UINT64*)Targ);
> + }
> + break;
> default:
> Error (NULL, 0, 3000, "Invalid", "%s unsupported ELF EM_X86_64 relocation 0x%x.", mInImageName, (unsigned) ELF_R_TYPE(Rel->r_info));
> }
> @@ -972,6 +1119,9 @@ WriteRelocations64 (
> case R_X86_64_NONE:
> case R_X86_64_PC32:
> case R_X86_64_PLT32:
> + case R_X86_64_GOTPCREL:
> + case R_X86_64_GOTPCRELX:
> + case R_X86_64_REX_GOTPCRELX:
> break;
> case R_X86_64_64:
> VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X",
> @@ -1040,10 +1190,24 @@ WriteRelocations64 (
> Error (NULL, 0, 3000, "Not Supported", "This tool does not support relocations for ELF with e_machine %u (processor type).", (unsigned) mEhdr->e_machine);
> }
> }
> + if (mEhdr->e_machine == EM_X86_64 && RelShdr->sh_info == mGOTShindex) {
> + //
> + // Tack relocations for GOT entries after other relocations for
> + // the section the GOT is in, as it's usually found at the end
> + // of the section.
> + //
> + EmitGOTRelocations();
> + }
> }
> }
> }
>
> + if (mEhdr->e_machine == EM_X86_64) {
> + //
> + // Just in case GOT is in a section with no other relocations
> + //
> + EmitGOTRelocations();
> + }
> //
> // Pad by adding empty entries.
> //
> diff --git a/BaseTools/Source/C/GenFw/elf_common.h b/BaseTools/Source/C/GenFw/elf_common.h
> index 766d0e42..50b4e1f2 100644
> --- a/BaseTools/Source/C/GenFw/elf_common.h
> +++ b/BaseTools/Source/C/GenFw/elf_common.h
> @@ -544,6 +544,12 @@ typedef struct {
> #define R_386_TLS_DTPMOD32 35 /* GOT entry containing TLS index */
> #define R_386_TLS_DTPOFF32 36 /* GOT entry containing TLS offset */
> #define R_386_TLS_TPOFF32 37 /* GOT entry of -ve static TLS offset */
> +#define R_386_SIZE32 38 /* 32-bit symbol size */
> +#define R_386_TLS_GOTDESC 39 /* GOT offset for TLS descriptor. */
> +#define R_386_TLS_DESC_CALL 40 /* Marker of call through TLS descriptor for relaxation. */
> +#define R_386_TLS_DESC 41 /* TLS descriptor containing pointer to code and to argument, returning the TLS offset for the symbol. */
> +#define R_386_IRELATIVE 42 /* Adjust indirectly by program base */
> +#define R_386_GOT32X 43 /* Load from 32 bit GOT entry, relaxable. */
>
> /* Null relocation */
> #define R_AARCH64_NONE 256 /* No relocation */
> @@ -1052,6 +1058,23 @@ typedef struct {
> #define R_X86_64_DTPOFF32 21 /* Offset in TLS block */
> #define R_X86_64_GOTTPOFF 22 /* PC relative offset to IE GOT entry */
> #define R_X86_64_TPOFF32 23 /* Offset in static TLS block */
> +#define R_X86_64_PC64 24 /* PC relative 64 bit */
> +#define R_X86_64_GOTOFF64 25 /* 64 bit offset to GOT */
> +#define R_X86_64_GOTPC32 26 /* 32 bit signed pc relative offset to GOT */
> +#define R_X86_64_GOT64 27 /* 64-bit GOT entry offset */
> +#define R_X86_64_GOTPCREL64 28 /* 64-bit PC relative offset to GOT entry */
> +#define R_X86_64_GOTPC64 29 /* 64-bit PC relative offset to GOT */
> +#define R_X86_64_GOTPLT64 30 /* like GOT64, says PLT entry needed */
> +#define R_X86_64_PLTOFF64 31 /* 64-bit GOT relative offset to PLT entry */
> +#define R_X86_64_SIZE32 32 /* Size of symbol plus 32-bit addend */
> +#define R_X86_64_SIZE64 33 /* Size of symbol plus 64-bit addend */
> +#define R_X86_64_GOTPC32_TLSDESC 34 /* GOT offset for TLS descriptor. */
> +#define R_X86_64_TLSDESC_CALL 35 /* Marker for call through TLS descriptor. */
> +#define R_X86_64_TLSDESC 36 /* TLS descriptor. */
> +#define R_X86_64_IRELATIVE 37 /* Adjust indirectly by program base */
> +#define R_X86_64_RELATIVE64 38 /* 64-bit adjust by program base */
> +#define R_X86_64_GOTPCRELX 41 /* Load from 32 bit signed pc relative offset to GOT entry without REX prefix, relaxable. */
> +#define R_X86_64_REX_GOTPCRELX 42 /* Load from 32 bit signed pc relative offset to GOT entry with REX prefix, relaxable. */
>
>
> #endif /* !_SYS_ELF_COMMON_H_ */
> --
> 2.17.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw
[not found] <1761571963.2244772.1528455976855.ref@mail.yahoo.com>
@ 2018-06-08 11:06 ` Zenith432
0 siblings, 0 replies; 4+ messages in thread
From: Zenith432 @ 2018-06-08 11:06 UTC (permalink / raw)
To: Andrew Fish; +Cc: edk2-devel@lists.01.org, LimingGao
It's a byproduct of aggressive GCC optimization for size -Os
GCC's code generator loads extern symbol addresses from the GOT when they produce smaller code.
1) When passing a pointer to an extern function as a non-register argument to another function it uses pushq instruction
pushq function@GOTPCREL(%rip)
This is shorter code than computing the address of function in register using leaq instruction, then pushing register. It also saves the use of a register.
There are 5 instances of this construct in MdeModulePkg when building OvmgPkgX64.dsc, but I don't have my notes to list them here.
2) When doing pointer arithmetic on extern symbols using addq, subq, cmpq with operand stored in the GOT.
This code in
MdeModulePkg/Core/Pei/PeiMain/PeiMain.c - ShadowPeiCore
return (PEICORE_FUNCTION_POINTER)((UINTN) EntryPoint + (UINTN) PeiCore - (UINTN) _ModuleEntryPoint);
emits a subq from the GOT entry for _ModuleEntryPoint.
The way things stand today is that when doing non-LTO GCC build,
#pragma GCC push visibility("hidden")
tells the compiler not to assume the existence of GOT entries for extern symbols and eliminates all GOT loads.
For the LTO build, it was found that this breaks the build. So the pragma is active only for non-LTO build.
For LTO build it was found that all above instances of GOT loads in MdeModulePkg disappear due to different code generation selected by LTO.
However, LTO build (without the pragma) can still emit GOT loads to carry out pointer arithmetic done on extern symbols. I have sample of this which is not in EDK2 codebase, and I hope to make a standalone test case of it.
My reasoning for adding this capability to GenFw is similar to the reasoning here
https://bugzilla.tianocore.org/show_bug.cgi?id=955
which is that if the latter build stage (GenFw) can handle anything GCC + binutils-ld throw it then it's not necessary to find the right compiler tweak to suppress the generation of unhandled product anymore.
As a side note, I tested that my patch to GenFw works right on all the GOTPCRELs emitted in build of OvmfPkgX64.dsc.
--------------------------------------------
On Fri, 6/8/18, Andrew Fish <afish@apple.com> wrote:
Subject: Re: [edk2] [PATCH v2] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw
To: "Zenith432" <zenith432@users.sourceforge.net>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>, "LimingGao" <liming.gao@intel.com>
Date: Friday, June 8, 2018, 7:02 AM
Zenith,
Stupid question, I'm not trying to
derail the code review.....
In a static lib world like EFI, I can't
figure out why we need support for GOT PC REL addressing
modes? I say that as I would think they would be relocation
entries in object files, and then the final link would
happen and these relocation types would not be required in
the final executable? Seems like the linker, at link time,
would know the relative offset for all the static libraries
that got linked together (edk2 build) and there is no need
for a relocation.
So I want to make sure this is not an
issue caused by the wrong linker flags (emitting relocation
for dynamic libs that is not supported in EF)? I'll freely
admit there could be some circumstances I don't understand
that require this support, but I just want to make sure we
understand why we need to do it. Or maybe I'm missing
something fundamental?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw
2018-06-07 12:49 ` [PATCH v2] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw Zenith432
2018-06-08 4:02 ` Andrew Fish
@ 2018-07-02 9:22 ` Shi, Steven
1 sibling, 0 replies; 4+ messages in thread
From: Shi, Steven @ 2018-07-02 9:22 UTC (permalink / raw)
To: Zenith432, edk2-devel@lists.01.org; +Cc: Gao, Liming
Hi Zenith,
I like your patch. Sorry for the late response. I see your patch has no impact to current supported relocation types, and I also hope we can add more relocation types in our GenFw tool.
Below are some code style issues in your patch:
> +STATIC
> +VOID
> +FindElfGOTSectionFromGOTEntryElfRva (
> + Elf64_Addr GOTEntryElfRva
> + )
> +{
> + UINT32 i;
> + if (mGOTShdr != NULL) {
> + if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
> + GOTEntryElfRva < mGOTShdr->sh_addr + mGOTShdr->sh_size)
> + return;
We usually use the {} to brace the if branch statements, even it has only one statement, like this:
if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
GOTEntryElfRva < mGOTShdr->sh_addr + mGOTShdr->sh_size){
return;
}
> +STATIC
> +BOOLEAN
> +AccumulateCoffGOTEntries (
> + UINT32 GOTCoffEntry
> + )
> +{
> + UINT32 i;
> + if (mGOTCoffEntries != NULL) {
> + for (i = 0; i < mGOTNumCoffEntries; i++)
> + if (mGOTCoffEntries[i] == GOTCoffEntry)
> + return FALSE;
Same issue as above, we usually always use {} to include if true branch statement like this:
if (mGOTCoffEntries[i] == GOTCoffEntry){
return FALSE;
}
> +STATIC
> +int
> +__comparator (
> + const void* lhs,
> + const void* rhs
> + )
> +{
> + if (*(const UINT32*)lhs < *(const UINT32*)rhs)
> + return -1;
> + return *(const UINT32*)lhs > *(const UINT32*)rhs;
> +}
We usually don't use '__' as prefix for a function, could you enhance the comparator() name and add {} to include if branch statement?
> +STATIC
> +VOID
> +EmitGOTRelocations (
> + VOID
> + )
> +{
> + UINT32 i;
> + if (mGOTCoffEntries == NULL)
> + return;
Please add {} to include if branch statement
> + if (mEhdr->e_machine == EM_X86_64 && RelShdr->sh_info ==
> mGOTShindex) {
> + //
> + // Tack relocations for GOT entries after other relocations for
> + // the section the GOT is in, as it's usually found at the end
> + // of the section.
> + //
> + EmitGOTRelocations();
> + }
> }
> }
> }
>
> + if (mEhdr->e_machine == EM_X86_64) {
> + //
> + // Just in case GOT is in a section with no other relocations
> + //
> + EmitGOTRelocations();
> + }
Could we combine the two EmitGOTRelocations() invokes and only keep the second one?
Thanks
Steven
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-07-02 9:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1917777633.1686290.1528375781778.ref@mail.yahoo.com>
2018-06-07 12:49 ` [PATCH v2] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw Zenith432
2018-06-08 4:02 ` Andrew Fish
2018-07-02 9:22 ` Shi, Steven
[not found] <1761571963.2244772.1528455976855.ref@mail.yahoo.com>
2018-06-08 11:06 ` Zenith432
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox