From d923309a17d1b7eeacc75798cdca905d5b143681 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Thu, 21 Mar 2019 09:20:11 -0400 Subject: [PATCH] cmd/link: add optional sanity checking for duplicate symbols Introduce a new linker command line option "-strictdups", which enables sanity checking of "ok to duplicate" symbols, especially DWARF info symbols. Acceptable values are 0 (no checking) 1 (issue warnings) and 2 (issue a fatal error checks fail). Currently if we read a DWARF symbol (such as "go.info.PKG.FUNCTION") from one object file, and then encounter the same symbol later on while reading another object file, we simply discard the second one and move on with the link, since the two should in theory be identical. If as a result of a compiler bug we wind up with symbols that are not identical, this tends to (silently) result in incorrect DWARF generation, which may or may not be discovered depending on who is consuming the DWARF and what's being done with it. When this option is turned on, at the point where a duplicate symbol is detected in the object file reader, we check to make sure that the length/contents of the symbol are the same as the previously read symbol, and print a descriptive warning (or error) if not. For the time being this can be used for one-off testing to find problems; at some point it would be nice if we can enable it by default. Updates #30908. Change-Id: I64c4e07c326b4572db674ff17c93307e2eec607c Reviewed-on: https://go-review.googlesource.com/c/go/+/168410 Run-TryBot: Than McIntosh TryBot-Result: Gobot Gobot Reviewed-by: Keith Randall --- src/cmd/link/internal/ld/lib.go | 13 ++++++- src/cmd/link/internal/ld/main.go | 1 + src/cmd/link/internal/objfile/objfile.go | 44 +++++++++++++++++++++++- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go index f2a9921c8e..1d44c0eb18 100644 --- a/src/cmd/link/internal/ld/lib.go +++ b/src/cmd/link/internal/ld/lib.go @@ -1725,7 +1725,18 @@ func ldobj(ctxt *Link, f *bio.Reader, lib *sym.Library, length int64, pn string, ldpkg(ctxt, f, lib, import1-import0-2, pn) // -2 for !\n f.Seek(import1, 0) - objfile.Load(ctxt.Arch, ctxt.Syms, f, lib, eof-f.Offset(), pn) + flags := 0 + switch *FlagStrictDups { + case 0: + break + case 1: + flags = objfile.StrictDupsWarnFlag + case 2: + flags = objfile.StrictDupsErrFlag + default: + log.Fatalf("invalid -strictdups flag value %d", *FlagStrictDups) + } + objfile.Load(ctxt.Arch, ctxt.Syms, f, lib, eof-f.Offset(), pn, flags) addImports(ctxt, lib, pn) return nil } diff --git a/src/cmd/link/internal/ld/main.go b/src/cmd/link/internal/ld/main.go index e1d2da3f30..e5859868b7 100644 --- a/src/cmd/link/internal/ld/main.go +++ b/src/cmd/link/internal/ld/main.go @@ -85,6 +85,7 @@ var ( Flag8 bool // use 64-bit addresses in symbol table flagInterpreter = flag.String("I", "", "use `linker` as ELF dynamic linker") FlagDebugTramp = flag.Int("debugtramp", 0, "debug trampolines") + FlagStrictDups = flag.Int("strictdups", 0, "sanity check duplicate symbol contents during object file reading (1=warn 2=err).") FlagRound = flag.Int("R", -1, "set address rounding `quantum`") FlagTextAddr = flag.Int64("T", -1, "set text segment `address`") diff --git a/src/cmd/link/internal/objfile/objfile.go b/src/cmd/link/internal/objfile/objfile.go index b39e052106..9b76f2801d 100644 --- a/src/cmd/link/internal/objfile/objfile.go +++ b/src/cmd/link/internal/objfile/objfile.go @@ -17,8 +17,10 @@ import ( "cmd/internal/objabi" "cmd/internal/sys" "cmd/link/internal/sym" + "fmt" "io" "log" + "os" "strconv" "strings" ) @@ -39,6 +41,7 @@ type objReader struct { pn string dupSym *sym.Symbol localSymVersion int + flags int // rdBuf is used by readString and readSymName as scratch for reading strings. rdBuf []byte @@ -54,9 +57,22 @@ type objReader struct { file []*sym.Symbol } +// Flags to enable optional behavior during object loading/reading. + +const ( + NoFlag int = iota + + // Sanity-check duplicate symbol contents, issuing warning + // when duplicates have different lengths or contents. + StrictDupsWarnFlag + + // Similar to StrictDupsWarnFlag, but issue fatal error. + StrictDupsErrFlag +) + // Load loads an object file f into library lib. // The symbols loaded are added to syms. -func Load(arch *sys.Arch, syms *sym.Symbols, f *bio.Reader, lib *sym.Library, length int64, pn string) { +func Load(arch *sys.Arch, syms *sym.Symbols, f *bio.Reader, lib *sym.Library, length int64, pn string, flags int) { start := f.Offset() r := &objReader{ rd: f.Reader, @@ -66,6 +82,7 @@ func Load(arch *sys.Arch, syms *sym.Symbols, f *bio.Reader, lib *sym.Library, le pn: pn, dupSym: &sym.Symbol{Name: ".dup"}, localSymVersion: syms.IncVersion(), + flags: flags, } r.loadObjFile() if f.Offset() != start+length { @@ -340,6 +357,31 @@ overwrite: if s.Type == sym.SDWARFINFO { r.patchDWARFName(s) } + + if isdup && r.flags&(StrictDupsWarnFlag|StrictDupsErrFlag) != 0 { + // Compare the just-read symbol with the previously read + // symbol of the same name, verifying that they have the same + // payload. If not, issue a warning and possibly an error. + if !bytes.Equal(s.P, dup.P) { + reason := "same length but different contents" + if len(s.P) != len(dup.P) { + reason = fmt.Sprintf("new length %d != old length %d", + len(data), len(dup.P)) + } + fmt.Fprintf(os.Stderr, "cmd/link: while reading object for '%v': duplicate symbol '%s', previous def at '%v', with mismatched payload: %s\n", r.lib, dup, dup.Lib, reason) + + // For the moment, whitelist DWARF subprogram DIEs for + // auto-generated wrapper functions. What seems to happen + // here is that we get different line numbers on formal + // params; I am guessing that the pos is being inherited + // from the spot where the wrapper is needed. + whitelist := strings.HasPrefix(dup.Name, "go.info.go.interface") + + if r.flags&StrictDupsErrFlag != 0 && !whitelist { + log.Fatalf("failed duplicate symbol check on '%s' reading %s", dup.Name, r.pn) + } + } + } } func (r *objReader) patchDWARFName(s *sym.Symbol) { -- 2.48.1