Re: [Patch] BaseTools PeCoffLib: Fix the issue to get RelocationsStripped from TE image

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 22 August 2016 at 09:07, Gao, Liming <liming.gao@intel.com> wrote:
> Ard:
>   This is done to convert PE/COFF image to TE image. TE image Relocation VirtualAddress will be set to non-zero. You can find the following logic in GenFw.c line 2515. Then, PeCoffLib can restore RELOCS_STRIPPED flag from TE image.
>
>   if (((PeHdr->Pe32.FileHeader.Characteristics & EFI_IMAGE_FILE_RELOCS_STRIPPED) == 0) && \
>     (TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress == 0) && \
>     (TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].Size == 0)) {
>       //
>       // PeImage can be loaded into memory, but it has no relocation section.
>       // Fix TeImage Header to set VA of relocation data directory to not zero, the size is still zero.
>       //
>       if (Optional32 != NULL) {
>         TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress = Optional32->SizeOfImage - sizeof (EFI_IMAGE_BASE_RELOCATION);
>       } else if (Optional64 != NULL) {
>         TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress = Optional64->SizeOfImage - sizeof (EFI_IMAGE_BASE_RELOCATION);
>       }
>   }
>

Apologies. Your patch does work, but I failed to clean BaseTools/
before rebuilding it.

Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thanks for fixing this,
Ard.


>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Monday, August 22, 2016 3:00 PM
>> To: Gao, Liming <liming.gao@intel.com>
>> Cc: edk2-devel-01 <edk2-devel@lists.01.org>; Zhu, Yonghong
>> <yonghong.zhu@intel.com>
>> Subject: Re: [Patch] BaseTools PeCoffLib: Fix the issue to get
>> RelocationsStripped from TE image
>>
>> On 22 August 2016 at 05:09, Liming Gao <liming.gao@intel.com> wrote:
>> > If PE image has no relocation section, and has not set RELOCS_STRIPPED,
>> > after it is converted to TE image, GenFw will set its relocation section
>> > VirtualAddress to non-zero address, and keep Size value as Zero.
>>
>> I cannot find this in the code. Instead, I see
>>
>> Elf64Convert.c-1031-  Dir->Size = mCoffOffset - mRelocOffset;
>> Elf64Convert.c-1032-  if (Dir->Size == 0) {
>> Elf64Convert.c:1033:    // If no relocations, null out the directory
>> entry and don't add the .reloc section
>> Elf64Convert.c-1034-    Dir->VirtualAddress = 0;
>> Elf64Convert.c-1035-    NtHdr->Pe32Plus.FileHeader.NumberOfSections--;
>> Elf64Convert.c-1036-  } else {
>> Elf64Convert.c-1037-    Dir->VirtualAddress = mRelocOffset;
>> Elf64Convert.c:1038:    CreateSectionHeader (".reloc", mRelocOffset,
>> mCoffOffset - mRelocOffset,
>> Elf64Convert.c-1039-            EFI_IMAGE_SCN_CNT_INITIALIZED_DATA
>> Elf64Convert.c-1040-            | EFI_IMAGE_SCN_MEM_DISCARDABLE
>> Elf64Convert.c-1041-            | EFI_IMAGE_SCN_MEM_READ);
>> Elf64Convert.c-1042-  }
>> Elf64Convert.c-1043-}
>>
>> so the virtual address is set to zero.
>>
>> I also tried applying the patch, and it does not solve the issue for me.
>>
>> Thanks,
>> Ard.
>>
>> > MdePkg
>> > BasePeCoffLib applied this rule to get RelocationsStripped attribute. But,
>> > it is missing in BaseTools BasePeCoffLib.
>> >
>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Liming Gao <liming.gao@intel.com>
>> > ---
>> >  BaseTools/Source/C/Common/BasePeCoff.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/BaseTools/Source/C/Common/BasePeCoff.c
>> b/BaseTools/Source/C/Common/BasePeCoff.c
>> > index 9652557..d0cc1af 100644
>> > --- a/BaseTools/Source/C/Common/BasePeCoff.c
>> > +++ b/BaseTools/Source/C/Common/BasePeCoff.c
>> > @@ -2,7 +2,7 @@
>> >
>> >    Functions to get info and load PE/COFF image.
>> >
>> > -Copyright (c) 2004 - 2010, Intel Corporation. All rights reserved.<BR>
>> > +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
>> >  Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
>> >  This program and the accompanying materials
>> >  are licensed and made available under the terms and conditions of the BSD
>> License
>> > @@ -336,7 +336,7 @@ Returns:
>> >    //
>> >    if ((!(ImageContext->IsTeImage)) && ((PeHdr-
>> >Pe32.FileHeader.Characteristics & EFI_IMAGE_FILE_RELOCS_STRIPPED) !=
>> 0)) {
>> >      ImageContext->RelocationsStripped = TRUE;
>> > -  } else if ((ImageContext->IsTeImage) && (TeHdr->DataDirectory[0].Size
>> == 0)) {
>> > +  } else if ((ImageContext->IsTeImage) && (TeHdr->DataDirectory[0].Size
>> == 0) && (TeHdr->DataDirectory[0].VirtualAddress == 0)) {
>> >      ImageContext->RelocationsStripped = TRUE;
>> >    } else {
>> >      ImageContext->RelocationsStripped = FALSE;
>> > --
>> > 2.8.0.windows.1
>> >