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....@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
