From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22c.google.com (mail-wm0-x22c.google.com [IPv6:2a00:1450:400c:c09::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 29C032095A6C9 for ; Wed, 5 Jul 2017 06:40:01 -0700 (PDT) Received: by mail-wm0-x22c.google.com with SMTP id f67so113428727wmh.1 for ; Wed, 05 Jul 2017 06:41:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=6KW8wMaHnw9uz6OJUUyu2WZch+iUWcWrW6EDm7s/MhQ=; b=HCnLJ/rvUBfDOFbQgVh3ivmFZ0WGIhcB2TxMbVpgRyY7AQCm4zYJVpX5QKX2E/DlpV hQqdvtrmd+1GLFWPrXzj3s0NP11ThdfxvWylT4F54zC/a/M3jqYztwu8r5mI52egV1cR DVV842KVhGK3t/MssMlBP6EVXe+ot/JeNUl8g= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=6KW8wMaHnw9uz6OJUUyu2WZch+iUWcWrW6EDm7s/MhQ=; b=Uo+k8f/Smvv3S+TWIKJtU1o26kC24czuFS+ETnFi6JxzTfD0N59/f46ZISA8uvBFfB 7f6TtyFTwlrkmYj/b6h+WPnZ+X7MQg7eACC36KWZftCICyiT//fN4Gaqn0COfxRMo/Rh eR/fZ7VH0nMSSqy3u7M/exLrJjqHH1UpYJypvGBV9i1h6fdquO550rSTWDySaL9Cgaoc gcDSNIAb5v5ivu44wYkGK70Gzw2RL+7Cbg1Sh7+RzKioXj8M4QLnE7QyE2brA1YfSIRm A25NS6Tw201Z0kKp6BuScWDRC2BLFCeRI1QBH6+fyjyE2yeiEmhHxutygqwSdhaQtoVX Fy9Q== X-Gm-Message-State: AIVw1136Apy7/ANr1If3hc+sSJWRY98WJpWgaPAs+pYOrXJA6NstIvId Y96+ybfe1ncYI+9A X-Received: by 10.28.30.213 with SMTP id e204mr1723191wme.40.1499262098911; Wed, 05 Jul 2017 06:41:38 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id 143sm14265283wmo.11.2017.07.05.06.41.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 05 Jul 2017 06:41:38 -0700 (PDT) Date: Wed, 5 Jul 2017 14:41:36 +0100 From: Leif Lindholm To: Laszlo Ersek Cc: Liming Gao , edk2-devel@lists.01.org, Ard Biesheuvel , "Kinney, Michael D" , Andrew Fish , Yonghong Zhu Message-ID: <20170705134136.GB26676@bivouac.eciton.net> References: <1499059281-11744-1-git-send-email-liming.gao@intel.com> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [Patch] BaseTools: Update GenFw to clear unused debug entry generated by VS tool chain X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Jul 2017 13:40:01 -0000 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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 > > --- > > 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 , > 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