code review 5697066: runtime/pprof: support OS X CPU profiling

runtime/pprof: support OS X CPU profiling

Work around profiling kernel bug with signal masks.
Still broken on 64-bit Snow Leopard kernel,
but I think we can ignore that one and let people
upgrade to Lion.

Add new trivial tools addr2line and objdump to take
the place of the GNU tools of the same name, since
those are not installed on OS X.

Adapt pprof to invoke 'go tool addr2line' and
'go tool objdump' if the system tools do not exist.

Clean up disassembly of base register on amd64.

Fixes issue 2008.

[rsc, 2012-02-26 20:01:16]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2012-02-27 00:45:56]

LGTM

Didn't review the addr2line and objdump parts incredibly deeply, though.

On Mon, Feb 27, 2012 at 7:01 AM, <rsc@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> runtime/pprof: support OS X CPU profiling
>
> Work around profiling kernel bug with signal masks.
> Still broken on 64-bit Snow Leopard kernel,
> but I think we can ignore that one and let people
> upgrade to Lion.
>
> Add new trivial tools addr2line and objdump to take
> the place of the GNU tools of the same name, since
> those are not installed on OS X.
>
> Adapt pprof to invoke 'go tool addr2line' and
> 'go tool objdump' if the system tools do not exist.
>
> Clean up disassembly of base register on amd64.
>
> Fixes issue 2008.
>
> Please review this at
http://codereview.appspot.com/**5697066/<http://codereview.appspot.com/5697066/>
>
> Affected files:
>  M misc/pprof
>  A src/cmd/addr2line/main.c
>  M src/cmd/dist/build.c
>  A src/cmd/objdump/main.c
>  M src/libmach/8db.c
>  M src/pkg/runtime/lock_futex.c
>  M src/pkg/runtime/lock_sema.c
>  M src/pkg/runtime/os_darwin.h
>  M src/pkg/runtime/os_freebsd.h
>  M src/pkg/runtime/os_netbsd.h
>  M src/pkg/runtime/os_openbsd.h
>  M src/pkg/runtime/pprof/pprof.go
>  M src/pkg/runtime/pprof/pprof_**test.go
>  M src/pkg/runtime/proc.c
>  M src/pkg/runtime/runtime.h
>  M src/pkg/runtime/signal_darwin_**386.c
>  M src/pkg/runtime/signal_darwin_**amd64.c
>  M src/pkg/runtime/signal_unix.c
>  M src/pkg/runtime/sys_darwin_**386.s
>  M src/pkg/runtime/sys_darwin_**amd64.s
>  M src/pkg/runtime/thread_darwin.**c
>  M src/pkg/runtime/thread_linux.c
>  M src/pkg/runtime/thread_netbsd.**c
>  M src/pkg/runtime/thread_**openbsd.c
>  M src/pkg/runtime/thread_plan9.c
>  M src/pkg/runtime/thread_**windows.c
>
>
>

[mikio, 2012-02-27 01:43:21]

freebsd requires runtime.setprof too.

[r, 2012-02-28 10:45:24]

LGTM (modulo freebsd)

http://codereview.appspot.com/5697066/diff/4001/src/pkg/runtime/pprof/pprof_t...
File src/pkg/runtime/pprof/pprof_test.go (right):

http://codereview.appspot.com/5697066/diff/4001/src/pkg/runtime/pprof/pprof_t...
src/pkg/runtime/pprof/pprof_test.go:27: if strings.Contains(vers, "Darwin Kernel
Version 10.8.0") && strings.Contains(vers,
"root:xnu-1504.15.3~1/RELEASE_X86_64") {
confusing.
i'm running 10.6.8 with kernel 10.8.0. go figure.

http://codereview.appspot.com/5697066/diff/4001/src/pkg/runtime/runtime.h
File src/pkg/runtime/runtime.h (right):

http://codereview.appspot.com/5697066/diff/4001/src/pkg/runtime/runtime.h#new...
src/pkg/runtime/runtime.h:734: // If appropriate, ask operating system to
control whether this
s/ask/ask the/

[iant, 2012-02-28 19:09:50]

LGTM

http://codereview.appspot.com/5697066/diff/4001/src/cmd/addr2line/main.c
File src/cmd/addr2line/main.c (right):

http://codereview.appspot.com/5697066/diff/4001/src/cmd/addr2line/main.c#newc...
src/cmd/addr2line/main.c:18: fprint(2, "reads symbols from standard input and
writes two lines for each:\n");
s/symbols/addresses/

http://codereview.appspot.com/5697066/diff/4001/src/pkg/runtime/signal_unix.c
File src/pkg/runtime/signal_unix.c (right):

http://codereview.appspot.com/5697066/diff/4001/src/pkg/runtime/signal_unix.c...
src/pkg/runtime/signal_unix.c:62: runtime·setprof(false);
This is a change on all non-OS-X systems.  Previously we would explicitly ignore
SIGPROF.  Now we do nothing.  I can't think of anything that could go wrong but
I want to make sure this change is intentional.

[rsc, 2012-02-28 19:49:35]

On Tue, Feb 28, 2012 at 14:09,  <iant@golang.org> wrote:
> This is a change on all non-OS-X systems.  Previously we would
> explicitly ignore SIGPROF.  Now we do nothing.  I can't think of
> anything that could go wrong but I want to make sure this change is
> intentional.

Yes, it was just to simplify the back and forth in the signal handlers.
This whole function seems a little sketchy - I am not sure why we
contrive to call setitimer for each thread when I thought it applied to
the whole process, and the same for the signal handlers - but at least
this one seemed worth simplifying.

(There are some notes in the perftools source code about per-thread
setitimer but I think those went away a long time ago.)

Russ

[rsc, 2012-02-28 21:18:28]

*** Submitted as http://code.google.com/p/go/source/detail?r=08554467526f ***

runtime/pprof: support OS X CPU profiling

Work around profiling kernel bug with signal masks.
Still broken on 64-bit Snow Leopard kernel,
but I think we can ignore that one and let people
upgrade to Lion.

Add new trivial tools addr2line and objdump to take
the place of the GNU tools of the same name, since
those are not installed on OS X.

Adapt pprof to invoke 'go tool addr2line' and
'go tool objdump' if the system tools do not exist.

Clean up disassembly of base register on amd64.

Fixes issue 2008.

R=golang-dev, bradfitz, mikioh.mikioh, r, iant
CC=golang-dev
http://codereview.appspot.com/5697066