cmd/link: store COFF symbol complex type in the LSB instead of the MSB

Microsoft's PE documentation is contradictory. It says that the
symbol's complex type [1] is stored in the pesym.Type most significant
byte (MSB), but MSVC, LLVM, and mingw store it in the 4 high bits of
the less significant byte (LSB). dumpbin understands both encoding.

Previous to CL 475355 the Go compiler mixed MSB and LSB encoding.
CL 475355 updated to compiler to use the MSB, but this causes problems
with mingw, which emits a warning when MSB is used.

For reference, LLVM also hit this issue long time ago:
https://github.com/llvm/llvm-project/issues/8692

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

Change-Id: I7e750bde9c20e2c4c1c023203d7abd6fb26d9d30
Reviewed-on: https://go-review.googlesource.com/c/go/+/475855
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>

diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go
index 266e165920c001b8228c4233d115960ecf959512..0167986c51f9d51168abe5edc38ba9192c4fc43a 100644 (file)
--- a/src/cmd/link/internal/ld/pe.go
+++ b/src/cmd/link/internal/ld/pe.go
@@ -753,13 +753,19 @@ func (f *peFile) writeSymbols(ctxt *Link) {
                if ctxt.IsExternal() {
                        peSymType = IMAGE_SYM_TYPE_NULL
                } else {
-                       peSymType = IMAGE_SYM_DTYPE_ARRAY<<8 + IMAGE_SYM_TYPE_STRUCT
+                       // 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
                }
                sect, value, err := f.mapToPESection(ldr, s, ctxt.LinkMode)
                if err != nil {
-                       if t == sym.SDYNIMPORT || t == sym.SHOSTOBJ || t == sym.SUNDEFEXT {
-                               peSymType = IMAGE_SYM_DTYPE_FUNCTION << 8
-                       } else {
+                       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)
                        }
                }

diff --git a/src/cmd/link/internal/loadpe/ldpe.go b/src/cmd/link/internal/loadpe/ldpe.go
index 7ad4db9052b0636769acc42a704ea20a8923ed00..00af640b7411100ef47208bb7b2ecaa2c4e831ae 100644 (file)
--- a/src/cmd/link/internal/loadpe/ldpe.go
+++ b/src/cmd/link/internal/loadpe/ldpe.go
@@ -672,7 +672,10 @@ func (state *peLoaderState) readpesym(pesym *pe.COFFSymbol) (*loader.SymbolBuild
 
        var s loader.Sym
        var bld *loader.SymbolBuilder
-       switch uint8(pesym.Type >> 8) {
+       // 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.
+       switch uint8(pesym.Type&0xf0) >> 4 {
        default:
                return nil, 0, fmt.Errorf("%s: invalid symbol type %d", symname, pesym.Type)