From: qmuntal <quimmuntal@gmail.com>
Date: Mon, 13 Mar 2023 13:34:38 +0000 (+0100)
Subject: cmd/link: store COFF symbol complex type in the LSB instead of the MSB
X-Git-Tag: go1.21rc1~1286
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=b37c0602cdc9b7f13b3d539663e68b12f10b44b1;p=gostls13.git

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 266e165920..0167986c51 100644
--- 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 7ad4db9052..00af640b74 100644
--- 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)