From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id A076B385DC0B for ; Wed, 1 Apr 2020 21:54:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A076B385DC0B Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 031LrwFv019588 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 1 Apr 2020 17:54:03 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 031LrwFv019588 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 31B771F376; Wed, 1 Apr 2020 17:53:58 -0400 (EDT) Subject: Re: [PATCH 6/7] gdb: select "Cygwin" OS ABI for Cygwin binaries To: Pedro Alves , gdb-patches@sourceware.org Cc: Jon Turney , Simon Marchi References: <20200316170845.184386-1-simon.marchi@polymtl.ca> <20200316170845.184386-7-simon.marchi@polymtl.ca> <0b76517a-f6dd-0aa8-17fb-ce5a9accbf42@redhat.com> From: Simon Marchi Message-ID: Date: Wed, 1 Apr 2020 17:53:57 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <0b76517a-f6dd-0aa8-17fb-ce5a9accbf42@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US-large Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 1 Apr 2020 21:53:58 +0000 X-Spam-Status: No, score=-24.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Apr 2020 21:54:17 -0000 On 2020-04-01 5:36 p.m., Pedro Alves wrote: > On 3/16/20 5:08 PM, Simon Marchi via Gdb-patches wrote: >> +bool >> +is_linked_with_cygwin_dll (bfd *abfd) >> +{ >> + /* The list of DLLs a PE is linked to is in the .idata section. See: >> + >> + https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#the-idata-section >> + */ >> + asection *idata_section = bfd_get_section_by_name (abfd, ".idata"); >> + if (idata_section == nullptr) >> + return false; >> + >> + /* Find the virtual address of the .idata section. We must subtract this >> + from the RVAs (relative virtual addresses) to obtain an offset in the >> + section. */ >> + bfd_vma idata_addr = >> + pe_data (abfd)->pe_opthdr.DataDirectory[PE_IMPORT_TABLE].VirtualAddress; > > = on next line. Unless it doesn't fit, then let's ignore. Yes it fits. >> + >> + /* Map the section's data. */ >> + bfd_size_type idata_size; >> + const gdb_byte *const idata_contents >> + = gdb_bfd_map_section (idata_section, &idata_size); >> + if (idata_contents == nullptr) >> + { >> + warning (_("Failed to get content of .idata section.")); >> + return false; >> + } >> + >> + const gdb_byte *iter = idata_contents; >> + const gdb_byte *end = idata_contents + idata_size; >> + const pe_import_directory_entry null_dir_entry = { 0 }; >> + >> + /* Iterate through all directory entries. */ >> + while (true) >> + { >> + /* Is there enough space left in the section for another entry? */ >> + if (iter + sizeof (pe_import_directory_entry) > end) >> + { >> + warning (_("Failed to parse .idata section: unexpected end of " >> + ".idata section.")); >> + break; >> + } >> + >> + pe_import_directory_entry *dir_entry = (pe_import_directory_entry *) iter; > > Is 'iter' guaranteed to be sufficiently aligned? Recall that this is > host-independent code. I admit I haven't thought about that. Within the PE file, the data of sections is aligned on at least 512 bytes. See "FileAlignment" here: https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#optional-header-windows-specific-fields-image-only However, when BFD maps the file/section data in memory for GDB to read, is that mapping guaranteed to be sufficiently aligned as well? >> + >> + /* Is it the end of list marker? */ >> + if (memcmp (dir_entry, &null_dir_entry, >> + sizeof (pe_import_directory_entry)) == 0) >> + break; >> + >> + bfd_vma name_addr = dir_entry->name_rva; >> + >> + /* If the name's virtual address is smaller than the section's virtual >> + address, there's a problem. */ >> + if (name_addr < idata_addr >> + || name_addr >= (idata_addr + idata_size)) >> + { >> + warning (_("\ >> +Failed to parse .idata section: name's virtual address (0x%" BFD_VMA_FMT "x) \ >> +is outside .idata section's range [0x%" BFD_VMA_FMT "x, 0x%" BFD_VMA_FMT "x[."), >> + name_addr, idata_addr, idata_addr + idata_size); >> + break; >> + } >> + >> + const gdb_byte *name = &idata_contents[name_addr - idata_addr]; >> + >> + /* Make sure we don't overshoot the end of the section with the streq. */ >> + if (name + sizeof(CYGWIN_DLL_NAME) > end) > > Missing space before parens. This patch is already pushed so I've pushed the following to fix the style issues. Thanks for the review, Simon >From 2836752f8ff55ea1fc7f6b1e7f8ff778775646f8 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 1 Apr 2020 17:41:31 -0400 Subject: [PATCH] gdb: fix style issues in is_linked_with_cygwin_dll gdb/ChangeLog: * windows-tdep.c (is_linked_with_cygwin_dll): Fix style. --- gdb/ChangeLog | 4 ++++ gdb/windows-tdep.c | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d5715a8fa005..4775ff858aab 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,7 @@ +2020-04-01 Simon Marchi + + * windows-tdep.c (is_linked_with_cygwin_dll): Fix style. + 2020-04-01 Bernd Edlinger * buildsym.c (record_line): Fix undefined behavior and preserve diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c index 31b7b57005df..0042ea350e80 100644 --- a/gdb/windows-tdep.c +++ b/gdb/windows-tdep.c @@ -932,8 +932,8 @@ is_linked_with_cygwin_dll (bfd *abfd) /* Find the virtual address of the .idata section. We must subtract this from the RVAs (relative virtual addresses) to obtain an offset in the section. */ - bfd_vma idata_addr = - pe_data (abfd)->pe_opthdr.DataDirectory[PE_IMPORT_TABLE].VirtualAddress; + bfd_vma idata_addr + = pe_data (abfd)->pe_opthdr.DataDirectory[PE_IMPORT_TABLE].VirtualAddress; /* Map the section's data. */ bfd_size_type idata_size; @@ -984,14 +984,14 @@ is outside .idata section's range [0x%" BFD_VMA_FMT "x, 0x%" BFD_VMA_FMT "x[."), const gdb_byte *name = &idata_contents[name_addr - idata_addr]; /* Make sure we don't overshoot the end of the section with the streq. */ - if (name + sizeof(CYGWIN_DLL_NAME) > end) + if (name + sizeof (CYGWIN_DLL_NAME) > end) continue; /* Finally, check if this is the dll name we are looking for. */ if (streq ((const char *) name, CYGWIN_DLL_NAME)) return true; - iter += sizeof(pe_import_directory_entry); + iter += sizeof (pe_import_directory_entry); } return false; -- 2.26.0