Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(269)

Issue 87580043: code review 87580043: cmd/objdump: rewrite in Go (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 2 months ago by rsc
Modified:
7 years, 2 months ago
Reviewers:
r, minux1, iant
CC:
golang-codereviews, minux1
Visibility:
Public.

Description

cmd/objdump: rewrite in Go Update cmd/dist not to build the C version. Update cmd/go to install the Go version to the tool directory. Update issue 7452 This is the basic logic needed for objdump, and it works well enough to support the pprof list and weblist commands. A real disassembler needs to be added in order to support the pprof disasm command and the per-line assembly displays in weblist. That's still to come. Probably objdump will move to go.tools when the disassembler is added, but it can stay here for now.

Patch Set 1 #

Patch Set 2 : diff -r 77578375f623 https://code.google.com/p/go/ #

Patch Set 3 : diff -r 77578375f623 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 77578375f623 https://code.google.com/p/go/ #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+60 lines, -114 lines) Patch
M src/cmd/dist/build.c View 1 2 chunks +0 lines, -2 lines 0 comments Download
M src/cmd/go/pkg.go View 1 1 chunk +1 line, -0 lines 0 comments Download
R src/cmd/objdump/main.c View 1 1 chunk +0 lines, -68 lines 0 comments Download
M src/cmd/objdump/main.go View 1 5 chunks +59 lines, -44 lines 5 comments Download

Messages

Total messages: 15
rsc
Hello golang-codereviews@googlegroups.com (cc: iant, r), I'd like you to review this change to https://code.google.com/p/go/
7 years, 2 months ago (2014-04-14 14:00:42 UTC) #1
minux1
So can we remove libmach now? Do you have plan to get the disassembler into ...
7 years, 2 months ago (2014-04-14 14:20:01 UTC) #2
rsc
On Mon, Apr 14, 2014 at 10:20 AM, minux <minux.ma@gmail.com> wrote: > So can we ...
7 years, 2 months ago (2014-04-14 14:38:02 UTC) #3
minux1
LGTM.
7 years, 2 months ago (2014-04-14 14:47:10 UTC) #4
rsc
iant, r, if you have any comments i'm happy to apply them in a new ...
7 years, 2 months ago (2014-04-14 14:55:48 UTC) #5
bradfitz
If we're losing pprof disasm for Go 1.3, we should change misc/pprof to be less ...
7 years, 2 months ago (2014-04-14 14:58:44 UTC) #6
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=3aa17be30dba *** cmd/objdump: rewrite in Go Update cmd/dist not to build the ...
7 years, 2 months ago (2014-04-14 14:58:53 UTC) #7
rsc
On Mon, Apr 14, 2014 at 10:58 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote: > If we're losing ...
7 years, 2 months ago (2014-04-14 15:00:51 UTC) #8
iant
LGTM https://codereview.appspot.com/87580043/diff/60001/src/cmd/objdump/main.go File src/cmd/objdump/main.go (right): https://codereview.appspot.com/87580043/diff/60001/src/cmd/objdump/main.go#newcode110 src/cmd/objdump/main.go:110: textData, _ = sect.Data() Don't ignore the error ...
7 years, 2 months ago (2014-04-14 15:39:55 UTC) #9
rsc
https://codereview.appspot.com/87580043/diff/60001/src/cmd/objdump/main.go File src/cmd/objdump/main.go (right): https://codereview.appspot.com/87580043/diff/60001/src/cmd/objdump/main.go#newcode110 src/cmd/objdump/main.go:110: textData, _ = sect.Data() On 2014/04/14 15:39:56, iant wrote: ...
7 years, 2 months ago (2014-04-14 16:39:15 UTC) #10
iant
On Mon, Apr 14, 2014 at 9:39 AM, <rsc@golang.org> wrote: > > https://codereview.appspot.com/87580043/diff/60001/src/cmd/objdump/main.go > File ...
7 years, 2 months ago (2014-04-14 16:43:40 UTC) #11
rsc
Flush only indexes into it if the bounds are okay. (There's a test against the ...
7 years, 2 months ago (2014-04-14 16:47:00 UTC) #12
iant
On Mon, Apr 14, 2014 at 9:47 AM, <rsc@golang.org> wrote: > Flush only indexes into ...
7 years, 2 months ago (2014-04-14 17:05:01 UTC) #13
r
https://codereview.appspot.com/87580043/diff/60001/src/cmd/objdump/main.go File src/cmd/objdump/main.go (right): https://codereview.appspot.com/87580043/diff/60001/src/cmd/objdump/main.go#newcode24 src/cmd/objdump/main.go:24: fmt.Fprintf(w, "disassembles binary from start PC to end PC.\n") ...
7 years, 2 months ago (2014-04-15 19:41:19 UTC) #14
rsc
7 years, 2 months ago (2014-04-15 19:53:05 UTC) #15
Sent CL 88050046 with more docs.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b