From 6e57ecdb03fba66007e081c7d5fce99d39d9fab8 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Mon, 13 Mar 2023 15:10:59 +0100 Subject: [PATCH] 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 Reviewed-by: Alex Brainman Reviewed-by: Than McIntosh Reviewed-by: Heschi Kreinick Run-TryBot: Quim Muntal --- src/cmd/link/internal/ld/pe.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go index 0167986c51..08e5b976b6 100644 --- 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) } -- 2.48.1