cmd/link/internal/ld: emit better complex types for COFF symbols

The Go linker has always used IMAGE_SYM_TYPE_NULL as COFF symbol
type [1] when external linking and array of structs
(IMAGE_SYM_DTYPE_ARRAY<<4+IMAGE_SYM_TYPE_STRUCT) when internal linking.
This behavior seems idiosyncratic, and looking at the git history it
seems that it has probably been cargo culted from earlier toolchains.

This CL updates the Go linker to use IMAGE_SYM_DTYPE_FUNCTION<<4 for
those symbols representing functions, and IMAGE_SYM_TYPE_NULL otherwise.

This new behavior better represents the symbol types, and can help
other tools interpreting the intent of each symbol, e.g. debuggers or
tools extracting debug info from Go binaries. It also mimics what other
toolchains do, i.e. MSVC, LLVM, and GCC.

[1] https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#type-representation

Change-Id: I6b39b2048e95f0324b2eb90c85802ce42db455d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/475856
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>

diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go
index 0167986c51f9d51168abe5edc38ba9192c4fc43a..08e5b976b6b0751e0f4a682ba9aee3bb9f328408 100644 (file)
--- a/src/cmd/link/internal/ld/pe.go
+++ b/src/cmd/link/internal/ld/pe.go
@@ -749,22 +749,20 @@ func (f *peFile) writeSymbols(ctxt *Link) {
 
                name = mangleABIName(ctxt, ldr, s, name)
 
-               var peSymType uint16
-               if ctxt.IsExternal() {
-                       peSymType = IMAGE_SYM_TYPE_NULL
-               } else {
+               var peSymType uint16 = IMAGE_SYM_TYPE_NULL
+               switch t {
+               case sym.STEXT, sym.SDYNIMPORT, sym.SHOSTOBJ, sym.SUNDEFEXT:
                        // Microsoft's PE documentation is contradictory. It says that the symbol's complex type
                        // is stored in the pesym.Type most significant byte, but MSVC, LLVM, and mingw store it
-                       // in the 4 high bits of the less significant byte.
-                       peSymType = IMAGE_SYM_DTYPE_ARRAY<<4 + IMAGE_SYM_TYPE_STRUCT
+                       // in the 4 high bits of the less significant byte. Also, the PE documentation says that
+                       // the basic type for a function should be IMAGE_SYM_TYPE_VOID,
+                       // but the reality is that it uses IMAGE_SYM_TYPE_NULL instead.
+                       peSymType = IMAGE_SYM_DTYPE_FUNCTION<<4 + IMAGE_SYM_TYPE_NULL
                }
                sect, value, err := f.mapToPESection(ldr, s, ctxt.LinkMode)
                if err != nil {
                        switch t {
                        case sym.SDYNIMPORT, sym.SHOSTOBJ, sym.SUNDEFEXT:
-                               // Microsoft's PE documentation says that the basic type for a function should be
-                               // IMAGE_SYM_TYPE_VOID, but the reality is that it uses IMAGE_SYM_TYPE_NULL instead.
-                               peSymType = IMAGE_SYM_DTYPE_FUNCTION<<4 + IMAGE_SYM_TYPE_NULL
                        default:
                                ctxt.Errorf(s, "addpesym: %v", err)
                        }