From: Cherry Zhang Date: Sat, 2 Nov 2019 04:38:21 +0000 (-0400) Subject: [dev.link] cmd/link: restore -strictdups flag in newobj mode X-Git-Tag: go1.14beta1~292^2^2~1 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=db75205a9b8c4d630fef38aa88ddb66a9b63f487;p=gostls13.git [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 TryBot-Result: Gobot Gobot Reviewed-by: Than McIntosh --- 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) + } +}