From 60e85a39fe49071683f3ac5e208f1582511d26bf Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Wed, 5 Jul 2017 19:33:59 +0100 Subject: [PATCH] BaseTools/GenFw: disregard payload in PE debug directory entry size Currently, the PE/COFF conversion routines in GenFw add a so-called NB10 CodeView debug record to the image, and update the associated directory entry in the PE/COFF optional header to contain its relative virtual address (RVA) and size. However, there are two levels of indirection at work here: the actual NB10 CodeView record (which is simply a magic number and some unused data fields followed by the NUL terminated filename) is emitted separately, and a separate descriptor is emitted that identifies the NB10 CodeView record as type EFI_IMAGE_DEBUG_TYPE_CODEVIEW, and records its size. The directory entry in the PE/COFF optional header should refer to this intermediate descriptor's address and size only, but the WriteDebug## () routines in GenFw erroneously record the size of both the descriptor and the NB10 CodeView record. This problem was exposed by commit e4129b0e5897 ("BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain", 2017-06-19), and GenFw now crashes when it attempts to iterate over what it thinks are multiple intermediate descriptors for different kinds of debug data embedded in the image. The error is understandable, given that both are carved out of the same file space allocation, but this is really an implementation detail of GenFw, and is not required. (Note that the intermediate descriptor does not require a RVA and so it does not even need to be inside a section) So omit the size of the NB10 CodeView record from the size recorded in the optional header. Link: https://lists.01.org/pipermail/edk2-devel/2017-July/012162.html Link: https://lists.01.org/pipermail/edk2-devel/2017-July/012181.html Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel Co-debugged-or-whatever-by: Laszlo Ersek Reviewed-by: Laszlo Ersek Tested-by: Laszlo Ersek Reviewed-by: Liming Gao --- BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +- BaseTools/Source/C/GenFw/Elf64Convert.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c index f7b084dc9b..14fe4a2858 100644 --- a/BaseTools/Source/C/GenFw/Elf32Convert.c +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c @@ -1142,7 +1142,7 @@ WriteDebug32 ( NtHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + mNtHdrOffset); DataDir = &NtHdr->Pe32.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG]; DataDir->VirtualAddress = mDebugOffset; - DataDir->Size = Dir->SizeOfData + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); + DataDir->Size = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); } STATIC diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c index 7eed7b92d3..c39bdff063 100644 --- a/BaseTools/Source/C/GenFw/Elf64Convert.c +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c @@ -1095,7 +1095,7 @@ WriteDebug64 ( NtHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + mNtHdrOffset); DataDir = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG]; DataDir->VirtualAddress = mDebugOffset; - DataDir->Size = Dir->SizeOfData + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); + DataDir->Size = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); } STATIC