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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>

Author: thanm

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
 }

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`")

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) {