]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.link] cmd/link: restore -strictdups flag in newobj mode
authorCherry Zhang <cherryyz@google.com>
Sat, 2 Nov 2019 04:38:21 +0000 (00:38 -0400)
committerCherry Zhang <cherryyz@google.com>
Sun, 3 Nov 2019 20:19:00 +0000 (20:19 +0000)
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>
src/cmd/link/internal/ld/lib.go
src/cmd/link/internal/loader/loader.go
src/cmd/link/link_test.go

index 4c7451a11448720d285bb796c8278b06e6a482a2..39ed53e38c5b54592d0395eaefeb3c444acf4559 100644 (file)
@@ -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.
index 46d93c512472ca010bc86a52959f3791a4434602..c0fa5fa7ce2f708d08c75a60b61481b78ed69929 100644 (file)
@@ -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))
index 155fd8bce3b305afb8b7193338bfee99f2ac39fc..92830fe8b3f9fe938c85b5b3c74a04a5f9670031 100644 (file)
@@ -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)
+       }
+}