Re: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw

["Shi, Steven" <steven.shi@intel.com>]

Hi Zenith,
BTW, besides the build pass, did you try to run a Uefi binary, e.g. a simple shell application, which contain the GOTPCREL relocations? If yes. Please send out your test case code as well. I appreciate if you could contribute your test cases as the patch together. 


Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522


> -----Original Message-----
> From: Shi, Steven
> Sent: Thursday, June 7, 2018 10:18 AM
> To: Shi, Steven <steven.shi@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zenith432 <zenith432@users.sourceforge.net>;
> edk2-devel@lists.01.org
> Subject: RE: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
> GenFw
> 
> Please see more details in
> https://bugzilla.tianocore.org/show_bug.cgi?id=970
> 
> 
> Steven Shi
> Intel\SSG\STO\UEFI Firmware
> 
> Tel: +86 021-61166522
> iNet: 821-6522
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Shi,
> > Steven
> > Sent: Thursday, June 7, 2018 10:16 AM
> > To: Gao, Liming <liming.gao@intel.com>; Zenith432
> > <zenith432@users.sourceforge.net>; edk2-devel@lists.01.org
> > Subject: Re: [edk2] [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support
> > to GenFw
> >
> > Yes. If we disable the '#pragma GCC visibility push (hidden)' in
> > ProcessorBind.h, the GOTPCREL support is mandatory. 3rd party module
> > build might need it. It's more complete to add GOTPCREL support.
> >
> > The hidden #pragma can hide all ELF symbols' visibility, including 'extern'
> > functions, which is to let Linux compilers aware that we don’t need loading
> > dynamic link library in Uefi runing, please not emit the GOTPCREL based
> > relocations in the obj file, and we will ensure the linker can get all extern
> > functions statically solved. The reason of not emitting GOTPCREL based
> > relocations is because it is hard to convert the GOTPCREL based relocations
> > in ELF binary to PE/COFF related relocations, and our current GenFw has
> not
> > supported them. I'm glade if we can add the GOTPCREL support in GenFW
> to
> > totally solve this problem.
> >
> > Steven Shi
> > Intel\SSG\STO\UEFI Firmware
> >
> > Tel: +86 021-61166522
> > iNet: 821-6522
> >
> > > -----Original Message-----
> > > From: Gao, Liming
> > > Sent: Thursday, June 7, 2018 9:32 AM
> > > To: Zenith432 <zenith432@users.sourceforge.net>; edk2-
> > devel@lists.01.org
> > > Cc: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
> > > <yonghong.zhu@intel.com>
> > > Subject: RE: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
> > > GenFw
> > >
> > > What's purpose to support GOTPCREL in GenFw? Could you introduce
> your
> > > usage model?
> > >
> > > > -----Original Message-----
> > > > From: Zenith432 [mailto:zenith432@users.sourceforge.net]
> > > > Sent: Thursday, June 7, 2018 2:01 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
> > > <yonghong.zhu@intel.com>; Gao, Liming <liming.gao@intel.com>
> > > > Subject: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
> > GenFw
> > > >
> > > > 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..d2f9bb46 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 + *(UINT32
> *)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