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

Shi, Steven <steven.shi@...>

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.

Steven Shi
Intel\SSG\STO\UEFI Firmware

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

-----Original Message-----
From: Ard Biesheuvel []
Sent: Thursday, August 04, 2016 4:55 AM
To: Justen, Jordan L <>
Cc: Shi, Steven <>; Kinney, Michael D
<>; edk2-devel-01 <>;; Gao, Liming <>;
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 <> wrote:

On 2016-08-03 13:47:09, Ard Biesheuvel wrote:

On 3 aug. 2016, at 22:13, Jordan Justen <>

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 <> wrote:

CoffAddFixup() must be used for absolute symbol references only.
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
(which is always in .text BTW) and adds a fixup for it, i.e.,

+ CoffAddFixup(
+ (UINT32)(UINTN)((UINT64)
mCoffSectionsOffset[RelShdr->sh_info] + GoTPcRelPtrOffset),

This code adds a fixup to the PE/COFF .reloc section for the GOT
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.

edk2-devel mailing list

Join to automatically receive all group messages.