code review 40600043: cmd/nm: reimplement in Go

cmd/nm: reimplement in Go

The immediate goal is to support the new object file format,
which libmach (nm's support library) does not understand.
Rather than add code to libmach or reengineer liblink to
support this new use, just write it in Go.

The C version of nm reads the Plan 9 symbol table stored in
Go binaries, now otherwise unused.

This reimplementation uses the standard symbol table for
the corresponding file format instead, bringing us one step
closer to removing the Plan 9 symbol table from Go binaries.

Tell cmd/dist not to build cmd/nm anymore.
Tell cmd/go to install cmd/nm in the tool directory.

[rsc, 2013-12-11 02:59:21]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[r, 2013-12-11 03:43:12]

https://codereview.appspot.com/40600043/diff/80001/src/cmd/nm/nm.go
File src/cmd/nm/nm.go (right):

https://codereview.appspot.com/40600043/diff/80001/src/cmd/nm/nm.go#newcode25
src/cmd/nm/nm.go:25: printType  = flag.Bool("T", false, "print type")
since we have no need to be compatible, and the tool is becoming incompatible,
i'd prefer more expressive names for the flags: -type, -size,
-sort={none,addr,name}, etc.

https://codereview.appspot.com/40600043/diff/80001/src/cmd/nm/pe.go
File src/cmd/nm/pe.go (right):

https://codereview.appspot.com/40600043/diff/80001/src/cmd/nm/pe.go#newcode7
src/cmd/nm/pe.go:7: import (
each format-parsing file could use a top-level comment saying what it handles.
yes, this is PE, but what version and what is it? etc.

[rsc, 2013-12-11 18:13:18]

PTAL

https://codereview.appspot.com/40600043/diff/80001/src/cmd/nm/nm.go
File src/cmd/nm/nm.go (right):

https://codereview.appspot.com/40600043/diff/80001/src/cmd/nm/nm.go#newcode25
src/cmd/nm/nm.go:25: printType  = flag.Bool("T", false, "print type")
On 2013/12/11 03:43:12, r wrote:
> since we have no need to be compatible, and the tool is becoming incompatible,
> i'd prefer more expressive names for the flags: -type, -size,
> -sort={none,addr,name}, etc.

Done.

https://codereview.appspot.com/40600043/diff/80001/src/cmd/nm/pe.go
File src/cmd/nm/pe.go (right):

https://codereview.appspot.com/40600043/diff/80001/src/cmd/nm/pe.go#newcode7
src/cmd/nm/pe.go:7: import (
On 2013/12/11 03:43:12, r wrote:
> each format-parsing file could use a top-level comment saying what it handles.
> yes, this is PE, but what version and what is it? etc.

There aren't really versions. I added a comment to each file saying what
operating system the file is found on.

[r, 2013-12-11 18:22:03]

LGTM

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/goobj.go
File src/cmd/nm/goobj.go (right):

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/goobj.go#newcode5
src/cmd/nm/goobj.go:5: package main
// Parsing of Go intermediate files generated by the compiler.

[iant, 2013-12-11 23:25:43]

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/elf.go
File src/cmd/nm/elf.go (right):

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/elf.go#newcode45
src/cmd/nm/elf.go:45: sym.Code = 'R'
The code 'R' is not documented in doc.go.

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/elf.go#newcode49
src/cmd/nm/elf.go:49: }
if elf.ST_BIND(sym.Info) == elf.STB_LOCAL {
    sym.Code += 'a' - 'A'
}

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/goobj.go
File src/cmd/nm/goobj.go (right):

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/goobj.go#newco...
src/cmd/nm/goobj.go:43: sym.Code = 'X'
The code 'X' is not documented in doc.go.

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/nm.go
File src/cmd/nm/nm.go (right):

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/nm.go#newcode140
src/cmd/nm/nm.go:140: fmt.Fprintf(w, "%8s", "")
When the current nm program is given more than one file on the command line, it
prints the file name before each symbol line.  Should this program act the same
way?  The current behaviour is unhelpful since there is no indication of when
one file ends and another begins.

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/nm.go#newcode156
src/cmd/nm/nm.go:156: func filter(syms []Sym, ok func(Sym) bool) []Sym {
I don't think anything uses this.

[brainman, 2013-12-11 23:58:03]

I am probably wrong but ...

Your new command is vastly different from the old command on windows. Your new
nm command displays "pe symbols". We have 4 of those. As far as I remember, one
of those points onto "go symbols" section. The old nm command displays the
later.

We had a plan at some stage in the past to copy "go symbols" into "pe symbols",
so all "standard" GNU commands will be able to interrogate our binaries. But we
never implemented it, because nm did a good job of it.

Alex

[rsc, 2013-12-11 23:58:04]

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/nm.go
File src/cmd/nm/nm.go (right):

https://codereview.appspot.com/40600043/diff/130001/src/cmd/nm/nm.go#newcode140
src/cmd/nm/nm.go:140: fmt.Fprintf(w, "%8s", "")
On 2013/12/11 23:25:43, iant wrote:
> When the current nm program is given more than one file on the command line,
it
> prints the file name before each symbol line.  Should this program act the
same
> way?  The current behaviour is unhelpful since there is no indication of when
> one file ends and another begins.

Perhaps it should. If so it should print the archive name instead of the member
name. I'll try that.

[brainman, 2013-12-12 00:03:29]

On 2013/12/11 23:58:03, brainman wrote:

That is what I see:

C:\go\root\src\pkg\archive\zip>go tool nm zip.test.exe     <- old nm command
  ...
  ...
  ...
  55fcb8 D unicode/utf8.gcargs┬╖16
  57e860 D unicode/utf8.gcargs┬╖18
  574b00 D unicode/utf8.gcargs┬╖2
  570ec0 D unicode/utf8.gcargs┬╖20
  566720 D unicode/utf8.gcargs┬╖22
  566740 D unicode/utf8.gcargs┬╖4
  566760 D unicode/utf8.gcargs┬╖8
  567300 D unicode/utf8.gclocals┬╖1
  55fcc8 D unicode/utf8.gclocals┬╖11
  574b40 D unicode/utf8.gclocals┬╖13
  574b80 D unicode/utf8.gclocals┬╖15
  55c310 D unicode/utf8.gclocals┬╖17
  574f40 D unicode/utf8.gclocals┬╖19
  570f00 D unicode/utf8.gclocals┬╖21
  566780 D unicode/utf8.gclocals┬╖23
  567320 D unicode/utf8.gclocals┬╖3
  55fcd8 D unicode/utf8.gclocals┬╖5
  55fce8 D unicode/utf8.gclocals┬╖9
  6b3e48 d unknown
  416bd0 t unlockOSThread
  414920 t unlockextra
  40d200 t updatememstats
  6bc102 b use_aeshash
  41c420 t vneg
  412d20 t vprintf
  414e40 t wakep
  6bc9a0 b work

C:\go\root\src\pkg\archive\zip>go tool nm zip.test.exe     <- new nm command
  2a43d5 T epclntab
  239d52 T esymtab
  239d60 T pclntab
  1a0900 T symtab

C:\go\root\src\pkg\archive\zip>

[rsc, 2013-12-12 00:13:42]

On Wed, Dec 11, 2013 at 3:58 PM, <alex.brainman@gmail.com> wrote:

> I am probably wrong but ...
>
> Your new command is vastly different from the old command on windows.
> Your new nm command displays "pe symbols". We have 4 of those. As far as
> I remember, one of those points onto "go symbols" section. The old nm
> command displays the later.
>
> We had a plan at some stage in the past to copy "go symbols" into "pe
> symbols", so all "standard" GNU commands will be able to interrogate our
> binaries. But we never implemented it, because nm did a good job of it.
>

Perhaps this will be a good motivation to make that work. :-)

The change here is intentional, and mentioned in the CL description. The
goal is to eliminate the uses of the Plan 9 symbol table so that we are not
carrying two.

It is probably okay for now if nm only shows those 4 symbols on Windows. It
may be easier to fill out the Windows symbol table once the linker is
written in Go (golang.org/s/go13linker).

I created golang.org/issue/6936 so we don't forget this. Thanks.

Russ

[r, 2013-12-12 00:18:47]

Once symtab goes away, it will show only 3 symbols. Now that's progress.

-rob

[rsc, 2013-12-16 17:52:14]

*** Submitted as https://code.google.com/p/go/source/detail?r=547e140f5c3c ***

cmd/nm: reimplement in Go

The immediate goal is to support the new object file format,
which libmach (nm's support library) does not understand.
Rather than add code to libmach or reengineer liblink to
support this new use, just write it in Go.

The C version of nm reads the Plan 9 symbol table stored in
Go binaries, now otherwise unused.

This reimplementation uses the standard symbol table for
the corresponding file format instead, bringing us one step
closer to removing the Plan 9 symbol table from Go binaries.

Tell cmd/dist not to build cmd/nm anymore.
Tell cmd/go to install cmd/nm in the tool directory.

R=golang-dev, r, iant, alex.brainman
CC=golang-dev
https://codereview.appspot.com/40600043