From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: "Shi, Steven" <steven.shi@intel.com>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
"Gao, Liming" <liming.gao@intel.com>,
"afish@apple.com" <afish@apple.com>,
Jordan Justen <jordan.l.justen@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [PATCH v2 2/7] BaseTools-GenFw:Add new x86_64 Elf relocation types for PIC/PIE code
Date: Sat, 30 Jul 2016 11:25:00 +0200 [thread overview]
Message-ID: <CAKv+Gu8jvW7nnRq71J5MAWkMsow1WuA1UJ3kwq_p1eh5ENc8xg@mail.gmail.com> (raw)
In-Reply-To: <1467967364-11556-3-git-send-email-steven.shi@intel.com>
On 8 July 2016 at 10:42, Shi, Steven <steven.shi@intel.com> wrote:
> Add support to convert new Elf relocation types
> (R_X86_64_PLT32, R_X86_64_GOTPCREL, R_X86_64_GOTPCRELX,
> R_X86_64_REX_GOTPCRELX) to PeCoff, which are required by
> position independent code (PIC) built Elf image.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Steven Shi <steven.shi@intel.com>
> ---
> BaseTools/Source/C/GenFw/Elf64Convert.c | 105 +++++++++++++++++++++++++++++---
> BaseTools/Source/C/GenFw/elf_common.h | 9 ++-
> 2 files changed, 105 insertions(+), 9 deletions(-)
> mode change 100644 => 100755 BaseTools/Source/C/GenFw/elf_common.h
>
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 7c838f3..fc241ab 100755
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -755,6 +755,11 @@ WriteSections64 (
> // Determine how to handle each relocation type based on the machine type.
> //
> if (mEhdr->e_machine == EM_X86_64) {
> + //
> + // See Relocation Types details semantics in Table 4.9 of
> + // SystemV x86_64 abi Draft Version 0.99.8
> + // https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-secure.pdf
> + //
> switch (ELF_R_TYPE(Rel->r_info)) {
> case R_X86_64_NONE:
> break;
> @@ -763,24 +768,24 @@ WriteSections64 (
> // Absolute relocation.
> //
> VerboseMsg ("R_X86_64_64");
> - VerboseMsg ("Offset: 0x%08X, Addend: 0x%016LX",
> - (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
> + VerboseMsg ("Offset: 0x%08X, Addend: 0x%016LX",
> + (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
No change?
> *(UINT64 *)Targ);
> *(UINT64 *)Targ = *(UINT64 *)Targ - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx];
> VerboseMsg ("Relocation: 0x%016LX", *(UINT64*)Targ);
> break;
> case R_X86_64_32:
> VerboseMsg ("R_X86_64_32");
> - VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
> - (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
> + VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
> + (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
and here
> *(UINT32 *)Targ);
> *(UINT32 *)Targ = (UINT32)((UINT64)(*(UINT32 *)Targ) - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx]);
> VerboseMsg ("Relocation: 0x%08X", *(UINT32*)Targ);
> break;
> case R_X86_64_32S:
> VerboseMsg ("R_X86_64_32S");
> - VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
> - (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
> + VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
> + (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
and here
> *(UINT32 *)Targ);
> *(INT32 *)Targ = (INT32)((INT64)(*(INT32 *)Targ) - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx]);
> VerboseMsg ("Relocation: 0x%08X", *(UINT32*)Targ);
> @@ -790,14 +795,45 @@ WriteSections64 (
> // Relative relocation: Symbol - Ip + Addend
> //
> VerboseMsg ("R_X86_64_PC32");
> - VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
> - (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
> + VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
> + (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
and here
> *(UINT32 *)Targ);
> *(UINT32 *)Targ = (UINT32) (*(UINT32 *)Targ
> + (mCoffSectionsOffset[Sym->st_shndx] - SymShdr->sh_addr)
> - (SecOffset - SecShdr->sh_addr));
> VerboseMsg ("Relocation: 0x%08X", *(UINT32 *)Targ);
> break;
> + case R_X86_64_PLT32:
> + //
> + // Relative relocation: L + A - P
> + //
> + VerboseMsg ("R_X86_64_PLT32");
> + VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
> + (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
> + *(UINT32 *)Targ);
> + *(UINT32 *)Targ = (UINT32) (*(UINT32 *)Targ
> + + (mCoffSectionsOffset[Sym->st_shndx] - SymShdr->sh_addr)
> + - (SecOffset - SecShdr->sh_addr));
> + VerboseMsg ("Relocation: 0x%08X", *(UINT32 *)Targ);
> + break;
> + case R_X86_64_GOTPCREL:
> + //
> + // Relative relocation: G + GOT + A - P
> + //
> + VerboseMsg ("R_X86_64_GOTPCREL");
> + break;
> + case R_X86_64_GOTPCRELX:
> + //
> + // Relative relocation: G + GOT + A - P
> + //
> + VerboseMsg ("R_X86_64_GOTPCRELX");
> + break;
> + case R_X86_64_REX_GOTPCRELX:
> + //
> + // Relative relocation: G + GOT + A - P
> + //
> + VerboseMsg ("R_X86_64_REX_GOTPCRELX");
> + break;
> default:
> Error (NULL, 0, 3000, "Invalid", "%s unsupported ELF EM_X86_64 relocation 0x%x.", mInImageName, (unsigned) ELF_R_TYPE(Rel->r_info));
> }
> @@ -887,6 +923,12 @@ WriteRelocations64 (
> UINT32 Index;
> EFI_IMAGE_OPTIONAL_HEADER_UNION *NtHdr;
> EFI_IMAGE_DATA_DIRECTORY *Dir;
> + UINT64 GoTPcRelPtrOffset = 0;
> + UINT64 *RipDisplacementPtr;
> + UINT64 *ElfFileGoTPcRelPtr;
> + UINT64 *CoffFileGoTPcRelPtr;
> + Elf_Shdr *shdr;
> + UINT32 i;
>
> for (Index = 0; Index < mEhdr->e_shnum; Index++) {
> Elf_Shdr *RelShdr = GetShdrByIndex(Index);
> @@ -902,6 +944,53 @@ WriteRelocations64 (
> switch (ELF_R_TYPE(Rel->r_info)) {
> case R_X86_64_NONE:
> case R_X86_64_PC32:
> + case R_X86_64_PLT32:
> + break;
> + case R_X86_64_GOTPCREL:
> + case R_X86_64_GOTPCRELX:
> + case R_X86_64_REX_GOTPCRELX:
> + //
> + // link script force .got and .got.* in .text section, so GoTPcRel pointer must be in .text section
> + // but its value might point to .text or .data section
> + //
> + RipDisplacementPtr = (UINT64 *)((UINT8*)mEhdr + SecShdr->sh_offset + Rel->r_offset - SecShdr->sh_addr);
> + GoTPcRelPtrOffset = Rel->r_offset + 4 + (INT32)(*RipDisplacementPtr) - SecShdr->sh_addr;
> + ElfFileGoTPcRelPtr = (UINT64 *)((UINT8*)mEhdr + SecShdr->sh_offset + GoTPcRelPtrOffset);
> + CoffFileGoTPcRelPtr = (UINT64 *)(mCoffFile + mCoffSectionsOffset[RelShdr->sh_info] + GoTPcRelPtrOffset);
> + //
> + // Check which section the GoTPcRel pointer point to, and calculate Elf to Coff sections displacement accordingly
> + //
> + shdr = NULL;
> + for (i = 0; i < mEhdr->e_shnum; i++) {
> + shdr = GetShdrByIndex(i);
> + if ((*ElfFileGoTPcRelPtr >= shdr->sh_addr) &&
> + (*ElfFileGoTPcRelPtr < shdr->sh_addr + shdr->sh_size)) {
> + break;
> + }
> + }
> + //
> + // Fix the Elf to Coff sections displacement
> + //
> + if(IsDataShdr(shdr)) {
> + *CoffFileGoTPcRelPtr = *CoffFileGoTPcRelPtr + mDataOffset - shdr->sh_addr;
> + }else if (IsTextShdr(SecShdr)){
> + *CoffFileGoTPcRelPtr = *CoffFileGoTPcRelPtr + mTextOffset - shdr->sh_addr;
> + }else {
> + //
> + // link script force to merge .rodata .rodata.*, .got and .got.* in .text section,
> + // not support GoTPcRel point to section other than .data or .text
> + //
> + Error (NULL, 0, 3000, "Invalid", "Not support relocate R_X86_64_GOTPCREL address in section other than .data or .text");
> + assert (FALSE);
> + }
> +
> + VerboseMsg ("R_X86_64_GOTPCREL to EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X",
> + mCoffSectionsOffset[RelShdr->sh_info] + GoTPcRelPtrOffset);
> + CoffAddFixup(
> + (UINT32)(UINTN)((UINT64) mCoffSectionsOffset[RelShdr->sh_info] + GoTPcRelPtrOffset),
> + EFI_IMAGE_REL_BASED_DIR64);
Is this necessary? I would expect the GOT entry itself to be already
covered by a R_X86_64_64 relocation, so I don't think there is a need
to emit a EFI_IMAGE_REL_BASED_DIR64 PE/COFF reloc here.
Thanks,
Ard.
next parent reply other threads:[~2016-07-30 9:25 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1467967364-11556-1-git-send-email-steven.shi@intel.com>
[not found] ` <1467967364-11556-3-git-send-email-steven.shi@intel.com>
2016-07-30 9:25 ` Ard Biesheuvel [this message]
2016-07-30 14:09 ` [PATCH v2 2/7] BaseTools-GenFw:Add new x86_64 Elf relocation types for PIC/PIE code Shi, Steven
2016-07-30 14:11 ` Ard Biesheuvel
2016-07-31 3:08 ` Shi, Steven
2016-07-31 5:42 ` Ard Biesheuvel
2016-07-31 19:10 ` Ard Biesheuvel
2016-08-01 4:39 ` Shi, Steven
2016-08-01 5:58 ` Ard Biesheuvel
2016-08-01 6:13 ` Shi, Steven
2016-08-01 6:43 ` Ard Biesheuvel
2016-08-01 7:19 ` Shi, Steven
2016-08-01 7:25 ` Ard Biesheuvel
2016-08-01 7:54 ` Shi, Steven
2016-08-01 8:00 ` Ard Biesheuvel
2016-08-01 8:28 ` Shi, Steven
[not found] ` <06C8AB66E78EE34A949939824ABE2B31033825EE@shsmsx102.ccr.corp.intel.com>
[not found] ` <CAKv+Gu80u+CJLVtD5tTo5RrJb7LH0Qfnbj=7b7NUqrbf2aOPrA@mail.gmail.com>
[not found] ` <06C8AB66E78EE34A949939824ABE2B31033826FE@shsmsx102.ccr.corp.intel.com>
[not found] ` <CAKv+Gu9MSisR1T_jr=DNyCs24We5=2vUgQZJ9t_rZmCYC8qvHg@mail.gmail.com>
[not found] ` <06C8AB66E78EE34A949939824ABE2B310338275F@shsmsx102.ccr.corp.intel.com>
2016-08-01 10:46 ` Ard Biesheuvel
2016-08-02 11:40 ` Shi, Steven
2016-08-02 12:00 ` Ard Biesheuvel
2016-08-03 20:13 ` Jordan Justen
2016-08-03 20:47 ` Ard Biesheuvel
2016-08-03 20:53 ` Jordan Justen
2016-08-03 20:55 ` Ard Biesheuvel
2016-08-03 23:26 ` Shi, Steven
2016-08-03 20:55 ` Nicolas Owens
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKv+Gu8jvW7nnRq71J5MAWkMsow1WuA1UJ3kwq_p1eh5ENc8xg@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox