* [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain @ 2017-07-03 5:21 Liming Gao 2017-07-03 5:28 ` Zhu, Yonghong 2017-07-05 12:42 ` Laszlo Ersek 0 siblings, 2 replies; 6+ messages in thread From: Liming Gao @ 2017-07-03 5:21 UTC (permalink / raw) To: edk2-devel https://bugzilla.tianocore.org/show_bug.cgi?id=600 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Liming Gao <liming.gao@intel.com> --- BaseTools/Source/C/GenFw/GenFw.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c index 22e4e72..6569460 100644 --- a/BaseTools/Source/C/GenFw/GenFw.c +++ b/BaseTools/Source/C/GenFw/GenFw.c @@ -2770,6 +2770,7 @@ Returns: { UINT32 Index; UINT32 DebugDirectoryEntryRva; + UINT32 DebugDirectoryEntrySize; UINT32 DebugDirectoryEntryFileOffset; UINT32 ExportDirectoryEntryRva; UINT32 ExportDirectoryEntryFileOffset; @@ -2781,12 +2782,14 @@ Returns: EFI_IMAGE_OPTIONAL_HEADER64 *Optional64Hdr; EFI_IMAGE_SECTION_HEADER *SectionHeader; EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *DebugEntry; + EFI_IMAGE_DEBUG_CODEVIEW_RSDS_ENTRY *RsdsEntry; UINT32 *NewTimeStamp; // // Init variable. // DebugDirectoryEntryRva = 0; + DebugDirectoryEntrySize = 0; ExportDirectoryEntryRva = 0; ResourceDirectoryEntryRva = 0; DebugDirectoryEntryFileOffset = 0; @@ -2822,6 +2825,7 @@ Returns: if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { DebugDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; + DebugDirectoryEntrySize = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; if (ZeroDebugFlag) { Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; @@ -2841,6 +2845,7 @@ Returns: if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { DebugDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; + DebugDirectoryEntrySize = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; if (ZeroDebugFlag) { Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; @@ -2886,11 +2891,23 @@ Returns: if (DebugDirectoryEntryFileOffset != 0) { DebugEntry = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset); - DebugEntry->TimeDateStamp = 0; - mImageTimeStamp = 0; - if (ZeroDebugFlag) { - memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); - memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); + Index = 0; + for (Index=0; Index < DebugDirectoryEntrySize / sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); Index ++, DebugEntry ++) { + DebugEntry->TimeDateStamp = 0; + if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { + memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); + memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); + } + if (DebugEntry->Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { + RsdsEntry = (EFI_IMAGE_DEBUG_CODEVIEW_RSDS_ENTRY *) (FileBuffer + DebugEntry->FileOffset); + if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_RSDS) { + RsdsEntry->Unknown = 0; + RsdsEntry->Unknown2 = 0; + RsdsEntry->Unknown3 = 0; + RsdsEntry->Unknown4 = 0; + RsdsEntry->Unknown5 = 0; + } + } } } -- 2.8.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain 2017-07-03 5:21 [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain Liming Gao @ 2017-07-03 5:28 ` Zhu, Yonghong 2017-07-05 12:42 ` Laszlo Ersek 1 sibling, 0 replies; 6+ messages in thread From: Zhu, Yonghong @ 2017-07-03 5:28 UTC (permalink / raw) To: Gao, Liming, edk2-devel@lists.01.org Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com> Best Regards, Zhu Yonghong -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Liming Gao Sent: Monday, July 03, 2017 1:21 PM To: edk2-devel@lists.01.org Subject: [edk2] [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain https://bugzilla.tianocore.org/show_bug.cgi?id=600 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Liming Gao <liming.gao@intel.com> --- BaseTools/Source/C/GenFw/GenFw.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c index 22e4e72..6569460 100644 --- a/BaseTools/Source/C/GenFw/GenFw.c +++ b/BaseTools/Source/C/GenFw/GenFw.c @@ -2770,6 +2770,7 @@ Returns: { UINT32 Index; UINT32 DebugDirectoryEntryRva; + UINT32 DebugDirectoryEntrySize; UINT32 DebugDirectoryEntryFileOffset; UINT32 ExportDirectoryEntryRva; UINT32 ExportDirectoryEntryFileOffset; @@ -2781,12 +2782,14 @@ Returns: EFI_IMAGE_OPTIONAL_HEADER64 *Optional64Hdr; EFI_IMAGE_SECTION_HEADER *SectionHeader; EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *DebugEntry; + EFI_IMAGE_DEBUG_CODEVIEW_RSDS_ENTRY *RsdsEntry; UINT32 *NewTimeStamp; // // Init variable. // DebugDirectoryEntryRva = 0; + DebugDirectoryEntrySize = 0; ExportDirectoryEntryRva = 0; ResourceDirectoryEntryRva = 0; DebugDirectoryEntryFileOffset = 0; @@ -2822,6 +2825,7 @@ Returns: if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { DebugDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; + DebugDirectoryEntrySize = + Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; if (ZeroDebugFlag) { Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; @@ -2841,6 +2845,7 @@ Returns: if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { DebugDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; + DebugDirectoryEntrySize = + Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; if (ZeroDebugFlag) { Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; @@ -2886,11 +2891,23 @@ Returns: if (DebugDirectoryEntryFileOffset != 0) { DebugEntry = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset); - DebugEntry->TimeDateStamp = 0; - mImageTimeStamp = 0; - if (ZeroDebugFlag) { - memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); - memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); + Index = 0; + for (Index=0; Index < DebugDirectoryEntrySize / sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); Index ++, DebugEntry ++) { + DebugEntry->TimeDateStamp = 0; + if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { + memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); + memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); + } + if (DebugEntry->Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { + RsdsEntry = (EFI_IMAGE_DEBUG_CODEVIEW_RSDS_ENTRY *) (FileBuffer + DebugEntry->FileOffset); + if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_RSDS) { + RsdsEntry->Unknown = 0; + RsdsEntry->Unknown2 = 0; + RsdsEntry->Unknown3 = 0; + RsdsEntry->Unknown4 = 0; + RsdsEntry->Unknown5 = 0; + } + } } } -- 2.8.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain 2017-07-03 5:21 [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain Liming Gao 2017-07-03 5:28 ` Zhu, Yonghong @ 2017-07-05 12:42 ` Laszlo Ersek 2017-07-05 13:29 ` Laszlo Ersek 2017-07-05 13:41 ` Leif Lindholm 1 sibling, 2 replies; 6+ messages in thread From: Laszlo Ersek @ 2017-07-05 12:42 UTC (permalink / raw) To: Liming Gao, edk2-devel; +Cc: Gerd Hoffmann, Ard Biesheuvel Hi Liming, Gerd's Jenkins CI reported a GenFw segfault, with this patch applied. I can reproduce the segfault locally. This is the command line: > GenFw \ > -e DXE_RUNTIME_DRIVER \ > -o .../Build/OvmfX64/NOOPT_GCC48/X64/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe/DEBUG/ReportStatusCodeRouterRuntimeDxe.efi \ > Build/OvmfX64/NOOPT_GCC48/X64/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe/DEBUG/ReportStatusCodeRouterRuntimeDxe.dll Backtrace from gdb: > Program received signal SIGSEGV, Segmentation fault. > 0x00007ffff789c814 in __memset_sse2 () from /lib64/libc.so.6 > Missing separate debuginfos, use: debuginfo-install glibc-2.17-194.el7.x86_64 libuuid-2.23.2-39.el7.x86_64 > (gdb) where > #0 0x00007ffff789c814 in __memset_sse2 () from /lib64/libc.so.6 > #1 0x0000000000407492 in ZeroDebugData (FileBuffer=0x636e50 "MZ", ZeroDebugFlag=0 '\000') at GenFw.c:2898 > #2 0x0000000000406a4d in main (argc=0, argv=0x7fffffffd0b8) at GenFw.c:2591 See below for the SIGSEGV location: On 07/03/17 07:21, Liming Gao wrote: > https://bugzilla.tianocore.org/show_bug.cgi?id=600 > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Liming Gao <liming.gao@intel.com> > --- > BaseTools/Source/C/GenFw/GenFw.c | 27 ++++++++++++++++++++++----- > 1 file changed, 22 insertions(+), 5 deletions(-) > > diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c > index 22e4e72..6569460 100644 > --- a/BaseTools/Source/C/GenFw/GenFw.c > +++ b/BaseTools/Source/C/GenFw/GenFw.c > @@ -2770,6 +2770,7 @@ Returns: > { > UINT32 Index; > UINT32 DebugDirectoryEntryRva; > + UINT32 DebugDirectoryEntrySize; > UINT32 DebugDirectoryEntryFileOffset; > UINT32 ExportDirectoryEntryRva; > UINT32 ExportDirectoryEntryFileOffset; > @@ -2781,12 +2782,14 @@ Returns: > EFI_IMAGE_OPTIONAL_HEADER64 *Optional64Hdr; > EFI_IMAGE_SECTION_HEADER *SectionHeader; > EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *DebugEntry; > + EFI_IMAGE_DEBUG_CODEVIEW_RSDS_ENTRY *RsdsEntry; > UINT32 *NewTimeStamp; > > // > // Init variable. > // > DebugDirectoryEntryRva = 0; > + DebugDirectoryEntrySize = 0; > ExportDirectoryEntryRva = 0; > ResourceDirectoryEntryRva = 0; > DebugDirectoryEntryFileOffset = 0; > @@ -2822,6 +2825,7 @@ Returns: > if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ > Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { > DebugDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; > + DebugDirectoryEntrySize = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; > if (ZeroDebugFlag) { > Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; > Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; > @@ -2841,6 +2845,7 @@ Returns: > if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ > Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { > DebugDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; > + DebugDirectoryEntrySize = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; > if (ZeroDebugFlag) { > Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; > Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; > @@ -2886,11 +2891,23 @@ Returns: > > if (DebugDirectoryEntryFileOffset != 0) { > DebugEntry = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset); > - DebugEntry->TimeDateStamp = 0; > - mImageTimeStamp = 0; > - if (ZeroDebugFlag) { > - memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); > - memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); > + Index = 0; > + for (Index=0; Index < DebugDirectoryEntrySize / sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); Index ++, DebugEntry ++) { > + DebugEntry->TimeDateStamp = 0; > + if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { > + memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); This memset() is the culprit. According to gdb (all values decimal), - Index = 1, - DebugDirectoryEntrySize = 237, - ZeroDebugFlag = 0. These values look suspicious, because sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) is 28, and DebugDirectoryEntrySize (237) is not an integral multiple of that. This is the first EFI_IMAGE_DEBUG_DIRECTORY_ENTRY element: > (gdb) print ((EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset))[0] > $11 = {Characteristics = 0, TimeDateStamp = 0, MajorVersion = 0, MinorVersion = 0, Type = 2, SizeOfData = 209, RVA = 33292, FileOffset = 33292} This is the second one (which triggers the crash, Index=1): > (gdb) print ((EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset))[1] > $12 = {Characteristics = 808534606, TimeDateStamp = 0, MajorVersion = 0, MinorVersion = 0, Type = 0, SizeOfData = 1836017711, RVA = 1634479973, FileOffset = 796094307} The second element is obviously garbage (FileOffset = 796094307, and then the memset() wanders off into the weeds). This is the contents of the DataDirectory: > (gdb) print Optional64Hdr->DataDirectory > $22 = {{VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_EXPORT > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_IMPORT > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_RESOURCE > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_EXCEPTION > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_SECURITY > {VirtualAddress = 36864, Size = 4096}, // EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC > {VirtualAddress = 33264, Size = 237}, // EFI_IMAGE_DIRECTORY_ENTRY_DEBUG > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_COPYRIGHT > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_GLOBALPTR > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_TLS > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG > {VirtualAddress = 0, Size = 0}, > {VirtualAddress = 0, Size = 0}, > {VirtualAddress = 0, Size = 0}, > {VirtualAddress = 0, Size = 0}, > {VirtualAddress = 0, Size = 0}} According to <https://msdn.microsoft.com/en-us/library/ms809762.aspx>, the format (element type) of the debug directory is specific to the toolchain; the article says, > To determine the number of entries in the Microsoft linker-generated > debug directory, divide the size of the debug directory (found in the > size field of the data directory entry) by the size of an > IMAGE_DEBUG_DIRECTORY structure Here's a hexdump of the debug directory (237 bytes starting from (FileBuffer+33264)): 00000000 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00 00 |................| 00000010 d1 00 00 00 0c 82 00 00 0c 82 00 00 4e 42 31 30 |Ñ...........NB10| 00000020 00 00 00 00 00 00 00 00 00 00 00 00 2f 68 6f 6d |............/hom| 00000030 65 2f 6c 61 63 6f 73 2f 73 72 63 2f 75 70 73 74 |e/lacos/src/upst| 00000040 72 65 61 6d 2f 65 64 6b 32 2f 42 75 69 6c 64 2f |ream/edk2/Build/| 00000050 4f 76 6d 66 58 36 34 2f 4e 4f 4f 50 54 5f 47 43 |OvmfX64/NOOPT_GC| 00000060 43 34 38 2f 58 36 34 2f 4d 64 65 4d 6f 64 75 6c |C48/X64/MdeModul| 00000070 65 50 6b 67 2f 55 6e 69 76 65 72 73 61 6c 2f 52 |ePkg/Universal/R| 00000080 65 70 6f 72 74 53 74 61 74 75 73 43 6f 64 65 52 |eportStatusCodeR| 00000090 6f 75 74 65 72 2f 52 75 6e 74 69 6d 65 44 78 65 |outer/RuntimeDxe| 000000a0 2f 52 65 70 6f 72 74 53 74 61 74 75 73 43 6f 64 |/ReportStatusCod| 000000b0 65 52 6f 75 74 65 72 52 75 6e 74 69 6d 65 44 78 |eRouterRuntimeDx| 000000c0 65 2f 44 45 42 55 47 2f 52 65 70 6f 72 74 53 74 |e/DEBUG/ReportSt| 000000d0 61 74 75 73 43 6f 64 65 52 6f 75 74 65 72 52 75 |atusCodeRouterRu| 000000e0 6e 74 69 6d 65 44 78 65 2e 64 6c 6c 00 |ntimeDxe.dll.| The bytes 0x4e 0x42 0x31 0x30 ("NB10"), at offset 28, can be seen above as "Characteristics = 808534606". Googling this value, it seems to be a signature / magic value: CVINFO_PDB20_CVSIGNATURE. Then, VirtualAddress = 33264 is 0x81F0 hex, and "objdump -x" reports: > Sections: > Idx Name Size VMA LMA File off Algn Flags > [...] > 2 .build-id 00000024 00000000000081f0 00000000000081f0 000081f0 2**2 CONTENTS, READONLY > [...] > SYMBOL TABLE: > [...] > 00000000000081f0 l d .build-id 0000000000000000 .build-id > [...] Also, I found a function called pe_bfd_read_buildid() in the GNU Binutils source that works with the PE_DEBUG_DATA entry of the data directory. In "BaseTools/Scripts/GccBase.lds", we have: > /* > * Retain the GNU build id but in a non-allocatable section so GenFw > * does not copy it into the PE/COFF image. > */ > .build-id (INFO) : { *(.note.gnu.build-id) } This is from commit 7fd5d619806d ("BaseTools GCC: drop GNU notes section from EFI image", 2016-07-27). So... I don't really know what's happening here, but the DEBUG entry of the data directory (i.e., the debug directory) doesn't seem to be structured like GenFw expects. > + memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); > + } > + if (DebugEntry->Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { > + RsdsEntry = (EFI_IMAGE_DEBUG_CODEVIEW_RSDS_ENTRY *) (FileBuffer + DebugEntry->FileOffset); > + if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_RSDS) { > + RsdsEntry->Unknown = 0; > + RsdsEntry->Unknown2 = 0; > + RsdsEntry->Unknown3 = 0; > + RsdsEntry->Unknown4 = 0; > + RsdsEntry->Unknown5 = 0; > + } > + } > } > } > > Thanks, Laszlo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain 2017-07-05 12:42 ` Laszlo Ersek @ 2017-07-05 13:29 ` Laszlo Ersek 2017-07-05 15:29 ` Laszlo Ersek 2017-07-05 13:41 ` Leif Lindholm 1 sibling, 1 reply; 6+ messages in thread From: Laszlo Ersek @ 2017-07-05 13:29 UTC (permalink / raw) To: Liming Gao, edk2-devel; +Cc: Ard Biesheuvel On 07/05/17 14:42, Laszlo Ersek wrote: > Hi Liming, > > Gerd's Jenkins CI reported a GenFw segfault, with this patch applied. > > I can reproduce the segfault locally. This is the command line: > >> GenFw \ >> -e DXE_RUNTIME_DRIVER \ >> -o .../Build/OvmfX64/NOOPT_GCC48/X64/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe/DEBUG/ReportStatusCodeRouterRuntimeDxe.efi \ >> Build/OvmfX64/NOOPT_GCC48/X64/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe/DEBUG/ReportStatusCodeRouterRuntimeDxe.dll > > Backtrace from gdb: > >> Program received signal SIGSEGV, Segmentation fault. >> 0x00007ffff789c814 in __memset_sse2 () from /lib64/libc.so.6 >> Missing separate debuginfos, use: debuginfo-install glibc-2.17-194.el7.x86_64 libuuid-2.23.2-39.el7.x86_64 >> (gdb) where >> #0 0x00007ffff789c814 in __memset_sse2 () from /lib64/libc.so.6 >> #1 0x0000000000407492 in ZeroDebugData (FileBuffer=0x636e50 "MZ", ZeroDebugFlag=0 '\000') at GenFw.c:2898 >> #2 0x0000000000406a4d in main (argc=0, argv=0x7fffffffd0b8) at GenFw.c:2591 > > See below for the SIGSEGV location: > > On 07/03/17 07:21, Liming Gao wrote: >> https://bugzilla.tianocore.org/show_bug.cgi?id=600 >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Liming Gao <liming.gao@intel.com> >> --- >> BaseTools/Source/C/GenFw/GenFw.c | 27 ++++++++++++++++++++++----- >> 1 file changed, 22 insertions(+), 5 deletions(-) >> >> diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c >> index 22e4e72..6569460 100644 >> --- a/BaseTools/Source/C/GenFw/GenFw.c >> +++ b/BaseTools/Source/C/GenFw/GenFw.c >> @@ -2770,6 +2770,7 @@ Returns: >> { >> UINT32 Index; >> UINT32 DebugDirectoryEntryRva; >> + UINT32 DebugDirectoryEntrySize; >> UINT32 DebugDirectoryEntryFileOffset; >> UINT32 ExportDirectoryEntryRva; >> UINT32 ExportDirectoryEntryFileOffset; >> @@ -2781,12 +2782,14 @@ Returns: >> EFI_IMAGE_OPTIONAL_HEADER64 *Optional64Hdr; >> EFI_IMAGE_SECTION_HEADER *SectionHeader; >> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *DebugEntry; >> + EFI_IMAGE_DEBUG_CODEVIEW_RSDS_ENTRY *RsdsEntry; >> UINT32 *NewTimeStamp; >> >> // >> // Init variable. >> // >> DebugDirectoryEntryRva = 0; >> + DebugDirectoryEntrySize = 0; >> ExportDirectoryEntryRva = 0; >> ResourceDirectoryEntryRva = 0; >> DebugDirectoryEntryFileOffset = 0; >> @@ -2822,6 +2825,7 @@ Returns: >> if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ >> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { >> DebugDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; >> + DebugDirectoryEntrySize = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; >> if (ZeroDebugFlag) { >> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; >> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; >> @@ -2841,6 +2845,7 @@ Returns: >> if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ >> Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { >> DebugDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; >> + DebugDirectoryEntrySize = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; >> if (ZeroDebugFlag) { >> Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; >> Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; >> @@ -2886,11 +2891,23 @@ Returns: >> >> if (DebugDirectoryEntryFileOffset != 0) { >> DebugEntry = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset); >> - DebugEntry->TimeDateStamp = 0; >> - mImageTimeStamp = 0; >> - if (ZeroDebugFlag) { >> - memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); >> - memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); >> + Index = 0; >> + for (Index=0; Index < DebugDirectoryEntrySize / sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); Index ++, DebugEntry ++) { >> + DebugEntry->TimeDateStamp = 0; >> + if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { >> + memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); > > This memset() is the culprit. > > According to gdb (all values decimal), > - Index = 1, > - DebugDirectoryEntrySize = 237, > - ZeroDebugFlag = 0. > > These values look suspicious, because > sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) is 28, and > DebugDirectoryEntrySize (237) is not an integral multiple of that. > > This is the first EFI_IMAGE_DEBUG_DIRECTORY_ENTRY element: > >> (gdb) print ((EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset))[0] >> $11 = {Characteristics = 0, TimeDateStamp = 0, MajorVersion = 0, MinorVersion = 0, Type = 2, SizeOfData = 209, RVA = 33292, FileOffset = 33292} > > This is the second one (which triggers the crash, Index=1): > >> (gdb) print ((EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset))[1] >> $12 = {Characteristics = 808534606, TimeDateStamp = 0, MajorVersion = 0, MinorVersion = 0, Type = 0, SizeOfData = 1836017711, RVA = 1634479973, FileOffset = 796094307} > > The second element is obviously garbage (FileOffset = 796094307, and > then the memset() wanders off into the weeds). > > This is the contents of the DataDirectory: > >> (gdb) print Optional64Hdr->DataDirectory >> $22 = {{VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_EXPORT >> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_IMPORT >> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_RESOURCE >> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_EXCEPTION >> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_SECURITY >> {VirtualAddress = 36864, Size = 4096}, // EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC >> {VirtualAddress = 33264, Size = 237}, // EFI_IMAGE_DIRECTORY_ENTRY_DEBUG >> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_COPYRIGHT >> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_GLOBALPTR >> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_TLS >> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG >> {VirtualAddress = 0, Size = 0}, >> {VirtualAddress = 0, Size = 0}, >> {VirtualAddress = 0, Size = 0}, >> {VirtualAddress = 0, Size = 0}, >> {VirtualAddress = 0, Size = 0}} > > According to <https://msdn.microsoft.com/en-us/library/ms809762.aspx>, > the format (element type) of the debug directory is specific to the > toolchain; the article says, > >> To determine the number of entries in the Microsoft linker-generated >> debug directory, divide the size of the debug directory (found in the >> size field of the data directory entry) by the size of an >> IMAGE_DEBUG_DIRECTORY structure > > Here's a hexdump of the debug directory (237 bytes starting from > (FileBuffer+33264)): > > 00000000 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00 00 |................| > 00000010 d1 00 00 00 0c 82 00 00 0c 82 00 00 4e 42 31 30 |Ñ...........NB10| > 00000020 00 00 00 00 00 00 00 00 00 00 00 00 2f 68 6f 6d |............/hom| > 00000030 65 2f 6c 61 63 6f 73 2f 73 72 63 2f 75 70 73 74 |e/lacos/src/upst| > 00000040 72 65 61 6d 2f 65 64 6b 32 2f 42 75 69 6c 64 2f |ream/edk2/Build/| > 00000050 4f 76 6d 66 58 36 34 2f 4e 4f 4f 50 54 5f 47 43 |OvmfX64/NOOPT_GC| > 00000060 43 34 38 2f 58 36 34 2f 4d 64 65 4d 6f 64 75 6c |C48/X64/MdeModul| > 00000070 65 50 6b 67 2f 55 6e 69 76 65 72 73 61 6c 2f 52 |ePkg/Universal/R| > 00000080 65 70 6f 72 74 53 74 61 74 75 73 43 6f 64 65 52 |eportStatusCodeR| > 00000090 6f 75 74 65 72 2f 52 75 6e 74 69 6d 65 44 78 65 |outer/RuntimeDxe| > 000000a0 2f 52 65 70 6f 72 74 53 74 61 74 75 73 43 6f 64 |/ReportStatusCod| > 000000b0 65 52 6f 75 74 65 72 52 75 6e 74 69 6d 65 44 78 |eRouterRuntimeDx| > 000000c0 65 2f 44 45 42 55 47 2f 52 65 70 6f 72 74 53 74 |e/DEBUG/ReportSt| > 000000d0 61 74 75 73 43 6f 64 65 52 6f 75 74 65 72 52 75 |atusCodeRouterRu| > 000000e0 6e 74 69 6d 65 44 78 65 2e 64 6c 6c 00 |ntimeDxe.dll.| > > The bytes 0x4e 0x42 0x31 0x30 ("NB10"), at offset 28, can be seen above > as "Characteristics = 808534606". Googling this value, it seems to be a > signature / magic value: CVINFO_PDB20_CVSIGNATURE. > > Then, VirtualAddress = 33264 is 0x81F0 hex, and "objdump -x" reports: > >> Sections: >> Idx Name Size VMA LMA File off Algn Flags >> [...] >> 2 .build-id 00000024 00000000000081f0 00000000000081f0 000081f0 2**2 CONTENTS, READONLY >> [...] >> SYMBOL TABLE: >> [...] >> 00000000000081f0 l d .build-id 0000000000000000 .build-id >> [...] > > Also, I found a function called pe_bfd_read_buildid() in the GNU > Binutils source that works with the PE_DEBUG_DATA entry of the data > directory. > > In "BaseTools/Scripts/GccBase.lds", we have: > >> /* >> * Retain the GNU build id but in a non-allocatable section so GenFw >> * does not copy it into the PE/COFF image. >> */ >> .build-id (INFO) : { *(.note.gnu.build-id) } > > This is from commit 7fd5d619806d ("BaseTools GCC: drop GNU notes section > from EFI image", 2016-07-27). > > So... I don't really know what's happening here, but the DEBUG entry of > the data directory (i.e., the debug directory) doesn't seem to be > structured like GenFw expects. After looking a bit more at the hexdump, it seems that the first entry in the debug directory is indeed of type EFI_IMAGE_DEBUG_DIRECTORY_ENTRY, and its Type field is 2 (EFI_IMAGE_DEBUG_TYPE_CODEVIEW), so GenFw branches to the "RsdsEntry" logic. The second entry however is bogus -- apparently a codeview structure (containing the build-id) is placed into the debug directory? typedef struct _CV_INFO_PDB20 { char CvHeader[4]; // NB10 char Offset[4]; // zeroes char Signature[4]; // zeroes char Age[4]; // zeroes char PdbFileName[]; // filename } CV_INFO_PDB20; This is from GNU Binutils commit 61e2488cd849 ("Add support for generating and inserting build IDs into COFF binaries.", 2014-04-08). What I don't understand is why "objdump -p" does not decode the debug directory for me. :/ Thanks Laszlo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain 2017-07-05 13:29 ` Laszlo Ersek @ 2017-07-05 15:29 ` Laszlo Ersek 0 siblings, 0 replies; 6+ messages in thread From: Laszlo Ersek @ 2017-07-05 15:29 UTC (permalink / raw) To: Liming Gao, edk2-devel Cc: Ard Biesheuvel, Leif Lindholm (Linaro address), Zhu, Yonghong I understand it now. The article at <http://www.debuginfo.com/articles/debuginfomatch.html> was of great help. On 07/05/17 15:29, Laszlo Ersek wrote: > On 07/05/17 14:42, Laszlo Ersek wrote: >> On 07/03/17 07:21, Liming Gao wrote: >>> @@ -2886,11 +2891,23 @@ Returns: >>> >>> if (DebugDirectoryEntryFileOffset != 0) { >>> DebugEntry = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset); >>> - DebugEntry->TimeDateStamp = 0; >>> - mImageTimeStamp = 0; >>> - if (ZeroDebugFlag) { >>> - memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); >>> - memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); >>> + Index = 0; >>> + for (Index=0; Index < DebugDirectoryEntrySize / sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); Index ++, DebugEntry ++) { >>> + DebugEntry->TimeDateStamp = 0; >>> + if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { >>> + memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); >> >> This memset() is the culprit. >> >> According to gdb (all values decimal), >> - Index = 1, >> - DebugDirectoryEntrySize = 237, >> - ZeroDebugFlag = 0. >> >> These values look suspicious, because >> sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) is 28, and >> DebugDirectoryEntrySize (237) is not an integral multiple of that. >> >> This is the first EFI_IMAGE_DEBUG_DIRECTORY_ENTRY element: >> >>> (gdb) print ((EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset))[0] >>> $11 = {Characteristics = 0, TimeDateStamp = 0, MajorVersion = 0, MinorVersion = 0, Type = 2, SizeOfData = 209, RVA = 33292, FileOffset = 33292} Notice this. The first entry (which is valid, with type = EFI_IMAGE_DEBUG_TYPE_CODEVIEW), points at offset 33292, and the pointed-to data (which is a CodeView information block) has size 209. This means that the first byte *past* the CodeView information is at offset 33292 + 209 = 33501. Now, look at the data dictionary again: >>> (gdb) print Optional64Hdr->DataDirectory >>> $22 = {{VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_EXPORT >>> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_IMPORT >>> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_RESOURCE >>> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_EXCEPTION >>> {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_SECURITY >>> {VirtualAddress = 36864, Size = 4096}, // EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC >>> {VirtualAddress = 33264, Size = 237}, // EFI_IMAGE_DIRECTORY_ENTRY_DEBUG Notice this: 33264 + 237 = 33501. The same end offset! And then: 237 - 209 = 28, which is exactly the size of EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. So, what happens is that the GNU linker (or some other bin-util that produces the input file for GenFw) (1) creates a valid EFI_IMAGE_DEBUG_DIRECTORY_ENTRY, of type EFI_IMAGE_DEBUG_TYPE_CODEVIEW, with size = 28, (2) this element points to a valid CV_INFO_PDB20 structure (having NB10 for signature), with size = 209, (3) The CV_INFO_PDB20 structure *immediately* follows the EFI_IMAGE_DEBUG_DIRECTORY_ENTRY element that points to it -- this is all fine, (4) *however*, the pointed-to CV_INFO_PDB20 element (from (3)) is *also* included in the size of the debug directory, even though the size of the debug directory should *only* describe the debug directory entry from (1)! So, this is most certainly a GNU Binutils bug. We can catch it though: as soon as we find an EFI_IMAGE_DEBUG_DIRECTORY_ENTRY whose FileOffset field points into the debug directory itself, we know that the debug directory's size has to be truncated at once to that offset. Let me see if I can write a patch for this. Thanks Laszlo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain 2017-07-05 12:42 ` Laszlo Ersek 2017-07-05 13:29 ` Laszlo Ersek @ 2017-07-05 13:41 ` Leif Lindholm 1 sibling, 0 replies; 6+ messages in thread From: Leif Lindholm @ 2017-07-05 13:41 UTC (permalink / raw) To: Laszlo Ersek Cc: Liming Gao, edk2-devel, Ard Biesheuvel, Kinney, Michael D, Andrew Fish, Yonghong Zhu Many thanks for tracking this down Laszlo - we'd stumbled over it ourselves this morning. Liming, Mike, Andrew: this currently makes the BaseTools unusable with gcc toolchains. Can we revert this commit until this has been resolved? Best Regards, Leif On Wed, Jul 05, 2017 at 02:42:15PM +0200, Laszlo Ersek wrote: > Hi Liming, > > Gerd's Jenkins CI reported a GenFw segfault, with this patch applied. > > I can reproduce the segfault locally. This is the command line: > > > GenFw \ > > -e DXE_RUNTIME_DRIVER \ > > -o .../Build/OvmfX64/NOOPT_GCC48/X64/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe/DEBUG/ReportStatusCodeRouterRuntimeDxe.efi \ > > Build/OvmfX64/NOOPT_GCC48/X64/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe/DEBUG/ReportStatusCodeRouterRuntimeDxe.dll > > Backtrace from gdb: > > > Program received signal SIGSEGV, Segmentation fault. > > 0x00007ffff789c814 in __memset_sse2 () from /lib64/libc.so.6 > > Missing separate debuginfos, use: debuginfo-install glibc-2.17-194.el7.x86_64 libuuid-2.23.2-39.el7.x86_64 > > (gdb) where > > #0 0x00007ffff789c814 in __memset_sse2 () from /lib64/libc.so.6 > > #1 0x0000000000407492 in ZeroDebugData (FileBuffer=0x636e50 "MZ", ZeroDebugFlag=0 '\000') at GenFw.c:2898 > > #2 0x0000000000406a4d in main (argc=0, argv=0x7fffffffd0b8) at GenFw.c:2591 > > See below for the SIGSEGV location: > > On 07/03/17 07:21, Liming Gao wrote: > > https://bugzilla.tianocore.org/show_bug.cgi?id=600 > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Liming Gao <liming.gao@intel.com> > > --- > > BaseTools/Source/C/GenFw/GenFw.c | 27 ++++++++++++++++++++++----- > > 1 file changed, 22 insertions(+), 5 deletions(-) > > > > diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c > > index 22e4e72..6569460 100644 > > --- a/BaseTools/Source/C/GenFw/GenFw.c > > +++ b/BaseTools/Source/C/GenFw/GenFw.c > > @@ -2770,6 +2770,7 @@ Returns: > > { > > UINT32 Index; > > UINT32 DebugDirectoryEntryRva; > > + UINT32 DebugDirectoryEntrySize; > > UINT32 DebugDirectoryEntryFileOffset; > > UINT32 ExportDirectoryEntryRva; > > UINT32 ExportDirectoryEntryFileOffset; > > @@ -2781,12 +2782,14 @@ Returns: > > EFI_IMAGE_OPTIONAL_HEADER64 *Optional64Hdr; > > EFI_IMAGE_SECTION_HEADER *SectionHeader; > > EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *DebugEntry; > > + EFI_IMAGE_DEBUG_CODEVIEW_RSDS_ENTRY *RsdsEntry; > > UINT32 *NewTimeStamp; > > > > // > > // Init variable. > > // > > DebugDirectoryEntryRva = 0; > > + DebugDirectoryEntrySize = 0; > > ExportDirectoryEntryRva = 0; > > ResourceDirectoryEntryRva = 0; > > DebugDirectoryEntryFileOffset = 0; > > @@ -2822,6 +2825,7 @@ Returns: > > if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ > > Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { > > DebugDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; > > + DebugDirectoryEntrySize = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; > > if (ZeroDebugFlag) { > > Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; > > Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; > > @@ -2841,6 +2845,7 @@ Returns: > > if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_DEBUG && \ > > Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size != 0) { > > DebugDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress; > > + DebugDirectoryEntrySize = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size; > > if (ZeroDebugFlag) { > > Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0; > > Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0; > > @@ -2886,11 +2891,23 @@ Returns: > > > > if (DebugDirectoryEntryFileOffset != 0) { > > DebugEntry = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset); > > - DebugEntry->TimeDateStamp = 0; > > - mImageTimeStamp = 0; > > - if (ZeroDebugFlag) { > > - memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); > > - memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); > > + Index = 0; > > + for (Index=0; Index < DebugDirectoryEntrySize / sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); Index ++, DebugEntry ++) { > > + DebugEntry->TimeDateStamp = 0; > > + if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { > > + memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData); > > This memset() is the culprit. > > According to gdb (all values decimal), > - Index = 1, > - DebugDirectoryEntrySize = 237, > - ZeroDebugFlag = 0. > > These values look suspicious, because > sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) is 28, and > DebugDirectoryEntrySize (237) is not an integral multiple of that. > > This is the first EFI_IMAGE_DEBUG_DIRECTORY_ENTRY element: > > > (gdb) print ((EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset))[0] > > $11 = {Characteristics = 0, TimeDateStamp = 0, MajorVersion = 0, MinorVersion = 0, Type = 2, SizeOfData = 209, RVA = 33292, FileOffset = 33292} > > This is the second one (which triggers the crash, Index=1): > > > (gdb) print ((EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset))[1] > > $12 = {Characteristics = 808534606, TimeDateStamp = 0, MajorVersion = 0, MinorVersion = 0, Type = 0, SizeOfData = 1836017711, RVA = 1634479973, FileOffset = 796094307} > > The second element is obviously garbage (FileOffset = 796094307, and > then the memset() wanders off into the weeds). > > This is the contents of the DataDirectory: > > > (gdb) print Optional64Hdr->DataDirectory > > $22 = {{VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_EXPORT > > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_IMPORT > > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_RESOURCE > > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_EXCEPTION > > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_SECURITY > > {VirtualAddress = 36864, Size = 4096}, // EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC > > {VirtualAddress = 33264, Size = 237}, // EFI_IMAGE_DIRECTORY_ENTRY_DEBUG > > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_COPYRIGHT > > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_GLOBALPTR > > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_TLS > > {VirtualAddress = 0, Size = 0}, // EFI_IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG > > {VirtualAddress = 0, Size = 0}, > > {VirtualAddress = 0, Size = 0}, > > {VirtualAddress = 0, Size = 0}, > > {VirtualAddress = 0, Size = 0}, > > {VirtualAddress = 0, Size = 0}} > > According to <https://msdn.microsoft.com/en-us/library/ms809762.aspx>, > the format (element type) of the debug directory is specific to the > toolchain; the article says, > > > To determine the number of entries in the Microsoft linker-generated > > debug directory, divide the size of the debug directory (found in the > > size field of the data directory entry) by the size of an > > IMAGE_DEBUG_DIRECTORY structure > > Here's a hexdump of the debug directory (237 bytes starting from > (FileBuffer+33264)): > > 00000000 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00 00 |................| > 00000010 d1 00 00 00 0c 82 00 00 0c 82 00 00 4e 42 31 30 |Ñ...........NB10| > 00000020 00 00 00 00 00 00 00 00 00 00 00 00 2f 68 6f 6d |............/hom| > 00000030 65 2f 6c 61 63 6f 73 2f 73 72 63 2f 75 70 73 74 |e/lacos/src/upst| > 00000040 72 65 61 6d 2f 65 64 6b 32 2f 42 75 69 6c 64 2f |ream/edk2/Build/| > 00000050 4f 76 6d 66 58 36 34 2f 4e 4f 4f 50 54 5f 47 43 |OvmfX64/NOOPT_GC| > 00000060 43 34 38 2f 58 36 34 2f 4d 64 65 4d 6f 64 75 6c |C48/X64/MdeModul| > 00000070 65 50 6b 67 2f 55 6e 69 76 65 72 73 61 6c 2f 52 |ePkg/Universal/R| > 00000080 65 70 6f 72 74 53 74 61 74 75 73 43 6f 64 65 52 |eportStatusCodeR| > 00000090 6f 75 74 65 72 2f 52 75 6e 74 69 6d 65 44 78 65 |outer/RuntimeDxe| > 000000a0 2f 52 65 70 6f 72 74 53 74 61 74 75 73 43 6f 64 |/ReportStatusCod| > 000000b0 65 52 6f 75 74 65 72 52 75 6e 74 69 6d 65 44 78 |eRouterRuntimeDx| > 000000c0 65 2f 44 45 42 55 47 2f 52 65 70 6f 72 74 53 74 |e/DEBUG/ReportSt| > 000000d0 61 74 75 73 43 6f 64 65 52 6f 75 74 65 72 52 75 |atusCodeRouterRu| > 000000e0 6e 74 69 6d 65 44 78 65 2e 64 6c 6c 00 |ntimeDxe.dll.| > > The bytes 0x4e 0x42 0x31 0x30 ("NB10"), at offset 28, can be seen above > as "Characteristics = 808534606". Googling this value, it seems to be a > signature / magic value: CVINFO_PDB20_CVSIGNATURE. > > Then, VirtualAddress = 33264 is 0x81F0 hex, and "objdump -x" reports: > > > Sections: > > Idx Name Size VMA LMA File off Algn Flags > > [...] > > 2 .build-id 00000024 00000000000081f0 00000000000081f0 000081f0 2**2 CONTENTS, READONLY > > [...] > > SYMBOL TABLE: > > [...] > > 00000000000081f0 l d .build-id 0000000000000000 .build-id > > [...] > > Also, I found a function called pe_bfd_read_buildid() in the GNU > Binutils source that works with the PE_DEBUG_DATA entry of the data > directory. > > In "BaseTools/Scripts/GccBase.lds", we have: > > > /* > > * Retain the GNU build id but in a non-allocatable section so GenFw > > * does not copy it into the PE/COFF image. > > */ > > .build-id (INFO) : { *(.note.gnu.build-id) } > > This is from commit 7fd5d619806d ("BaseTools GCC: drop GNU notes section > from EFI image", 2016-07-27). > > So... I don't really know what's happening here, but the DEBUG entry of > the data directory (i.e., the debug directory) doesn't seem to be > structured like GenFw expects. > > > + memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)); > > + } > > + if (DebugEntry->Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { > > + RsdsEntry = (EFI_IMAGE_DEBUG_CODEVIEW_RSDS_ENTRY *) (FileBuffer + DebugEntry->FileOffset); > > + if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_RSDS) { > > + RsdsEntry->Unknown = 0; > > + RsdsEntry->Unknown2 = 0; > > + RsdsEntry->Unknown3 = 0; > > + RsdsEntry->Unknown4 = 0; > > + RsdsEntry->Unknown5 = 0; > > + } > > + } > > } > > } > > > > > > Thanks, > Laszlo > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-07-05 15:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-07-03 5:21 [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain Liming Gao 2017-07-03 5:28 ` Zhu, Yonghong 2017-07-05 12:42 ` Laszlo Ersek 2017-07-05 13:29 ` Laszlo Ersek 2017-07-05 15:29 ` Laszlo Ersek 2017-07-05 13:41 ` Leif Lindholm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox