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

Issue 91360046: code review 91360046: objdump: implement disassembly (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 1 month ago by rsc
Modified:
7 years, 1 month ago
CC:
crawshaw, iant, golang-codereviews
Visibility:
Public.

Description

objdump: implement disassembly There is some duplication here with cmd/nm. There is a TODO to address that after 1.3 is out. Update issue 7452 x86 disassembly works and is tested. The arm disassembler does not exist yet and is therefore not yet hooked up.

Patch Set 1 #

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

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

Total comments: 12

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+360 lines, -63 lines) Patch
M src/cmd/objdump/Makefile View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/objdump/elf.go View 1 2 chunks +13 lines, -5 lines 0 comments Download
M src/cmd/objdump/macho.go View 1 3 chunks +13 lines, -5 lines 0 comments Download
M src/cmd/objdump/main.go View 1 2 3 8 chunks +226 lines, -27 lines 0 comments Download
M src/cmd/objdump/objdump_test.go View 1 2 chunks +86 lines, -12 lines 0 comments Download
M src/cmd/objdump/pe.go View 1 4 chunks +8 lines, -7 lines 0 comments Download
M src/cmd/objdump/plan9obj.go View 1 3 chunks +6 lines, -6 lines 0 comments Download
A src/cmd/objdump/testdata/fmthello.go View 1 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 10
rsc
Hello crawshaw (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
7 years, 1 month ago (2014-05-14 18:33:50 UTC) #1
crawshaw
LGTM https://codereview.appspot.com/91360046/diff/40001/src/cmd/objdump/main.go File src/cmd/objdump/main.go (right): https://codereview.appspot.com/91360046/diff/40001/src/cmd/objdump/main.go#newcode97 src/cmd/objdump/main.go:97: keep := syms[:0] // reuse memory for filtering ...
7 years, 1 month ago (2014-05-14 19:30:02 UTC) #2
iant
LGTM https://codereview.appspot.com/91360046/diff/40001/src/cmd/objdump/main.go File src/cmd/objdump/main.go (right): https://codereview.appspot.com/91360046/diff/40001/src/cmd/objdump/main.go#newcode26 src/cmd/objdump/main.go:26: // The disassembler is missing (golang.org/issue/7452) but will ...
7 years, 1 month ago (2014-05-14 21:33:47 UTC) #3
rsc
https://codereview.appspot.com/91360046/diff/40001/src/cmd/objdump/testdata/fmthello.go File src/cmd/objdump/testdata/fmthello.go (right): https://codereview.appspot.com/91360046/diff/40001/src/cmd/objdump/testdata/fmthello.go#newcode1 src/cmd/objdump/testdata/fmthello.go:1: package main On 2014/05/14 21:33:47, iant wrote: > What ...
7 years, 1 month ago (2014-05-14 23:43:17 UTC) #4
rsc
https://codereview.appspot.com/91360046/diff/40001/src/cmd/objdump/main.go File src/cmd/objdump/main.go (right): https://codereview.appspot.com/91360046/diff/40001/src/cmd/objdump/main.go#newcode26 src/cmd/objdump/main.go:26: // The disassembler is missing (golang.org/issue/7452) but will be ...
7 years, 1 month ago (2014-05-14 23:50:34 UTC) #5
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=712f6b41d382 *** objdump: implement disassembly There is some duplication here with cmd/nm. ...
7 years, 1 month ago (2014-05-14 23:51:07 UTC) #6
gobot
This CL appears to have broken the freebsd-amd64 builder. See http://build.golang.org/log/40a55cc6aeb4477cb2f6c227eecca64c833bbcc3
7 years, 1 month ago (2014-05-15 00:00:04 UTC) #7
bradfitz
Real. On May 14, 2014 5:00 PM, <gobot@golang.org> wrote: > This CL appears to have ...
7 years, 1 month ago (2014-05-15 00:02:32 UTC) #8
brainman
On 2014/05/15 00:02:32, bradfitz wrote: > Real. I suspect start of text pe section is ...
7 years, 1 month ago (2014-05-15 01:49:24 UTC) #9
brainman
7 years, 1 month ago (2014-05-15 02:42:22 UTC) #10
Message was sent while issue was closed.
On 2014/05/15 01:49:24, brainman wrote:
> On 2014/05/15 00:02:32, bradfitz wrote:
> > Real.
> 
> I suspect start of text pe section is wrong. I am investigating.
> 

https://codereview.appspot.com/97500043/

Alex
Sign in to reply to this message.

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