[binutils-gdb] gdb: is_linked_with_cygwin_dll: handle import table not at beginning of .idata section

[binutils-gdb] gdb: is_linked_with_cygwin_dll: handle import table not at beginning of .idata section

[Simon Marchi]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=00ac85d3751b763155adb4e9d15dd134399b4e77

commit 00ac85d3751b763155adb4e9d15dd134399b4e77
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date:   Thu Apr 16 15:46:03 2020 -0400

    gdb: is_linked_with_cygwin_dll: handle import table not at beginning of .idata section
    
    When loading the file C:\Windows\SysWOW64\msvcrt.dll, taken from a
    Windows 10 system, into GDB, we get the following warning:
    
        warning: Failed to parse .idata section: name's virtual address (0x0) is outside .idata section's range [0xb82b8, 0xb97f0[.
    
    This uncovers an issue with how we parse the import table, part of the
    .idata section.  Right now, we assume that the import table is located
    at the beginning of the section.  That was the case in everything I had
    tried so far, but this file is an example where that's not true.
    
    We need to compute the offset of the import table within the .idata
    section, and start there, instead of at the beginning of the .idata
    section.  Using the file mentioned above, this is the values we have to
    work with:
    
      A) bfd_section_vma (idata_section)    101b8000
      B) Import table's virtual address        b82b8
      C) Image base                         10100000
    
    The virtual address that BFD returns us for the section has the image
    base applied, so we need to subtract it first.  The offset of the table
    in the section is therefore:
    
        B - (A - C)
    
    This patch implements that.
    
    gdb/ChangeLog:
    
            * windows-tdep.c (is_linked_with_cygwin_dll): Consider case where
            import table is not at beginning of .idata section.

Diff:
---
 gdb/ChangeLog      |  5 +++++
 gdb/windows-tdep.c | 47 ++++++++++++++++++++++++++++++++++-------------
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6c280e3f49f..ebba52faef9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-04-16  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* windows-tdep.c (is_linked_with_cygwin_dll): Consider case where
+	import table is not at beginning of .idata section.
+
 2020-04-16  Pedro Alves  <palves@redhat.com>
 
 	* inferior.c (delete_inferior): Use delete operator directly
diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index d1894ca0888..50bb9591f93 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -999,11 +999,32 @@ is_linked_with_cygwin_dll (bfd *abfd)
   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;
+  bfd_size_type idata_section_size = bfd_section_size (idata_section);
+  internal_extra_pe_aouthdr *pe_extra = &pe_data (abfd)->pe_opthdr;
+  bfd_vma import_table_va = pe_extra->DataDirectory[PE_IMPORT_TABLE].VirtualAddress;
+  bfd_vma idata_section_va = bfd_section_vma (idata_section);
+
+  /* The section's virtual address as reported by BFD has the image base applied,
+     remove it.  */
+  gdb_assert (idata_section_va >= pe_extra->ImageBase);
+  idata_section_va -= pe_extra->ImageBase;
+
+  bfd_vma idata_section_end_va = idata_section_va + idata_section_size;
+
+  /* Make sure that the import table is indeed within the .idata section's range.  */
+  if (import_table_va < idata_section_va
+      || import_table_va >= idata_section_end_va)
+    {
+      warning (_("\
+%s: import table's virtual address (0x%" BFD_VMA_FMT "x) is outside .idata \
+section's range [0x%" BFD_VMA_FMT "x, 0x%" BFD_VMA_FMT "x[."),
+	       bfd_get_filename (abfd), import_table_va, idata_section_va,
+	       idata_section_end_va);
+      return false;
+    }
+
+  /* The import table starts at this offset into the .idata section.  */
+  bfd_vma import_table_offset_in_sect = import_table_va - idata_section_va;
 
   /* Get the section's data.  */
   gdb::byte_vector idata_contents;
@@ -1013,9 +1034,10 @@ is_linked_with_cygwin_dll (bfd *abfd)
       return false;
     }
 
-  size_t idata_size = idata_contents.size ();
-  const gdb_byte *iter = idata_contents.data ();
-  const gdb_byte *end = idata_contents.data () + idata_size;
+  gdb_assert (idata_contents.size () == idata_section_size);
+
+  const gdb_byte *iter = idata_contents.data () + import_table_offset_in_sect;
+  const gdb_byte *end = idata_contents.data () + idata_section_size;
   const pe_import_directory_entry null_dir_entry = { 0 };
 
   /* Iterate through all directory entries.  */
@@ -1036,21 +1058,20 @@ is_linked_with_cygwin_dll (bfd *abfd)
 		  sizeof (pe_import_directory_entry)) == 0)
 	break;
 
-      bfd_vma name_addr = dir_entry->name_rva;
+      bfd_vma name_va = 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))
+      if (name_va < idata_section_va || name_va >= idata_section_end_va)
 	{
 	  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);
+		   name_va, idata_section_va, idata_section_end_va);
 	  break;
 	}
 
-      const gdb_byte *name = &idata_contents[name_addr - idata_addr];
+      const gdb_byte *name = &idata_contents[name_va - idata_section_va];
 
       /* Make sure we don't overshoot the end of the section with the streq.  */
       if (name + sizeof (CYGWIN_DLL_NAME) > end)