Re: [PATCH v2 2/7] BaseTools-GenFw:Add new x86_64 Elf relocation types for PIC/PIE code

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

Ard,
Pls go ahead to add R_X86_64_PLT32 firstly. Below is my original path. And for the GOTPCREL type ones, I could finish enhance them after I back from vacation next week. Thanks.
https://github.com/shijunjing/edk2/commit/dd8b1c4625cbffc7e149571266e0ae0581bd207d



Steven Shi
Intel\SSG\STO\UEFI Firmware

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


> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, August 04, 2016 4:55 AM
> To: Justen, Jordan L <jordan.l.justen@intel.com>
> Cc: Shi, Steven <steven.shi@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>;
> afish@apple.com; Gao, Liming <liming.gao@intel.com>;
> mischief@offblast.org
> Subject: Re: [edk2] [PATCH v2 2/7] BaseTools-GenFw:Add new x86_64 Elf
> relocation types for PIC/PIE code
> 
> 
> > On 3 aug. 2016, at 22:53, Jordan Justen <jordan.l.justen@intel.com> wrote:
> >
> >> On 2016-08-03 13:47:09, Ard Biesheuvel wrote:
> >>
> >>> On 3 aug. 2016, at 22:13, Jordan Justen <jordan.l.justen@intel.com>
> wrote:
> >>>
> >>> Where does this patch stand? I think Ard was asking for it to be
> >>> split, but also wondering if it was really required...
> >>>
> >>> Some devs are still reporting unsupported relocation errors during the
> >>> build. (mischief on irc, for example.)
> >>
> >> The patch is not correct in its current form. PLT relocations (0x4)
> >> can be handled ok, so we can split that off and merge it. The GOT
> >> based relocation handling needs non-trivial rework, and may
> >> currently create corrupt binaries. Which unhandled reloc types are
> >> being reported?
> >
> > "unsupported ELF EM_X86_64 relocation 0x4"
> >
> > You might join irc if you want to get some more info from mischief
> > about it.
> >
> 
> That's the plt one, which we can fix easily. I'll whip up a patch in the morning
> (11 pm here now)
> 
> 
> >
> >>
> >>>
> >>>> On 2016-08-02 05:00:43, Ard Biesheuvel wrote:
> >>>> On 2 August 2016 at 13:40, Shi, Steven <steven.shi@intel.com> wrote:
> >>>>>>
> >>>>>> CoffAddFixup() must be used for absolute symbol references only.
> These
> >>>>>> instructions contain relative symbol references, which are
> >>>>>> recalculated in WriteSections64().
> >>>>>>
> >>>>>> The only absolute symbol reference is the GOT entry for 'n', and your
> >>>>>> code (in WriteRelocations64()) calculates the address of the GOT
> entry
> >>>>>> (which is always in .text BTW) and adds a fixup for it, i.e.,
> >>>>>>
> >>>>>> +              CoffAddFixup(
> >>>>>> +                (UINT32)(UINTN)((UINT64)
> >>>>>> mCoffSectionsOffset[RelShdr->sh_info] + GoTPcRelPtrOffset),
> >>>>>> +                EFI_IMAGE_REL_BASED_DIR64);
> >>>>>>
> >>>>>> This code adds a fixup to the PE/COFF .reloc section for the GOT
> entry
> >>>>>> containing the address of 'n', and the instructions perform a IP
> >>>>>> relative load of the contents of the GOT entry to retrieve the address
> >>>>>> of 'n'.
> >>>>>>
> >>>>>> By adding two fixups, the PE/COFF loader will apply the load offset
> >>>>>> twice, resulting in an incorrect value.
> >>>>> OK, I get your point now. Yes, the current patch could generate
> multiple fixups for the same GOT relocation entry. How about we introduce a
> simple IsDuplicatedCoffFixup() to check whether a converting fixup offset is
> duplicated before we use CoffAddFixup() to really add it? If it is new, we add
> it, otherwise just skip it.
> >>>>
> >>>> That could work, but you have to be aware that fixups are best emitted
> >>>> in the order they need to be applied in the binary, or it will become
> >>>> very inefficient. (Please refer to the PE/COFF spec section that
> >>>> explains the layout of the .reloc section)
> >>>>
> >>>> What it comes down to is that relocations are grouped by target page,
> >>>> and for every place in the page that requires a relocation to be
> >>>> applied, a 4 bit type is emitted followed by a 12-bit offset, which is
> >>>> the offset into the current page. If you emit fixups for the current
> >>>> instruction, followed by one for the GOT, it will basically take two
> >>>> 'page switches' every time.
> >>>>
> >>>> So it would be better to simply emit the relocations, but introduce a
> >>>> sorting pass that merges all duplicates as well.
> >>>>
> >>>> Thanks,
> >>>> Ard.
> >>>> _______________________________________________
> >>>> edk2-devel mailing list
> >>>> edk2-devel@lists.01.org
> >>>> https://lists.01.org/mailman/listinfo/edk2-devel