[dev.link] cmd/link: restore -strictdups flag in newobj mode

Change-Id: I93ad769595fa343400afa342af12e1445abff084
Reviewed-on: https://go-review.googlesource.com/c/go/+/204918
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>

3 files changed, 145 insertions, 12 deletions

diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index 4c7451a114..39ed53e38c 100644
--- a/src/cmd/link/internal/ld/lib.go
+++ b/src/cmd/link/internal/ld/lib.go
@@ -378,7 +378,16 @@ func (ctxt *Link) findLibPath(libname string) string {
 
 func (ctxt *Link) loadlib() {
 	if *flagNewobj {
-		ctxt.loader = loader.NewLoader()
+		var flags uint32
+		switch *FlagStrictDups {
+		case 0:
+			// nothing to do
+		case 1, 2:
+			flags = loader.FlagStrictDups
+		default:
+			log.Fatalf("invalid -strictdups flag value %d", *FlagStrictDups)
+		}
+		ctxt.loader = loader.NewLoader(flags)
 	}
 
 	ctxt.cgo_export_static = make(map[string]bool)
@@ -550,6 +559,10 @@ func (ctxt *Link) loadlib() {
 	ctxt.Loaded = true
 
 	importcycles()
+
+	if *flagNewobj {
+		strictDupMsgCount = ctxt.loader.NStrictDupMsgs()
+	}
 }
 
 // Set up dynexp list.
diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go
index 46d93c5124..c0fa5fa7ce 100644
--- a/src/cmd/link/internal/loader/loader.go
+++ b/src/cmd/link/internal/loader/loader.go
@@ -119,9 +119,18 @@ type Loader struct {
 	Reachparent []Sym
 
 	relocBatch []sym.Reloc // for bulk allocation of relocations
+
+	flags uint32
+
+	strictDupMsgs int // number of strict-dup warning/errors, when FlagStrictDups is enabled
 }
 
-func NewLoader() *Loader {
+const (
+	// Loader.flags
+	FlagStrictDups = 1 << iota
+)
+
+func NewLoader(flags uint32) *Loader {
 	nbuiltin := goobj2.NBuiltin()
 	return &Loader{
 		start:         make(map[*oReader]Sym),
@@ -132,6 +141,7 @@ func NewLoader() *Loader {
 		itablink:      make(map[Sym]struct{}),
 		extStaticSyms: make(map[nameVer]Sym),
 		builtinSyms:   make([]Sym, nbuiltin),
+		flags:         flags,
 	}
 }
 
@@ -170,6 +180,9 @@ func (l *Loader) AddSym(name string, ver int, i Sym, r *oReader, dupok bool, typ
 	}
 	if oldi, ok := l.symsByName[ver][name]; ok {
 		if dupok {
+			if l.flags&FlagStrictDups != 0 {
+				l.checkdup(name, i, r, oldi)
+			}
 			return false
 		}
 		oldr, li := l.toLocal(oldi)
@@ -366,6 +379,42 @@ func (l *Loader) IsDup(i Sym) bool {
 	return l.symsByName[ver][name] != i
 }
 
+// Check that duplicate symbols have same contents.
+func (l *Loader) checkdup(name string, i Sym, r *oReader, dup Sym) {
+	li := int(i - l.startIndex(r))
+	p := r.Data(li)
+	if strings.HasPrefix(name, "go.info.") {
+		p, _ = patchDWARFName1(p, r)
+	}
+	rdup, ldup := l.toLocal(dup)
+	pdup := rdup.Data(ldup)
+	if strings.HasPrefix(name, "go.info.") {
+		pdup, _ = patchDWARFName1(pdup, rdup)
+	}
+	if bytes.Equal(p, pdup) {
+		return
+	}
+	reason := "same length but different contents"
+	if len(p) != len(pdup) {
+		reason = fmt.Sprintf("new length %d != old length %d", len(p), len(pdup))
+	}
+	fmt.Fprintf(os.Stderr, "cmd/link: while reading object for '%v': duplicate symbol '%s', previous def at '%v', with mismatched payload: %s\n", r.unit.Lib, name, rdup.unit.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(name, "go.info.go.interface") ||
+		strings.HasPrefix(name, "go.info.go.builtin") ||
+		strings.HasPrefix(name, "go.debuglines")
+	if !whitelist {
+		l.strictDupMsgs++
+	}
+}
+
+func (l *Loader) NStrictDupMsgs() int { return l.strictDupMsgs }
+
 // Number of total symbols.
 func (l *Loader) NSym() int {
 	return int(l.max + 1)
@@ -1194,24 +1243,30 @@ func loadObjFull(l *Loader, r *oReader) {
 
 var emptyPkg = []byte(`"".`)
 
-func patchDWARFName(s *sym.Symbol, r *oReader) {
+func patchDWARFName1(p []byte, r *oReader) ([]byte, int) {
 	// This is kind of ugly. Really the package name should not
 	// even be included here.
-	if s.Size < 1 || s.P[0] != dwarf.DW_ABRV_FUNCTION {
-		return
+	if len(p) < 1 || p[0] != dwarf.DW_ABRV_FUNCTION {
+		return p, -1
 	}
-	e := bytes.IndexByte(s.P, 0)
+	e := bytes.IndexByte(p, 0)
 	if e == -1 {
-		return
+		return p, -1
 	}
-	p := bytes.Index(s.P[:e], emptyPkg)
-	if p == -1 {
-		return
+	if !bytes.Contains(p[:e], emptyPkg) {
+		return p, -1
 	}
 	pkgprefix := []byte(r.pkgprefix)
-	patched := bytes.Replace(s.P[:e], emptyPkg, pkgprefix, -1)
+	patched := bytes.Replace(p[:e], emptyPkg, pkgprefix, -1)
+	return append(patched, p[e:]...), e
+}
 
-	s.P = append(patched, s.P[e:]...)
+func patchDWARFName(s *sym.Symbol, r *oReader) {
+	patched, e := patchDWARFName1(s.P, r)
+	if e == -1 {
+		return
+	}
+	s.P = patched
 	s.Attr.Set(sym.AttrReadOnly, false)
 	delta := int64(len(s.P)) - s.Size
 	s.Size = int64(len(s.P))
diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go
index 155fd8bce3..92830fe8b3 100644
--- a/src/cmd/link/link_test.go
+++ b/src/cmd/link/link_test.go
@@ -376,3 +376,68 @@ func TestIssue34788Android386TLSSequence(t *testing.T) {
 		}
 	}
 }
+
+const testStrictDupGoSrc = `
+package main
+func f()
+func main() { f() }
+`
+
+const testStrictDupAsmSrc1 = `
+#include "textflag.h"
+TEXT	·f(SB), NOSPLIT|DUPOK, $0-0
+	RET
+`
+
+const testStrictDupAsmSrc2 = `
+#include "textflag.h"
+TEXT	·f(SB), NOSPLIT|DUPOK, $0-0
+	JMP	0(PC)
+`
+
+func TestStrictDup(t *testing.T) {
+	// Check that -strictdups flag works.
+	testenv.MustHaveGoBuild(t)
+
+	tmpdir, err := ioutil.TempDir("", "TestStrictDup")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpdir)
+
+	src := filepath.Join(tmpdir, "x.go")
+	err = ioutil.WriteFile(src, []byte(testStrictDupGoSrc), 0666)
+	if err != nil {
+		t.Fatal(err)
+	}
+	src = filepath.Join(tmpdir, "a.s")
+	err = ioutil.WriteFile(src, []byte(testStrictDupAsmSrc1), 0666)
+	if err != nil {
+		t.Fatal(err)
+	}
+	src = filepath.Join(tmpdir, "b.s")
+	err = ioutil.WriteFile(src, []byte(testStrictDupAsmSrc2), 0666)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := exec.Command(testenv.GoToolPath(t), "build", "-ldflags=-strictdups=1")
+	cmd.Dir = tmpdir
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Errorf("linking with -strictdups=1 failed: %v", err)
+	}
+	if !bytes.Contains(out, []byte("mismatched payload")) {
+		t.Errorf("unexpected output:\n%s", out)
+	}
+
+	cmd = exec.Command(testenv.GoToolPath(t), "build", "-ldflags=-strictdups=2")
+	cmd.Dir = tmpdir
+	out, err = cmd.CombinedOutput()
+	if err == nil {
+		t.Errorf("linking with -strictdups=2 did not fail")
+	}
+	if !bytes.Contains(out, []byte("mismatched payload")) {
+		t.Errorf("unexpected output:\n%s", out)
+	}
+}