> On Aug 10, 2017, at 3:38 AM, Gao, Liming <liming....@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: af...@apple.com <mailto:af...@apple.com> [mailto:af...@apple.com > <mailto:af...@apple.com>] > Sent: Tuesday, August 8, 2017 12:26 AM > To: Zhu, Yonghong <yonghong....@intel.com <mailto:yonghong....@intel.com>> > Cc: edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>; Gao, Liming > <liming....@intel.com <mailto:liming....@intel.com>>; Kinney, Michael D > <michael.d.kin...@intel.com <mailto:michael.d.kin...@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....@intel.com > <mailto:yonghong....@intel.com><mailto:yonghong....@intel.com > <mailto:yonghong....@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....@intel.com<mailto:liming....@intel.com>> > Cc: Michael D Kinney > <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Andrew Fish <af...@apple.com<mailto:af...@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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
