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


Ard Biesheuvel
 

On 31 July 2016 at 05:08, Shi, Steven <steven.shi@intel.com> wrote:
OK. Could we do that in a separate patch?
[Steven]: Yes. We could separate it in a new patch.

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.

No. The question is whether it is necessary to emit the
EFI_IMAGE_REL_BASED_DIR64 fixup reloc here. The relocation place is
the instruction, not the GOT entry, and I would expect the GOT entry
to be covered by a R_X86_64_64 relocation already
[Steven]: Yes, it is necessary. Even the R_X86_64_64 relocation need to emit the EFI_IMAGE_REL_BASED_DIR64 fixup reloc like below in the original GenFw code. You can test it like this, remove the below CoffAddFixup() code and build the OVMF with GCC5, you will see the Ovmf boot failure with cpu exception.

case R_X86_64_64:
VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X",
mCoffSectionsOffset[RelShdr->sh_info] + (Rel->r_offset - SecShdr->sh_addr));
CoffAddFixup(
(UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
+ (Rel->r_offset - SecShdr->sh_addr)),
EFI_IMAGE_REL_BASED_DIR64);
That was not my point. With your code, how many
EFI_IMAGE_REL_BASED_DIR64 fixups are added to the .reloc section for
the GOT entry of 'n'?

int n;
int f () { return n; }
int g () { return n; }
int h () { return n; }

Join devel@edk2.groups.io to automatically receive all group messages.