* [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
@ 2017-08-07 4:00 Yonghong Zhu
2017-08-07 16:26 ` Andrew Fish
2017-08-14 7:10 ` Gao, Liming
0 siblings, 2 replies; 11+ messages in thread
From: Yonghong Zhu @ 2017-08-07 4:00 UTC (permalink / raw)
To: edk2-devel; +Cc: Liming Gao, Michael D Kinney, Andrew Fish
it is a bug in mtoc setting the size of the debug directory entry to
the size of the .debug section, not the size of the
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
memset() and boom.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Andrew Fish <afish@apple.com>
---
BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 246deb0..af60c92 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2813,10 +2813,11 @@ Returns:
//
// Get Debug, Export and Resource EntryTable RVA address.
// Resource Directory entry need to review.
//
Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
+ Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
@@ -2833,11 +2834,10 @@ Returns:
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
}
}
} else {
- Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
}
@@ -2907,10 +2907,20 @@ Returns:
RsdsEntry->Unknown = 0;
RsdsEntry->Unknown2 = 0;
RsdsEntry->Unknown3 = 0;
RsdsEntry->Unknown4 = 0;
RsdsEntry->Unknown5 = 0;
+ } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
+ // MTOC sets DebugDirectoryEntrySize to size of the .debug section, so fix it.
+ if (!ZeroDebugFlag) {
+ if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ } else {
+ Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ }
+ }
+ break;
}
}
}
}
--
2.6.1.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-07 4:00 [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5 Yonghong Zhu
@ 2017-08-07 16:26 ` Andrew Fish
2017-08-08 1:14 ` Zhu, Yonghong
2017-08-10 10:38 ` Gao, Liming
2017-08-14 7:10 ` Gao, Liming
1 sibling, 2 replies; 11+ messages in thread
From: Andrew Fish @ 2017-08-07 16:26 UTC (permalink / raw)
To: Yonghong Zhu; +Cc: edk2-devel, Liming Gao, Mike Kinney
Should that be:
Contributed-under: TianoCore Contribution Agreement 1.1
I also noticed the PeCoff lib is going to loop and reload the .debug suction due to this mtoc bug, so it would be good to harden that code too.
git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 8d1daba..1e4c67e 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
}
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
@@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
https://bugzilla.tianocore.org/show_bug.cgi?id=663
Contributed-under: TianoCore Contribution Agreement 1.1
Thanks,
Andrew Fish
> On Aug 6, 2017, at 9:00 PM, Yonghong Zhu <yonghong.zhu@intel.com> wrote:
>
> it is a bug in mtoc setting the size of the debug directory entry to
> the size of the .debug section, not the size of the
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
> get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
> memset() and boom.
>
> Cc: Liming Gao <liming.gao@intel.com <mailto:liming.gao@intel.com>>
> Cc: Michael D Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Andrew Fish <afish@apple.com <mailto:afish@apple.com>>
> ---
> BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
> index 246deb0..af60c92 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -2813,10 +2813,11 @@ Returns:
> //
> // Get Debug, Export and Resource EntryTable RVA address.
> // Resource Directory entry need to review.
> //
> Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
> + Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
> if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr + FileHdr->SizeOfOptionalHeader);
> if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
> ExportDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
> @@ -2833,11 +2834,10 @@ Returns:
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
> }
> }
> } else {
> - Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr + FileHdr->SizeOfOptionalHeader);
> if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
> Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
> ExportDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
> }
> @@ -2907,10 +2907,20 @@ Returns:
> RsdsEntry->Unknown = 0;
> RsdsEntry->Unknown2 = 0;
> RsdsEntry->Unknown3 = 0;
> RsdsEntry->Unknown4 = 0;
> RsdsEntry->Unknown5 = 0;
> + } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
> + // MTOC sets DebugDirectoryEntrySize to size of the .debug section, so fix it.
> + if (!ZeroDebugFlag) {
> + if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> + } else {
> + Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> + }
> + }
> + break;
> }
> }
> }
> }
>
> --
> 2.6.1.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-07 16:26 ` Andrew Fish
@ 2017-08-08 1:14 ` Zhu, Yonghong
2017-08-10 10:38 ` Gao, Liming
1 sibling, 0 replies; 11+ messages in thread
From: Zhu, Yonghong @ 2017-08-08 1:14 UTC (permalink / raw)
To: afish@apple.com
Cc: edk2-devel@lists.01.org, Gao, Liming, Kinney, Michael D,
Zhu, Yonghong
When push this GenFw fix, I will update the Agreement to 1.1.
I also help to send out PeCoff lib 's patch.
Best Regards,
Zhu Yonghong
From: afish@apple.com [mailto:afish@apple.com]
Sent: Tuesday, August 8, 2017 12:26 AM
To: Zhu, Yonghong <yonghong.zhu@intel.com>
Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
Should that be:
Contributed-under: TianoCore Contribution Agreement 1.1
I also noticed the PeCoff lib is going to loop and reload the .debug suction due to this mtoc bug, so it would be good to harden that code too.
git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 8d1daba..1e4c67e 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
}
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
@@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
https://bugzilla.tianocore.org/show_bug.cgi?id=663
Contributed-under: TianoCore Contribution Agreement 1.1
Thanks,
Andrew Fish
On Aug 6, 2017, at 9:00 PM, Yonghong Zhu <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com>> wrote:
it is a bug in mtoc setting the size of the debug directory entry to
the size of the .debug section, not the size of the
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
memset() and boom.
Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
---
BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 246deb0..af60c92 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2813,10 +2813,11 @@ Returns:
//
// Get Debug, Export and Resource EntryTable RVA address.
// Resource Directory entry need to review.
//
Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
+ Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
@@ -2833,11 +2834,10 @@ Returns:
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
}
}
} else {
- Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
}
@@ -2907,10 +2907,20 @@ Returns:
RsdsEntry->Unknown = 0;
RsdsEntry->Unknown2 = 0;
RsdsEntry->Unknown3 = 0;
RsdsEntry->Unknown4 = 0;
RsdsEntry->Unknown5 = 0;
+ } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
+ // MTOC sets DebugDirectoryEntrySize to size of the .debug section, so fix it.
+ if (!ZeroDebugFlag) {
+ if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ } else {
+ Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ }
+ }
+ break;
}
}
}
}
--
2.6.1.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-07 16:26 ` Andrew Fish
2017-08-08 1:14 ` Zhu, Yonghong
@ 2017-08-10 10:38 ` Gao, Liming
2017-08-10 16:59 ` Andrew Fish
1 sibling, 1 reply; 11+ messages in thread
From: Gao, Liming @ 2017-08-10 10:38 UTC (permalink / raw)
To: afish@apple.com, Zhu, Yonghong; +Cc: edk2-devel@lists.01.org, Kinney, Michael D
Andrew:
If this is a mtoc bug, I suggest to update GenFw to always correct it in the generated EFI image. If so, the EFI image is always correct. There is no change requirement in PeCoff library in MdePkg.
Thanks
Liming
From: afish@apple.com [mailto:afish@apple.com]
Sent: Tuesday, August 8, 2017 12:26 AM
To: Zhu, Yonghong <yonghong.zhu@intel.com>
Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
Should that be:
Contributed-under: TianoCore Contribution Agreement 1.1
I also noticed the PeCoff lib is going to loop and reload the .debug suction due to this mtoc bug, so it would be good to harden that code too.
git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 8d1daba..1e4c67e 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
}
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
@@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
https://bugzilla.tianocore.org/show_bug.cgi?id=663
Contributed-under: TianoCore Contribution Agreement 1.1
Thanks,
Andrew Fish
On Aug 6, 2017, at 9:00 PM, Yonghong Zhu <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com>> wrote:
it is a bug in mtoc setting the size of the debug directory entry to
the size of the .debug section, not the size of the
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
memset() and boom.
Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
---
BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 246deb0..af60c92 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2813,10 +2813,11 @@ Returns:
//
// Get Debug, Export and Resource EntryTable RVA address.
// Resource Directory entry need to review.
//
Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
+ Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
@@ -2833,11 +2834,10 @@ Returns:
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
}
}
} else {
- Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
}
@@ -2907,10 +2907,20 @@ Returns:
RsdsEntry->Unknown = 0;
RsdsEntry->Unknown2 = 0;
RsdsEntry->Unknown3 = 0;
RsdsEntry->Unknown4 = 0;
RsdsEntry->Unknown5 = 0;
+ } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
+ // MTOC sets DebugDirectoryEntrySize to size of the .debug section, so fix it.
+ if (!ZeroDebugFlag) {
+ if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ } else {
+ Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ }
+ }
+ break;
}
}
}
}
--
2.6.1.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-10 10:38 ` Gao, Liming
@ 2017-08-10 16:59 ` Andrew Fish
2017-08-11 4:48 ` Gao, Liming
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Fish @ 2017-08-10 16:59 UTC (permalink / raw)
To: Gao, Liming; +Cc: Zhu, Yonghong, Mike Kinney, edk2-devel@lists.01.org
> On Aug 10, 2017, at 3:38 AM, Gao, Liming <liming.gao@intel.com> wrote:
>
> Andrew:
> If this is a mtoc bug, I suggest to update GenFw to always correct it in the generated EFI image. If so, the EFI image is always correct. There is no change requirement in PeCoff library in MdePkg.
>
Liming,
EFI supports loading PE/COFF images that are not built at the same time as the platform firmware (UEFI Shell, OS loader), and that is why I added the fix to the PE/COFF library code.
Thanks,
Andrew Fish
> Thanks
> Liming
> From: afish@apple.com <mailto:afish@apple.com> [mailto:afish@apple.com <mailto:afish@apple.com>]
> Sent: Tuesday, August 8, 2017 12:26 AM
> To: Zhu, Yonghong <yonghong.zhu@intel.com <mailto:yonghong.zhu@intel.com>>
> Cc: edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>; Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
>
> Should that be:
> Contributed-under: TianoCore Contribution Agreement 1.1
>
> I also noticed the PeCoff lib is going to loop and reload the .debug suction due to this mtoc bug, so it would be good to harden that code too.
>
> git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> index 8d1daba..1e4c67e 100644
> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> @@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
> }
>
> return RETURN_SUCCESS;
> + } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
> + return RETURN_SUCCESS;
> }
> }
> }
> @@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
> if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
> ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
> return RETURN_SUCCESS;
> + } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
> + return RETURN_SUCCESS;
> }
> }
> }
>
>
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=663
> Contributed-under: TianoCore Contribution Agreement 1.1
>
> Thanks,
>
> Andrew Fish
>
>
> On Aug 6, 2017, at 9:00 PM, Yonghong Zhu <yonghong.zhu@intel.com <mailto:yonghong.zhu@intel.com><mailto:yonghong.zhu@intel.com <mailto:yonghong.zhu@intel.com>>> wrote:
>
> it is a bug in mtoc setting the size of the debug directory entry to
> the size of the .debug section, not the size of the
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
> get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
> memset() and boom.
>
> Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
> Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
> ---
> BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
> index 246deb0..af60c92 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -2813,10 +2813,11 @@ Returns:
> //
> // Get Debug, Export and Resource EntryTable RVA address.
> // Resource Directory entry need to review.
> //
> Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
> + Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
> if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr + FileHdr->SizeOfOptionalHeader);
> if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
> ExportDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
> @@ -2833,11 +2834,10 @@ Returns:
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
> }
> }
> } else {
> - Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr + FileHdr->SizeOfOptionalHeader);
> if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
> Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
> ExportDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
> }
> @@ -2907,10 +2907,20 @@ Returns:
> RsdsEntry->Unknown = 0;
> RsdsEntry->Unknown2 = 0;
> RsdsEntry->Unknown3 = 0;
> RsdsEntry->Unknown4 = 0;
> RsdsEntry->Unknown5 = 0;
> + } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
> + // MTOC sets DebugDirectoryEntrySize to size of the .debug section, so fix it.
> + if (!ZeroDebugFlag) {
> + if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> + } else {
> + Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> + }
> + }
> + break;
> }
> }
> }
> }
>
> --
> 2.6.1.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-10 16:59 ` Andrew Fish
@ 2017-08-11 4:48 ` Gao, Liming
2017-08-11 5:07 ` Andrew Fish
0 siblings, 1 reply; 11+ messages in thread
From: Gao, Liming @ 2017-08-11 4:48 UTC (permalink / raw)
To: afish@apple.com
Cc: Zhu, Yonghong, Kinney, Michael D, edk2-devel@lists.01.org,
Gao, Liming
Andrew:
Edk2 Build system calls GenFw to generate EFI image in build phase. Even if this image is not built into BIOS image, its EFI image will be generated by GenFw. So, only if this EFI image is built from EDK2 project, it can be updated by GenFw tool. You can see this step in build_rule.txt to convert .dll to .efi image.
Thanks
Liming
From: afish@apple.com [mailto:afish@apple.com]
Sent: Friday, August 11, 2017 12:59 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
On Aug 10, 2017, at 3:38 AM, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
Andrew:
If this is a mtoc bug, I suggest to update GenFw to always correct it in the generated EFI image. If so, the EFI image is always correct. There is no change requirement in PeCoff library in MdePkg.
Liming,
EFI supports loading PE/COFF images that are not built at the same time as the platform firmware (UEFI Shell, OS loader), and that is why I added the fix to the PE/COFF library code.
Thanks,
Andrew Fish
Thanks
Liming
From: afish@apple.com<mailto:afish@apple.com> [mailto:afish@apple.com]
Sent: Tuesday, August 8, 2017 12:26 AM
To: Zhu, Yonghong <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com>>
Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
Should that be:
Contributed-under: TianoCore Contribution Agreement 1.1
I also noticed the PeCoff lib is going to loop and reload the .debug suction due to this mtoc bug, so it would be good to harden that code too.
git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 8d1daba..1e4c67e 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
}
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
@@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
https://bugzilla.tianocore.org/show_bug.cgi?id=663
Contributed-under: TianoCore Contribution Agreement 1.1
Thanks,
Andrew Fish
On Aug 6, 2017, at 9:00 PM, Yonghong Zhu <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com><mailto:yonghong.zhu@intel.com>> wrote:
it is a bug in mtoc setting the size of the debug directory entry to
the size of the .debug section, not the size of the
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
memset() and boom.
Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com><mailto:liming.gao@intel.com>>
Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com><mailto:michael.d.kinney@intel.com>>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Andrew Fish <afish@apple.com<mailto:afish@apple.com><mailto:afish@apple.com>>
---
BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 246deb0..af60c92 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2813,10 +2813,11 @@ Returns:
//
// Get Debug, Export and Resource EntryTable RVA address.
// Resource Directory entry need to review.
//
Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
+ Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
@@ -2833,11 +2834,10 @@ Returns:
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
}
}
} else {
- Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
}
@@ -2907,10 +2907,20 @@ Returns:
RsdsEntry->Unknown = 0;
RsdsEntry->Unknown2 = 0;
RsdsEntry->Unknown3 = 0;
RsdsEntry->Unknown4 = 0;
RsdsEntry->Unknown5 = 0;
+ } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
+ // MTOC sets DebugDirectoryEntrySize to size of the .debug section, so fix it.
+ if (!ZeroDebugFlag) {
+ if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ } else {
+ Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ }
+ }
+ break;
}
}
}
}
--
2.6.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-11 4:48 ` Gao, Liming
@ 2017-08-11 5:07 ` Andrew Fish
2017-08-11 5:49 ` Gao, Liming
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Fish @ 2017-08-11 5:07 UTC (permalink / raw)
To: Gao, Liming; +Cc: Mike Kinney, edk2-devel@lists.01.org
> On Aug 10, 2017, at 9:48 PM, Gao, Liming <liming.gao@intel.com> wrote:
>
> Andrew:
> Edk2 Build system calls GenFw to generate EFI image in build phase. Even if this image is not built into BIOS image, its EFI image will be generated by GenFw. So, only if this EFI image is built from EDK2 project, it can be updated by GenFw tool. You can see this step in build_rule.txt to convert .dll to .efi image.
>
Gao,
We can fix the future for edk2, but we can't change the past, or change other build systems. It is not safe to assume that every EFI executable that the DXE Core will see is as new as the firmware, or was even built via the edk2. Option ROMs and OS loaders are good examples of this issue.
Thanks,
Andrew Fish
> Thanks
> Liming
> From: afish@apple.com [mailto:afish@apple.com]
> Sent: Friday, August 11, 2017 12:59 AM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
>
>
> On Aug 10, 2017, at 3:38 AM, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
>
> Andrew:
> If this is a mtoc bug, I suggest to update GenFw to always correct it in the generated EFI image. If so, the EFI image is always correct. There is no change requirement in PeCoff library in MdePkg.
>
>
> Liming,
>
> EFI supports loading PE/COFF images that are not built at the same time as the platform firmware (UEFI Shell, OS loader), and that is why I added the fix to the PE/COFF library code.
>
> Thanks,
>
> Andrew Fish
>
>
> Thanks
> Liming
> From: afish@apple.com<mailto:afish@apple.com> [mailto:afish@apple.com]
> Sent: Tuesday, August 8, 2017 12:26 AM
> To: Zhu, Yonghong <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com>>
> Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
>
> Should that be:
> Contributed-under: TianoCore Contribution Agreement 1.1
>
> I also noticed the PeCoff lib is going to loop and reload the .debug suction due to this mtoc bug, so it would be good to harden that code too.
>
> git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> index 8d1daba..1e4c67e 100644
> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> @@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
> }
>
> return RETURN_SUCCESS;
> + } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
> + return RETURN_SUCCESS;
> }
> }
> }
> @@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
> if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
> ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
> return RETURN_SUCCESS;
> + } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
> + return RETURN_SUCCESS;
> }
> }
> }
>
>
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=663
> Contributed-under: TianoCore Contribution Agreement 1.1
>
> Thanks,
>
> Andrew Fish
>
>
> On Aug 6, 2017, at 9:00 PM, Yonghong Zhu <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com><mailto:yonghong.zhu@intel.com>> wrote:
>
> it is a bug in mtoc setting the size of the debug directory entry to
> the size of the .debug section, not the size of the
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
> get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
> memset() and boom.
>
> Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com><mailto:liming.gao@intel.com>>
> Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com><mailto:michael.d.kinney@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Andrew Fish <afish@apple.com<mailto:afish@apple.com><mailto:afish@apple.com>>
> ---
> BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
> index 246deb0..af60c92 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -2813,10 +2813,11 @@ Returns:
> //
> // Get Debug, Export and Resource EntryTable RVA address.
> // Resource Directory entry need to review.
> //
> Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
> + Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
> if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr + FileHdr->SizeOfOptionalHeader);
> if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
> ExportDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
> @@ -2833,11 +2834,10 @@ Returns:
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
> }
> }
> } else {
> - Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr + FileHdr->SizeOfOptionalHeader);
> if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
> Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
> ExportDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
> }
> @@ -2907,10 +2907,20 @@ Returns:
> RsdsEntry->Unknown = 0;
> RsdsEntry->Unknown2 = 0;
> RsdsEntry->Unknown3 = 0;
> RsdsEntry->Unknown4 = 0;
> RsdsEntry->Unknown5 = 0;
> + } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
> + // MTOC sets DebugDirectoryEntrySize to size of the .debug section, so fix it.
> + if (!ZeroDebugFlag) {
> + if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> + } else {
> + Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> + }
> + }
> + break;
> }
> }
> }
> }
>
> --
> 2.6.1.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-11 5:07 ` Andrew Fish
@ 2017-08-11 5:49 ` Gao, Liming
2017-08-11 16:50 ` Andrew Fish
0 siblings, 1 reply; 11+ messages in thread
From: Gao, Liming @ 2017-08-11 5:49 UTC (permalink / raw)
To: afish@apple.com; +Cc: Kinney, Michael D, edk2-devel@lists.01.org
Andrew:
I get your point. I agree edk2 firmware to support old EFI image. And, I review the change in BasePeCoffLib. But, I think it is unnecessary. Below is the current logic. On the first loop, DebugEntry.Type is EFI_IMAGE_DEBUG_TYPE_CODEVIEW, then will return. It doesn't enter into second loop. For mtoc debug entry, its DebugEntry.Type is also EFI_IMAGE_DEBUG_TYPE_CODEVIEW, its DEBUG_CODEVIEW_ ENTRY Signature is CODEVIEW_SIGNATURE_MTOC. So, it will be handled in this logic.
for (Index = 0; Index < DebugDirectoryEntry->Size; Index += sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)) {
//
// Read next debug directory entry
//
Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
...
//
// From PeCoff spec, when DebugEntry.RVA == 0 means this debug info will not load into memory.
// Here we will always load EFI_IMAGE_DEBUG_TYPE_CODEVIEW type debug info. so need adjust the
// ImageContext->ImageSize when DebugEntry.RVA == 0.
//
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
if (DebugEntry.RVA == 0 && DebugEntry.FileOffset != 0) {
ImageContext->ImageSize += DebugEntry.SizeOfData;
}
return RETURN_SUCCESS;
}
}
Thanks
Liming
>-----Original Message-----
>From: afish@apple.com [mailto:afish@apple.com]
>Sent: Friday, August 11, 2017 1:08 PM
>To: Gao, Liming <liming.gao@intel.com>
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-
>devel@lists.01.org
>Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build
>AppPkg with XCODE5
>
>
>> On Aug 10, 2017, at 9:48 PM, Gao, Liming <liming.gao@intel.com> wrote:
>>
>> Andrew:
>> Edk2 Build system calls GenFw to generate EFI image in build phase. Even if
>this image is not built into BIOS image, its EFI image will be generated by
>GenFw. So, only if this EFI image is built from EDK2 project, it can be updated
>by GenFw tool. You can see this step in build_rule.txt to convert .dll to .efi
>image.
>>
>
>Gao,
>
>We can fix the future for edk2, but we can't change the past, or change other
>build systems. It is not safe to assume that every EFI executable that the DXE
>Core will see is as new as the firmware, or was even built via the edk2. Option
>ROMs and OS loaders are good examples of this issue.
>
>Thanks,
>
>Andrew Fish
>
>> Thanks
>> Liming
>> From: afish@apple.com [mailto:afish@apple.com]
>> Sent: Friday, August 11, 2017 12:59 AM
>> To: Gao, Liming <liming.gao@intel.com>
>> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Kinney, Michael D
><michael.d.kinney@intel.com>; edk2-devel@lists.01.org
>> Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build
>AppPkg with XCODE5
>>
>>
>> On Aug 10, 2017, at 3:38 AM, Gao, Liming
><liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
>>
>> Andrew:
>> If this is a mtoc bug, I suggest to update GenFw to always correct it in the
>generated EFI image. If so, the EFI image is always correct. There is no change
>requirement in PeCoff library in MdePkg.
>>
>>
>> Liming,
>>
>> EFI supports loading PE/COFF images that are not built at the same time as
>the platform firmware (UEFI Shell, OS loader), and that is why I added the fix
>to the PE/COFF library code.
>>
>> Thanks,
>>
>> Andrew Fish
>>
>>
>> Thanks
>> Liming
>> From: afish@apple.com<mailto:afish@apple.com> [mailto:afish@apple.com]
>> Sent: Tuesday, August 8, 2017 12:26 AM
>> To: Zhu, Yonghong
><yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com>>
>> Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Gao, Liming
><liming.gao@intel.com<mailto:liming.gao@intel.com>>; Kinney, Michael D
><michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
>> Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build
>AppPkg with XCODE5
>>
>> Should that be:
>> Contributed-under: TianoCore Contribution Agreement 1.1
>>
>> I also noticed the PeCoff lib is going to loop and reload the .debug suction
>due to this mtoc bug, so it would be good to harden that code too.
>>
>> git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>> index 8d1daba..1e4c67e 100644
>> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>> @@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
>> }
>>
>> return RETURN_SUCCESS;
>> + } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
>> + return RETURN_SUCCESS;
>> }
>> }
>> }
>> @@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
>> if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
>> ImageContext->DebugDirectoryEntryRva = (UINT32)
>(DebugDirectoryEntryRva + Index);
>> return RETURN_SUCCESS;
>> + } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
>> + return RETURN_SUCCESS;
>> }
>> }
>> }
>>
>>
>>
>> https://bugzilla.tianocore.org/show_bug.cgi?id=663
>> Contributed-under: TianoCore Contribution Agreement 1.1
>>
>> Thanks,
>>
>> Andrew Fish
>>
>>
>> On Aug 6, 2017, at 9:00 PM, Yonghong Zhu
><yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com><mailto:yongho
>ng.zhu@intel.com>> wrote:
>>
>> it is a bug in mtoc setting the size of the debug directory entry to
>> the size of the .debug section, not the size of the
>> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
>> get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
>> memset() and boom.
>>
>> Cc: Liming Gao
><liming.gao@intel.com<mailto:liming.gao@intel.com><mailto:liming.gao@int
>el.com>>
>> Cc: Michael D Kinney
><michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com><mailto:m
>ichael.d.kinney@intel.com>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Andrew Fish
><afish@apple.com<mailto:afish@apple.com><mailto:afish@apple.com>>
>> ---
>> BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/BaseTools/Source/C/GenFw/GenFw.c
>b/BaseTools/Source/C/GenFw/GenFw.c
>> index 246deb0..af60c92 100644
>> --- a/BaseTools/Source/C/GenFw/GenFw.c
>> +++ b/BaseTools/Source/C/GenFw/GenFw.c
>> @@ -2813,10 +2813,11 @@ Returns:
>> //
>> // Get Debug, Export and Resource EntryTable RVA address.
>> // Resource Directory entry need to review.
>> //
>> Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr +
>sizeof (EFI_IMAGE_FILE_HEADER));
>> + Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr
>+ sizeof (EFI_IMAGE_FILE_HEADER));
>> if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *)
>Optional32Hdr + FileHdr->SizeOfOptionalHeader);
>> if (Optional32Hdr->NumberOfRvaAndSizes >
>EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
>> Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
>> ExportDirectoryEntryRva = Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
>> @@ -2833,11 +2834,10 @@ Returns:
>> Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
>> Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
>> }
>> }
>> } else {
>> - Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr
>+ sizeof (EFI_IMAGE_FILE_HEADER));
>> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *)
>Optional64Hdr + FileHdr->SizeOfOptionalHeader);
>> if (Optional64Hdr->NumberOfRvaAndSizes >
>EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
>> Optional64Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
>> ExportDirectoryEntryRva = Optional64Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
>> }
>> @@ -2907,10 +2907,20 @@ Returns:
>> RsdsEntry->Unknown = 0;
>> RsdsEntry->Unknown2 = 0;
>> RsdsEntry->Unknown3 = 0;
>> RsdsEntry->Unknown4 = 0;
>> RsdsEntry->Unknown5 = 0;
>> + } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
>> + // MTOC sets DebugDirectoryEntrySize to size of the .debug section,
>so fix it.
>> + if (!ZeroDebugFlag) {
>> + if (Optional32Hdr->Magic ==
>EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof
>(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>> + } else {
>> + Optional64Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof
>(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>> + }
>> + }
>> + break;
>> }
>> }
>> }
>> }
>>
>> --
>> 2.6.1.windows.1
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-11 5:49 ` Gao, Liming
@ 2017-08-11 16:50 ` Andrew Fish
2017-08-14 2:13 ` Gao, Liming
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Fish @ 2017-08-11 16:50 UTC (permalink / raw)
To: Gao, Liming; +Cc: Mike Kinney, edk2-devel@lists.01.org
Liming,
Your right I'm off by one. The signature is in the thing EFI_IMAGE_DEBUG_DIRECTORY_ENTRY points to.
I do have a question. Why does the PE/COFF code have an extra step. The TE images are converted PE/COFF images so would not you need this code in both places? Is there some other assumption in play here?
https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffLib/BasePeCoff.c#L769
//
// From PeCoff spec, when DebugEntry.RVA == 0 means this debug info will not load into memory.
// Here we will always load EFI_IMAGE_DEBUG_TYPE_CODEVIEW type debug info. so need adjust the
// ImageContext->ImageSize when DebugEntry.RVA == 0.
//
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
if (DebugEntry.RVA == 0 && DebugEntry.FileOffset != 0) {
ImageContext->ImageSize += DebugEntry.SizeOfData;
}
return RETURN_SUCCESS;
}
https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffLib/BasePeCoff.c#L862
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
return RETURN_SUCCESS;
}
Thanks,
Andrew Fish
> On Aug 10, 2017, at 10:49 PM, Gao, Liming <liming.gao@intel.com> wrote:
>
> Andrew:
> I get your point. I agree edk2 firmware to support old EFI image. And, I review the change in BasePeCoffLib. But, I think it is unnecessary. Below is the current logic. On the first loop, DebugEntry.Type is EFI_IMAGE_DEBUG_TYPE_CODEVIEW, then will return. It doesn't enter into second loop. For mtoc debug entry, its DebugEntry.Type is also EFI_IMAGE_DEBUG_TYPE_CODEVIEW, its DEBUG_CODEVIEW_ ENTRY Signature is CODEVIEW_SIGNATURE_MTOC. So, it will be handled in this logic.
>
> for (Index = 0; Index < DebugDirectoryEntry->Size; Index += sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)) {
> //
> // Read next debug directory entry
> //
> Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> ...
>
> //
> // From PeCoff spec, when DebugEntry.RVA == 0 means this debug info will not load into memory.
> // Here we will always load EFI_IMAGE_DEBUG_TYPE_CODEVIEW type debug info. so need adjust the
> // ImageContext->ImageSize when DebugEntry.RVA == 0.
> //
> if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
> ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
> if (DebugEntry.RVA == 0 && DebugEntry.FileOffset != 0) {
> ImageContext->ImageSize += DebugEntry.SizeOfData;
> }
>
> return RETURN_SUCCESS;
> }
> }
>
> Thanks
> Liming
>> -----Original Message-----
>> From: afish@apple.com [mailto:afish@apple.com]
>> Sent: Friday, August 11, 2017 1:08 PM
>> To: Gao, Liming <liming.gao@intel.com>
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-
>> devel@lists.01.org
>> Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build
>> AppPkg with XCODE5
>>
>>
>>> On Aug 10, 2017, at 9:48 PM, Gao, Liming <liming.gao@intel.com> wrote:
>>>
>>> Andrew:
>>> Edk2 Build system calls GenFw to generate EFI image in build phase. Even if
>> this image is not built into BIOS image, its EFI image will be generated by
>> GenFw. So, only if this EFI image is built from EDK2 project, it can be updated
>> by GenFw tool. You can see this step in build_rule.txt to convert .dll to .efi
>> image.
>>>
>>
>> Gao,
>>
>> We can fix the future for edk2, but we can't change the past, or change other
>> build systems. It is not safe to assume that every EFI executable that the DXE
>> Core will see is as new as the firmware, or was even built via the edk2. Option
>> ROMs and OS loaders are good examples of this issue.
>>
>> Thanks,
>>
>> Andrew Fish
>>
>>> Thanks
>>> Liming
>>> From: afish@apple.com [mailto:afish@apple.com]
>>> Sent: Friday, August 11, 2017 12:59 AM
>>> To: Gao, Liming <liming.gao@intel.com>
>>> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
>>> Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build
>> AppPkg with XCODE5
>>>
>>>
>>> On Aug 10, 2017, at 3:38 AM, Gao, Liming
>> <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
>>>
>>> Andrew:
>>> If this is a mtoc bug, I suggest to update GenFw to always correct it in the
>> generated EFI image. If so, the EFI image is always correct. There is no change
>> requirement in PeCoff library in MdePkg.
>>>
>>>
>>> Liming,
>>>
>>> EFI supports loading PE/COFF images that are not built at the same time as
>> the platform firmware (UEFI Shell, OS loader), and that is why I added the fix
>> to the PE/COFF library code.
>>>
>>> Thanks,
>>>
>>> Andrew Fish
>>>
>>>
>>> Thanks
>>> Liming
>>> From: afish@apple.com<mailto:afish@apple.com> [mailto:afish@apple.com]
>>> Sent: Tuesday, August 8, 2017 12:26 AM
>>> To: Zhu, Yonghong
>> <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com>>
>>> Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Gao, Liming
>> <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Kinney, Michael D
>> <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
>>> Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build
>> AppPkg with XCODE5
>>>
>>> Should that be:
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>
>>> I also noticed the PeCoff lib is going to loop and reload the .debug suction
>> due to this mtoc bug, so it would be good to harden that code too.
>>>
>>> git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>>> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>> b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>>> index 8d1daba..1e4c67e 100644
>>> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>>> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>>> @@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
>>> }
>>>
>>> return RETURN_SUCCESS;
>>> + } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
>>> + return RETURN_SUCCESS;
>>> }
>>> }
>>> }
>>> @@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
>>> if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
>>> ImageContext->DebugDirectoryEntryRva = (UINT32)
>> (DebugDirectoryEntryRva + Index);
>>> return RETURN_SUCCESS;
>>> + } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
>>> + return RETURN_SUCCESS;
>>> }
>>> }
>>> }
>>>
>>>
>>>
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=663
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>
>>> Thanks,
>>>
>>> Andrew Fish
>>>
>>>
>>> On Aug 6, 2017, at 9:00 PM, Yonghong Zhu
>> <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com><mailto:yongho
>> ng.zhu@intel.com>> wrote:
>>>
>>> it is a bug in mtoc setting the size of the debug directory entry to
>>> the size of the .debug section, not the size of the
>>> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
>>> get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
>>> memset() and boom.
>>>
>>> Cc: Liming Gao
>> <liming.gao@intel.com<mailto:liming.gao@intel.com><mailto:liming.gao@int
>> el.com>>
>>> Cc: Michael D Kinney
>> <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com><mailto:m
>> ichael.d.kinney@intel.com>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Andrew Fish
>> <afish@apple.com<mailto:afish@apple.com><mailto:afish@apple.com>>
>>> ---
>>> BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/BaseTools/Source/C/GenFw/GenFw.c
>> b/BaseTools/Source/C/GenFw/GenFw.c
>>> index 246deb0..af60c92 100644
>>> --- a/BaseTools/Source/C/GenFw/GenFw.c
>>> +++ b/BaseTools/Source/C/GenFw/GenFw.c
>>> @@ -2813,10 +2813,11 @@ Returns:
>>> //
>>> // Get Debug, Export and Resource EntryTable RVA address.
>>> // Resource Directory entry need to review.
>>> //
>>> Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr +
>> sizeof (EFI_IMAGE_FILE_HEADER));
>>> + Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr
>> + sizeof (EFI_IMAGE_FILE_HEADER));
>>> if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>>> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *)
>> Optional32Hdr + FileHdr->SizeOfOptionalHeader);
>>> if (Optional32Hdr->NumberOfRvaAndSizes >
>> EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
>>> Optional32Hdr-
>>> DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
>>> ExportDirectoryEntryRva = Optional32Hdr-
>>> DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
>>> @@ -2833,11 +2834,10 @@ Returns:
>>> Optional32Hdr-
>>> DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
>>> Optional32Hdr-
>>> DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
>>> }
>>> }
>>> } else {
>>> - Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr
>> + sizeof (EFI_IMAGE_FILE_HEADER));
>>> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *)
>> Optional64Hdr + FileHdr->SizeOfOptionalHeader);
>>> if (Optional64Hdr->NumberOfRvaAndSizes >
>> EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
>>> Optional64Hdr-
>>> DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
>>> ExportDirectoryEntryRva = Optional64Hdr-
>>> DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
>>> }
>>> @@ -2907,10 +2907,20 @@ Returns:
>>> RsdsEntry->Unknown = 0;
>>> RsdsEntry->Unknown2 = 0;
>>> RsdsEntry->Unknown3 = 0;
>>> RsdsEntry->Unknown4 = 0;
>>> RsdsEntry->Unknown5 = 0;
>>> + } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
>>> + // MTOC sets DebugDirectoryEntrySize to size of the .debug section,
>> so fix it.
>>> + if (!ZeroDebugFlag) {
>>> + if (Optional32Hdr->Magic ==
>> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>>> + Optional32Hdr-
>>> DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof
>> (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>>> + } else {
>>> + Optional64Hdr-
>>> DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof
>> (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>>> + }
>>> + }
>>> + break;
>>> }
>>> }
>>> }
>>> }
>>>
>>> --
>>> 2.6.1.windows.1
>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-11 16:50 ` Andrew Fish
@ 2017-08-14 2:13 ` Gao, Liming
0 siblings, 0 replies; 11+ messages in thread
From: Gao, Liming @ 2017-08-14 2:13 UTC (permalink / raw)
To: afish@apple.com; +Cc: Kinney, Michael D, edk2-devel@lists.01.org
Andrew:
I have no more detail than the comment. This logic is here since the first version edk2 project. Per the comment, it should handle the one special PE image that was generated in the old compiler. For TE image, its image size is calculated by the RVA plus the VirtualSize of the last section mapped into memory. So, its image size should include the debug info. This is just my analysis. If there is such PE image, I can further verify this logic.
Thanks
Liming
From: afish@apple.com [mailto:afish@apple.com]
Sent: Saturday, August 12, 2017 12:51 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
Liming,
Your right I'm off by one. The signature is in the thing EFI_IMAGE_DEBUG_DIRECTORY_ENTRY points to.
I do have a question. Why does the PE/COFF code have an extra step. The TE images are converted PE/COFF images so would not you need this code in both places? Is there some other assumption in play here?
https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffLib/BasePeCoff.c#L769
//
// From PeCoff spec, when DebugEntry.RVA == 0 means this debug info will not load into memory.
// Here we will always load EFI_IMAGE_DEBUG_TYPE_CODEVIEW type debug info. so need adjust the
// ImageContext->ImageSize when DebugEntry.RVA == 0.
//
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
if (DebugEntry.RVA == 0 && DebugEntry.FileOffset != 0) {
ImageContext->ImageSize += DebugEntry.SizeOfData;
}
return RETURN_SUCCESS;
}
https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffLib/BasePeCoff.c#L862
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
return RETURN_SUCCESS;
}
Thanks,
Andrew Fish
On Aug 10, 2017, at 10:49 PM, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
Andrew:
I get your point. I agree edk2 firmware to support old EFI image. And, I review the change in BasePeCoffLib. But, I think it is unnecessary. Below is the current logic. On the first loop, DebugEntry.Type is EFI_IMAGE_DEBUG_TYPE_CODEVIEW, then will return. It doesn't enter into second loop. For mtoc debug entry, its DebugEntry.Type is also EFI_IMAGE_DEBUG_TYPE_CODEVIEW, its DEBUG_CODEVIEW_ ENTRY Signature is CODEVIEW_SIGNATURE_MTOC. So, it will be handled in this logic.
for (Index = 0; Index < DebugDirectoryEntry->Size; Index += sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)) {
//
// Read next debug directory entry
//
Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
...
//
// From PeCoff spec, when DebugEntry.RVA == 0 means this debug info will not load into memory.
// Here we will always load EFI_IMAGE_DEBUG_TYPE_CODEVIEW type debug info. so need adjust the
// ImageContext->ImageSize when DebugEntry.RVA == 0.
//
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
if (DebugEntry.RVA == 0 && DebugEntry.FileOffset != 0) {
ImageContext->ImageSize += DebugEntry.SizeOfData;
}
return RETURN_SUCCESS;
}
}
Thanks
Liming
-----Original Message-----
From: afish@apple.com<mailto:afish@apple.com> [mailto:afish@apple.com]
Sent: Friday, August 11, 2017 1:08 PM
To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-
devel@lists.01.org<mailto:devel@lists.01.org>
Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build
AppPkg with XCODE5
On Aug 10, 2017, at 9:48 PM, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
Andrew:
Edk2 Build system calls GenFw to generate EFI image in build phase. Even if
this image is not built into BIOS image, its EFI image will be generated by
GenFw. So, only if this EFI image is built from EDK2 project, it can be updated
by GenFw tool. You can see this step in build_rule.txt to convert .dll to .efi
image.
Gao,
We can fix the future for edk2, but we can't change the past, or change other
build systems. It is not safe to assume that every EFI executable that the DXE
Core will see is as new as the firmware, or was even built via the edk2. Option
ROMs and OS loaders are good examples of this issue.
Thanks,
Andrew Fish
Thanks
Liming
From: afish@apple.com<mailto:afish@apple.com> [mailto:afish@apple.com]
Sent: Friday, August 11, 2017 12:59 AM
To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Cc: Zhu, Yonghong <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com>>; Kinney, Michael D
<michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build
AppPkg with XCODE5
On Aug 10, 2017, at 3:38 AM, Gao, Liming
<liming.gao@intel.com<mailto:liming.gao@intel.com<mailto:liming.gao@intel.com%3cmailto:liming.gao@intel.com>>> wrote:
Andrew:
If this is a mtoc bug, I suggest to update GenFw to always correct it in the
generated EFI image. If so, the EFI image is always correct. There is no change
requirement in PeCoff library in MdePkg.
Liming,
EFI supports loading PE/COFF images that are not built at the same time as
the platform firmware (UEFI Shell, OS loader), and that is why I added the fix
to the PE/COFF library code.
Thanks,
Andrew Fish
Thanks
Liming
From: afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>> [mailto:afish@apple.com]
Sent: Tuesday, August 8, 2017 12:26 AM
To: Zhu, Yonghong
<yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com%3cmailto:yonghong.zhu@intel.com>>>
Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>; Gao, Liming
<liming.gao@intel.com<mailto:liming.gao@intel.com<mailto:liming.gao@intel.com%3cmailto:liming.gao@intel.com>>>; Kinney, Michael D
<michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build
AppPkg with XCODE5
Should that be:
Contributed-under: TianoCore Contribution Agreement 1.1
I also noticed the PeCoff lib is going to loop and reload the .debug suction
due to this mtoc bug, so it would be good to harden that code too.
git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 8d1daba..1e4c67e 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
}
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
@@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32)
(DebugDirectoryEntryRva + Index);
return RETURN_SUCCESS;
+ } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+ return RETURN_SUCCESS;
}
}
}
https://bugzilla.tianocore.org/show_bug.cgi?id=663
Contributed-under: TianoCore Contribution Agreement 1.1
Thanks,
Andrew Fish
On Aug 6, 2017, at 9:00 PM, Yonghong Zhu
<yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com%3cmailto:yonghong.zhu@intel.com>><mailto:yongho
ng.zhu@intel.com<mailto:yongho%0bng.zhu@intel.com>>> wrote:
it is a bug in mtoc setting the size of the debug directory entry to
the size of the .debug section, not the size of the
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
memset() and boom.
Cc: Liming Gao
<liming.gao@intel.com<mailto:liming.gao@intel.com<mailto:liming.gao@intel.com%3cmailto:liming.gao@intel.com>><mailto:liming.gao@int
el.com<mailto:liming.gao@int%0bel.com>>>
Cc: Michael D Kinney
<michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>><mailto:m
ichael.d.kinney@intel.com<mailto:m%0bichael.d.kinney@intel.com>>>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Andrew Fish
<afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>><mailto:afish@apple.com>>
---
BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Source/C/GenFw/GenFw.c
b/BaseTools/Source/C/GenFw/GenFw.c
index 246deb0..af60c92 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2813,10 +2813,11 @@ Returns:
//
// Get Debug, Export and Resource EntryTable RVA address.
// Resource Directory entry need to review.
//
Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr +
sizeof (EFI_IMAGE_FILE_HEADER));
+ Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr
+ sizeof (EFI_IMAGE_FILE_HEADER));
if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *)
Optional32Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional32Hdr->NumberOfRvaAndSizes >
EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional32Hdr-
DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional32Hdr-
DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
@@ -2833,11 +2834,10 @@ Returns:
Optional32Hdr-
DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
Optional32Hdr-
DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
}
}
} else {
- Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr
+ sizeof (EFI_IMAGE_FILE_HEADER));
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *)
Optional64Hdr + FileHdr->SizeOfOptionalHeader);
if (Optional64Hdr->NumberOfRvaAndSizes >
EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
Optional64Hdr-
DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
ExportDirectoryEntryRva = Optional64Hdr-
DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
}
@@ -2907,10 +2907,20 @@ Returns:
RsdsEntry->Unknown = 0;
RsdsEntry->Unknown2 = 0;
RsdsEntry->Unknown3 = 0;
RsdsEntry->Unknown4 = 0;
RsdsEntry->Unknown5 = 0;
+ } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
+ // MTOC sets DebugDirectoryEntrySize to size of the .debug section,
so fix it.
+ if (!ZeroDebugFlag) {
+ if (Optional32Hdr->Magic ==
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ Optional32Hdr-
DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof
(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ } else {
+ Optional64Hdr-
DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof
(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ }
+ }
+ break;
}
}
}
}
--
2.6.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
2017-08-07 4:00 [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5 Yonghong Zhu
2017-08-07 16:26 ` Andrew Fish
@ 2017-08-14 7:10 ` Gao, Liming
1 sibling, 0 replies; 11+ messages in thread
From: Gao, Liming @ 2017-08-14 7:10 UTC (permalink / raw)
To: Zhu, Yonghong, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Andrew Fish
Reviewed-by: Liming Gao <liming.gao@intel.com>
>-----Original Message-----
>From: Zhu, Yonghong
>Sent: Monday, August 07, 2017 12:00 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
><michael.d.kinney@intel.com>; Andrew Fish <afish@apple.com>
>Subject: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg
>with XCODE5
>
>it is a bug in mtoc setting the size of the debug directory entry to
>the size of the .debug section, not the size of the
>EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
>get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
>memset() and boom.
>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Andrew Fish <afish@apple.com>
>---
> BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
>diff --git a/BaseTools/Source/C/GenFw/GenFw.c
>b/BaseTools/Source/C/GenFw/GenFw.c
>index 246deb0..af60c92 100644
>--- a/BaseTools/Source/C/GenFw/GenFw.c
>+++ b/BaseTools/Source/C/GenFw/GenFw.c
>@@ -2813,10 +2813,11 @@ Returns:
> //
> // Get Debug, Export and Resource EntryTable RVA address.
> // Resource Directory entry need to review.
> //
> Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr +
>sizeof (EFI_IMAGE_FILE_HEADER));
>+ Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr +
>sizeof (EFI_IMAGE_FILE_HEADER));
> if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *)
>Optional32Hdr + FileHdr->SizeOfOptionalHeader);
> if (Optional32Hdr->NumberOfRvaAndSizes >
>EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
> Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
> ExportDirectoryEntryRva = Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
>@@ -2833,11 +2834,10 @@ Returns:
> Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
> Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
> }
> }
> } else {
>- Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr +
>sizeof (EFI_IMAGE_FILE_HEADER));
> SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *)
>Optional64Hdr + FileHdr->SizeOfOptionalHeader);
> if (Optional64Hdr->NumberOfRvaAndSizes >
>EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
> Optional64Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
> ExportDirectoryEntryRva = Optional64Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
> }
>@@ -2907,10 +2907,20 @@ Returns:
> RsdsEntry->Unknown = 0;
> RsdsEntry->Unknown2 = 0;
> RsdsEntry->Unknown3 = 0;
> RsdsEntry->Unknown4 = 0;
> RsdsEntry->Unknown5 = 0;
>+ } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
>+ // MTOC sets DebugDirectoryEntrySize to size of the .debug section, so
>fix it.
>+ if (!ZeroDebugFlag) {
>+ if (Optional32Hdr->Magic ==
>EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+ Optional32Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof
>(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>+ } else {
>+ Optional64Hdr-
>>DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof
>(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>+ }
>+ }
>+ break;
> }
> }
> }
> }
>
>--
>2.6.1.windows.1
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-08-14 7:08 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-07 4:00 [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5 Yonghong Zhu
2017-08-07 16:26 ` Andrew Fish
2017-08-08 1:14 ` Zhu, Yonghong
2017-08-10 10:38 ` Gao, Liming
2017-08-10 16:59 ` Andrew Fish
2017-08-11 4:48 ` Gao, Liming
2017-08-11 5:07 ` Andrew Fish
2017-08-11 5:49 ` Gao, Liming
2017-08-11 16:50 ` Andrew Fish
2017-08-14 2:13 ` Gao, Liming
2017-08-14 7:10 ` Gao, Liming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox