From: "Shi, Steven" <steven.shi@intel.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
"Gao, Liming" <liming.gao@intel.com>,
"afish@apple.com" <afish@apple.com>,
"Justen, Jordan L" <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 14:09:06 +0000 [thread overview]
Message-ID: <06C8AB66E78EE34A949939824ABE2B3103381245@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <CAKv+Gu8jvW7nnRq71J5MAWkMsow1WuA1UJ3kwq_p1eh5ENc8xg@mail.gmail.com>
> > 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?
[Steven]: remove the unnecessary blank at end of line, which is required by coding style
>
> > *(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
[Steven]: remove the unnecessary blank at end of line, which is required by coding style
>
> > *(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
[Steven]: remove the unnecessary blank at end of line, which is required by coding style
>
> > *(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
[Steven]: remove the unnecessary blank at end of line, which is required by coding style
>
> > *(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.
[Steven]: Do you ask whether it is still necessary to support the GOTPCREL/GOTPCRELX/REX_GOTPCRELX new relocation types? I think they are nice to have now, since new type support has no impact to old ones, we could just leave the code there for reference.
Thanks
Steven
next prev parent reply other threads:[~2016-07-30 14:09 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 ` [PATCH v2 2/7] BaseTools-GenFw:Add new x86_64 Elf relocation types for PIC/PIE code Ard Biesheuvel
2016-07-30 14:09 ` Shi, Steven [this message]
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=06C8AB66E78EE34A949939824ABE2B3103381245@shsmsx102.ccr.corp.intel.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