From b7717e46340b7ffe5fd53313f10dfa85a141f77a Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Tue, 2 Jun 2020 14:52:16 -0400 Subject: [PATCH] Revert "cmd/internal/goobj: add index to symbol name for indexed symbols" This reverts CL 229246. For new indexed object files, in CL 229246 we added symbol index to tools (nm, objdump) output. This affects external tools that parse those outputs. And the added index doesn't look very nice. In this release we take it out. For future releases we may introduce a flag to tools (nm, objdump) and optionally dump the symbol index. For refererenced (not defined) indexed symbols, currently the symbol is still referenced only by index, not by name. The next CL will make the object file self-contained, so tools can dump the symbol names properly (as before). For #38875. Change-Id: I07375e85a8e826e15c82fa452d11f0eaf8535a00 Reviewed-on: https://go-review.googlesource.com/c/go/+/236167 Reviewed-by: Than McIntosh Reviewed-by: Jeremy Faller --- src/cmd/internal/goobj/goobj_test.go | 19 +++++-------------- src/cmd/internal/goobj/readnew.go | 9 +-------- src/cmd/nm/nm_test.go | 10 +--------- src/cmd/objdump/objdump_test.go | 12 ++++-------- 4 files changed, 11 insertions(+), 39 deletions(-) diff --git a/src/cmd/internal/goobj/goobj_test.go b/src/cmd/internal/goobj/goobj_test.go index 9f827c6a32..4a4d35a413 100644 --- a/src/cmd/internal/goobj/goobj_test.go +++ b/src/cmd/internal/goobj/goobj_test.go @@ -17,7 +17,6 @@ import ( "os/exec" "path/filepath" "runtime" - "strings" "testing" ) @@ -151,14 +150,6 @@ func buildGoobj() error { return nil } -// Check that a symbol has a given name, accepting both -// new and old objects. -// TODO(go115newobj): remove. -func matchSymName(symname, want string) bool { - return symname == want || - strings.HasPrefix(symname, want+"#") // new style, with index -} - func TestParseGoobj(t *testing.T) { path := go1obj @@ -177,7 +168,7 @@ func TestParseGoobj(t *testing.T) { } var found bool for _, s := range p.Syms { - if matchSymName(s.Name, "mypkg.go1") { + if s.Name == "mypkg.go1" { found = true break } @@ -206,10 +197,10 @@ func TestParseArchive(t *testing.T) { var found1 bool var found2 bool for _, s := range p.Syms { - if matchSymName(s.Name, "mypkg.go1") { + if s.Name == "mypkg.go1" { found1 = true } - if matchSymName(s.Name, "mypkg.go2") { + if s.Name == "mypkg.go2" { found2 = true } } @@ -242,10 +233,10 @@ func TestParseCGOArchive(t *testing.T) { var found1 bool var found2 bool for _, s := range p.Syms { - if matchSymName(s.Name, "mycgo.go1") { + if s.Name == "mycgo.go1" { found1 = true } - if matchSymName(s.Name, "mycgo.go2") { + if s.Name == "mycgo.go2" { found2 = true } } diff --git a/src/cmd/internal/goobj/readnew.go b/src/cmd/internal/goobj/readnew.go index 3e710576b6..0b89034287 100644 --- a/src/cmd/internal/goobj/readnew.go +++ b/src/cmd/internal/goobj/readnew.go @@ -58,10 +58,8 @@ func (r *objReader) readNew() { case goobj2.PkgIdxSelf: i = int(s.SymIdx) default: - // Symbol from other package, referenced by index. - // We don't know the name. Use index. pkg := pkglist[p] - return SymID{fmt.Sprintf("%s.#%d", pkg, s.SymIdx), 0} + return SymID{fmt.Sprintf("%s.<#%d>", pkg, s.SymIdx), 0} } sym := rr.Sym(i) return SymID{sym.Name(rr), abiToVer(sym.ABI())} @@ -72,7 +70,6 @@ func (r *objReader) readNew() { // Symbols pcdataBase := start + rr.PcdataBase() n := rr.NSym() + rr.NNonpkgdef() + rr.NNonpkgref() - npkgdef := rr.NSym() ndef := rr.NSym() + rr.NNonpkgdef() for i := 0; i < n; i++ { osym := rr.Sym(i) @@ -83,10 +80,6 @@ func (r *objReader) readNew() { // prefix for the package in which the object file has been found. // Expand it. name := strings.ReplaceAll(osym.Name(rr), `"".`, r.pkgprefix) - if i < npkgdef { - // Indexed symbol. Attach index to the name. - name += fmt.Sprintf("#%d", i) - } symID := SymID{Name: name, Version: abiToVer(osym.ABI())} r.p.SymRefs = append(r.p.SymRefs, symID) diff --git a/src/cmd/nm/nm_test.go b/src/cmd/nm/nm_test.go index a49423b212..5d7fff0f99 100644 --- a/src/cmd/nm/nm_test.go +++ b/src/cmd/nm/nm_test.go @@ -315,7 +315,7 @@ func testGoLib(t *testing.T, iscgo bool) { } for i := range syms { sym := &syms[i] - if sym.Type == typ && matchSymName(name, sym.Name) && sym.CSym == csym { + if sym.Type == typ && sym.Name == name && sym.CSym == csym { if sym.Found { t.Fatalf("duplicate symbol %s %s", sym.Type, sym.Name) } @@ -334,14 +334,6 @@ func TestGoLib(t *testing.T) { testGoLib(t, false) } -// Check that a symbol has a given name, accepting both -// new and old objects. -// TODO(go115newobj): remove. -func matchSymName(symname, want string) bool { - return symname == want || - strings.HasPrefix(symname, want+"#") // new style, with index -} - const testexec = ` package main diff --git a/src/cmd/objdump/objdump_test.go b/src/cmd/objdump/objdump_test.go index 814cbf4564..c974d6707b 100644 --- a/src/cmd/objdump/objdump_test.go +++ b/src/cmd/objdump/objdump_test.go @@ -14,7 +14,6 @@ import ( "os" "os/exec" "path/filepath" - "regexp" "runtime" "strings" "testing" @@ -285,11 +284,9 @@ func TestDisasmGoobj(t *testing.T) { if err != nil { t.Fatalf("go tool compile fmthello.go: %v\n%s", err, out) } - - // TODO(go115newobj): drop old object file support. need := []string{ - `main(#\d+)?\(SB\)`, // either new or old object file - `fmthello\.go:6`, + "main(SB)", + "fmthello.go:6", } args = []string{ @@ -305,9 +302,8 @@ func TestDisasmGoobj(t *testing.T) { text := string(out) ok := true for _, s := range need { - re := regexp.MustCompile(s) - if !re.MatchString(text) { - t.Errorf("disassembly missing %q", s) + if !strings.Contains(text, s) { + t.Errorf("disassembly missing '%s'", s) ok = false } }