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 <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index f2a9921c8e1b648e6323a7a13f8f192a67bb167e..1d44c0eb18b68213e9998c79120f566d4dd15bbf 100644 (file)
--- 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 e1d2da3f303adf4486e85363406e3ea50c882bbc..e5859868b7482fcbcbcb0f5615b5142953ede611 100644 (file)
--- 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 b39e0521066bdcfac5445462c7caa600ed0dfeab..9b76f2801d1386765414e8be3411f0adce2ebb96 100644 (file)
--- 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) {