From b37c0602cdc9b7f13b3d539663e68b12f10b44b1 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Mon, 13 Mar 2023 14:34:38 +0100 Subject: [PATCH] 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 Run-TryBot: Quim Muntal Reviewed-by: Than McIntosh TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor --- src/cmd/link/internal/ld/pe.go | 14 ++++++++++---- src/cmd/link/internal/loadpe/ldpe.go | 5 ++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go index 266e165920c..0167986c51f 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 7ad4db9052b..00af640b741 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)