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

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Gao, Liming 
Sent: Monday, August 22, 2016 11:09 AM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel ; Zhu, Yonghong 

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

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. MdePkg 
BasePeCoffLib applied this rule to get RelocationsStripped attribute. But, it 
is missing in BaseTools BasePeCoffLib.

Cc: Ard Biesheuvel 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 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.
+Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
 Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.
 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

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

[Ard Biesheuvel]

On 22 August 2016 at 09:07, Gao, Liming  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 

Thanks for fixing this,
Ard.


>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Monday, August 22, 2016 3:00 PM
>> To: Gao, Liming 
>> Cc: edk2-devel-01 ; Zhu, Yonghong
>> 
>> Subject: Re: [Patch] BaseTools PeCoffLib: Fix the issue to get
>> RelocationsStripped from TE image
>>
>> On 22 August 2016 at 05:09, Liming Gao  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 
>> > Cc: Yonghong Zhu 
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Liming Gao 
>> > ---
>> >  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.
>> > +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
>> >  Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.
>> >  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
>> >
___
edk2-devel mailing list
edk2-devel@lists.01.org

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

[Gao, Liming]

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);
  }
  }

Thanks
Liming
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Monday, August 22, 2016 3:00 PM
> To: Gao, Liming 
> Cc: edk2-devel-01 ; Zhu, Yonghong
> 
> Subject: Re: [Patch] BaseTools PeCoffLib: Fix the issue to get
> RelocationsStripped from TE image
> 
> On 22 August 2016 at 05:09, Liming Gao  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 
> > Cc: Yonghong Zhu 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Liming Gao 
> > ---
> >  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.
> > +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
> >  Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.
> >  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
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

[Ard Biesheuvel]

On 22 August 2016 at 05:09, Liming Gao  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 
> Cc: Yonghong Zhu 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  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.
> +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
>  Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.
>  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
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

[Liming Gao]

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. MdePkg
BasePeCoffLib applied this rule to get RelocationsStripped attribute. But,
it is missing in BaseTools BasePeCoffLib.

Cc: Ard Biesheuvel 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 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.
+Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
 Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.
 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

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel