debug/pe: parse the import directory correctly

This parses the import table properly which allows for debug/pe
to extract import symbols from pecoffs linked with an import
table in a section named something other than ".idata"

The section names in a pecoff object aren't guaranteed to actually
mean anything, so hardcoding a search for the ".idata" section
is not guaranteed to find the import table in all shared libraries.
This resulted in debug/pe being unable to read import symbols
from some libraries.

The proper way to locate the import table is to validate the
number of data directory entries, locate the import entry, and
then use the va to identify the section containing the import
table. This patch does exactly this.

Fixes #16103.

Change-Id: I3ab6de7f896a0c56bb86c3863e504e8dd4c8faf3
GitHub-Last-Rev: ce8077cb154f18ada7a86e152ab03de813937816
GitHub-Pull-Request: golang/go#25193
Reviewed-on: https://go-review.googlesource.com/110555
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>

diff --git a/src/debug/pe/file.go b/src/debug/pe/file.go
index 87f225cb3981008af03e24905ee4d4142b59114c..d1b1407f96937dfa2f5ad4b25d6130925ade3336 100644 (file)
--- a/src/debug/pe/file.go
+++ b/src/debug/pe/file.go
@@ -260,19 +260,59 @@ type ImportDirectory struct {
 // It does not return weak symbols.
 func (f *File) ImportedSymbols() ([]string, error) {
        pe64 := f.Machine == IMAGE_FILE_MACHINE_AMD64
-       ds := f.Section(".idata")
+
+       // grab the number of data directory entries
+       var dd_length uint32
+       if pe64 {
+               dd_length = f.OptionalHeader.(*OptionalHeader64).NumberOfRvaAndSizes
+       } else {
+               dd_length = f.OptionalHeader.(*OptionalHeader32).NumberOfRvaAndSizes
+       }
+
+       // check that the length of data directory entries is large 
+       // enough to include the imports directory.
+       if dd_length < IMAGE_DIRECTORY_ENTRY_IMPORT + 1 {
+               return nil, nil
+       }
+
+       // grab the import data directory entry
+       var idd DataDirectory
+       if pe64 {
+               idd = f.OptionalHeader.(*OptionalHeader64).DataDirectory[IMAGE_DIRECTORY_ENTRY_IMPORT]
+       } else {
+               idd = f.OptionalHeader.(*OptionalHeader32).DataDirectory[IMAGE_DIRECTORY_ENTRY_IMPORT]
+       }
+
+       // figure out which section contains the import directory table
+       var ds *Section
+       ds = nil
+       for _, s := range f.Sections {
+               if s.VirtualAddress <= idd.VirtualAddress && idd.VirtualAddress < s.VirtualAddress + s.VirtualSize {
+                       ds = s
+                       break
+               }
+       }
+
+       // didn't find a section, so no import libraries were found
        if ds == nil {
-               // not dynamic, so no libraries
                return nil, nil
        }
+
        d, err := ds.Data()
        if err != nil {
                return nil, err
        }
+
+       // seek to the virtual address specified in the import data directory
+       d = d[idd.VirtualAddress - ds.VirtualAddress:]
+
+       // start decoding the import directory
        var ida []ImportDirectory
        for len(d) > 0 {
                var dt ImportDirectory
                dt.OriginalFirstThunk = binary.LittleEndian.Uint32(d[0:4])
+               dt.TimeDateStamp = binary.LittleEndian.Uint32(d[4:8])
+               dt.ForwarderChain = binary.LittleEndian.Uint32(d[8:12])
                dt.Name = binary.LittleEndian.Uint32(d[12:16])
                dt.FirstThunk = binary.LittleEndian.Uint32(d[16:20])
                d = d[20:]
@@ -282,7 +322,7 @@ func (f *File) ImportedSymbols() ([]string, error) {
                ida = append(ida, dt)
        }
        // TODO(brainman): this needs to be rewritten
-       //  ds.Data() return contets of .idata section. Why store in variable called "names"?
+       //  ds.Data() returns contents of section containing import table. Why store in variable called "names"?
        //  Why we are retrieving it second time? We already have it in "d", and it is not modified anywhere.
        //  getString does not extracts a string from symbol string table (as getString doco says).
        //  Why ds.Data() called again and again in the loop?

diff --git a/src/debug/pe/file_test.go b/src/debug/pe/file_test.go
index 8645d676b752b2cf9624ff6ab040a7a5a8cea4c9..24cd673254e5f67f7318c7249e80d5559266302a 100644 (file)
--- a/src/debug/pe/file_test.go
+++ b/src/debug/pe/file_test.go
@@ -532,3 +532,31 @@ func TestBuildingWindowsGUI(t *testing.T) {
                t.Fatalf("unexpected OptionalHeader type: have %T, but want *pe.OptionalHeader32 or *pe.OptionalHeader64", oh)
        }
 }
+
+func TestImportTableInUnknownSection(t *testing.T) {
+       if runtime.GOOS != "windows" {
+               t.Skip("skipping windows only test")
+       }
+
+       // first we need to find this font driver
+       path, err := exec.LookPath("atmfd.dll")
+       if err != nil {
+               t.Fatalf("unable to locate required file %q in search path: %s", "atmfd.dll", err)
+       }
+
+       f, err := Open(path)
+       if err != nil {
+               t.Error(err)
+       }
+       defer f.Close()
+
+       // now we can extract its imports
+       symbols, err := f.ImportedSymbols()
+       if err != nil {
+               t.Error(err)
+       }
+
+       if len(symbols) == 0 {
+               t.Fatalf("unable to locate any imported symbols within file %q.", path)
+       }
+}

diff --git a/src/debug/pe/pe.go b/src/debug/pe/pe.go
index 8050d59c70dd4f2d060edec7bcb1d45275738930..9eaf33c3101da3c91821fdb3f2fe73173ccf9997 100644 (file)
--- a/src/debug/pe/pe.go
+++ b/src/debug/pe/pe.go
@@ -108,3 +108,23 @@ const (
        IMAGE_FILE_MACHINE_THUMB     = 0x1c2
        IMAGE_FILE_MACHINE_WCEMIPSV2 = 0x169
 )
+
+// IMAGE_DIRECTORY_ENTRY constants
+const (
+       IMAGE_DIRECTORY_ENTRY_EXPORT = 0
+       IMAGE_DIRECTORY_ENTRY_IMPORT = 1
+       IMAGE_DIRECTORY_ENTRY_RESOURCE = 2
+       IMAGE_DIRECTORY_ENTRY_EXCEPTION = 3
+       IMAGE_DIRECTORY_ENTRY_SECURITY = 4
+       IMAGE_DIRECTORY_ENTRY_BASERELOC = 5
+       IMAGE_DIRECTORY_ENTRY_DEBUG = 6
+       IMAGE_DIRECTORY_ENTRY_ARCHITECTURE = 7
+       IMAGE_DIRECTORY_ENTRY_GLOBALPTR = 8
+       IMAGE_DIRECTORY_ENTRY_TLS = 9
+       IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG = 10
+       IMAGE_DIRECTORY_ENTRY_BOUND_IMPORT = 11
+       IMAGE_DIRECTORY_ENTRY_IAT = 12
+       IMAGE_DIRECTORY_ENTRY_DELAY_IMPORT = 13
+       IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR = 14
+)
+