cmd/link/internal/loadpe: generalize handling of "__imp_*" syms

The existing PE file loader has a special case for the symbol
"__acrt_iob_func", whose hosting object file contains both an actual
definition and also a DLL import symbol "__imp___acrt_iob_func". The
normal way of handling __imp_XXX symbols is for the host object loader
to rename them to their intended target (e.g. "XXX") however if the
target is also defined locally, you get a duplicate definition.

This patch generalizes the def/import symbol detection to apply to all
symbols in the object file being loaded (not just a hard-coded set),
since it will be needed when reading things like crt2.o.

Updates #35006.

Change-Id: I0d0607c27bb7d5f3cb415bc95db816aa13746ba2
Reviewed-on: https://go-review.googlesource.com/c/go/+/382837
Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/cmd/link/internal/loadpe/ldpe.go b/src/cmd/link/internal/loadpe/ldpe.go
index c9fde60d0fda4be9e98fa83bbfba70b247c15d5e..871ec73e01fdeac48c251a69968f0d1e96b3404b 100644 (file)
--- a/src/cmd/link/internal/loadpe/ldpe.go
+++ b/src/cmd/link/internal/loadpe/ldpe.go
@@ -180,6 +180,7 @@ type peLoaderState struct {
        arch            *sys.Arch
        f               *pe.File
        sectsyms        map[*pe.Section]loader.Sym
+       defWithImp      map[string]struct{}
        sectdata        map[*pe.Section][]byte
        localSymVersion int
 }
@@ -261,6 +262,13 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read
                }
        }
 
+       // Make a prepass over the symbols to detect situations where
+       // we have both a defined symbol X and an import symbol __imp_X
+       // (needed by readpesym()).
+       if err := state.preprocessSymbols(); err != nil {
+               return nil, nil, err
+       }
+
        // load relocations
        for _, rsect := range f.Sections {
                if _, found := state.sectsyms[rsect]; !found {
@@ -516,26 +524,20 @@ func (state *peLoaderState) readpesym(pesym *pe.COFFSymbol) (*loader.SymbolBuild
                name = state.l.SymName(state.sectsyms[state.f.Sections[pesym.SectionNumber-1]])
        } else {
                name = symname
-               switch state.arch.Family {
-               case sys.AMD64:
-                       if name == "__imp___acrt_iob_func" {
-                               // Do not rename __imp___acrt_iob_func into __acrt_iob_func,
-                               // because __imp___acrt_iob_func symbol is real
-                               // (see commit b295099 from git://git.code.sf.net/p/mingw-w64/mingw-w64 for details).
-                       } else {
-                               name = strings.TrimPrefix(name, "__imp_") // __imp_Name => Name
-                       }
-               case sys.I386:
-                       if name == "__imp____acrt_iob_func" {
-                               // Do not rename __imp____acrt_iob_func into ___acrt_iob_func,
-                               // because __imp____acrt_iob_func symbol is real
-                               // (see commit b295099 from git://git.code.sf.net/p/mingw-w64/mingw-w64 for details).
+               if strings.HasPrefix(symname, "__imp_") {
+                       orig := symname[len("__imp_"):]
+                       if _, ok := state.defWithImp[orig]; ok {
+                               // Don't rename __imp_XXX to XXX, since if we do this
+                               // we'll wind up with a duplicate definition. One
+                               // example is "__acrt_iob_func"; see commit b295099
+                               // from git://git.code.sf.net/p/mingw-w64/mingw-w64
+                               // for details.
                        } else {
                                name = strings.TrimPrefix(name, "__imp_") // __imp_Name => Name
                        }
-                       if name[0] == '_' {
-                               name = name[1:] // _Name => Name
-                       }
+               }
+               if state.arch.Family == sys.I386 && name[0] == '_' {
+                       name = name[1:] // _Name => Name
                }
        }
 
@@ -576,3 +578,35 @@ func (state *peLoaderState) readpesym(pesym *pe.COFFSymbol) (*loader.SymbolBuild
 
        return bld, s, nil
 }
+
+// preprocessSymbols walks the COFF symbols for the PE file we're
+// reading and looks for cases where we have both a symbol definition
+// for "XXX" and an "__imp_XXX" symbol, recording these cases in a map
+// in the state struct. This information will be used in readpesym()
+// above to give such symbols special treatment.
+func (state *peLoaderState) preprocessSymbols() error {
+       imp := make(map[string]struct{})
+       def := make(map[string]struct{})
+       for i, numaux := 0, 0; i < len(state.f.COFFSymbols); i += numaux + 1 {
+               pesym := &state.f.COFFSymbols[i]
+               numaux = int(pesym.NumberOfAuxSymbols)
+               if pesym.SectionNumber == 0 { // extern
+                       continue
+               }
+               symname, err := pesym.FullName(state.f.StringTable)
+               if err != nil {
+                       return err
+               }
+               def[symname] = struct{}{}
+               if strings.HasPrefix(symname, "__imp_") {
+                       imp[strings.TrimPrefix(symname, "__imp_")] = struct{}{}
+               }
+       }
+       state.defWithImp = make(map[string]struct{})
+       for n := range imp {
+               if _, ok := def[n]; ok {
+                       state.defWithImp[n] = struct{}{}
+               }
+       }
+       return nil
+}