Issue 106460044: code review 106460044: debug/plan9obj, cmd/addr2line: on Plan 9 use a.out header

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 106460044: code review 106460044: debug/plan9obj, cmd/addr2line: on Plan 9 use a.out header (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
10 years, 10 months ago by aram

Modified:
10 years, 9 months ago

Reviewers:
gobot, 0intro

CC:
0intro, ality, golang-codereviews, jas, mischief_9.offblast.org

Visibility:
Public.

Description

debug/plan9obj, cmd/addr2line: on Plan 9 use a.out header size instead of abusing text symbol cmd/addr2line needs to know the virtual address of the start of the text segment (load address plus header size). For this, it used the text symbol added by the linker. This is wrong on amd64. Header size is 40 bytes, not 32 like on 386 and arm. Function alignment is 16 bytes causing text to be at 0x200030. debug/plan9obj now exports both the load address and the header size; cmd/addr2line uses this new information and doesn't rely on text anymore.

Patch Set 1 #

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

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

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

Patch Set 5 : diff -r 0918ba0c24ac https://code.google.com/p/go #

Created: 10 years, 9 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+16 lines, -17 lines)			Patch
	M	src/cmd/addr2line/main.go	View	1	2 chunks	+1 line, -5 lines	0 comments	Download
	M	src/pkg/debug/plan9obj/file.go	View	1	3 chunks	+15 lines, -12 lines	0 comments	Download

Messages

Total messages: 6

Expand All Messages | Collapse All Messages

aram

Hello 0intro (cc: ality, golang-codereviews@googlegroups.com, jas, mischief@9.offblast.org), I'd like you to review this change to ...

10 years, 10 months ago (2014-07-07 18:10:39 UTC) #1

aram

*** Submitted as https://code.google.com/p/go/source/detail?r=ba5b861c474f *** debug/plan9obj, cmd/addr2line: on Plan 9 use a.out header size instead ...

10 years, 9 months ago (2014-07-09 10:33:23 UTC) #4

gobot

This CL appears to have broken the linux-amd64-gce builder. See http://build.golang.org/log/a9bb175233f9489b31ec9e04f684a23ad7fbf451

10 years, 9 months ago (2014-07-09 10:45:21 UTC) #5

aram

10 years, 9 months ago (2014-07-09 10:48:59 UTC) #6

On Wed, Jul 9, 2014 at 12:45 PM,  <gobot@golang.org> wrote:
> This CL appears to have broken the linux-amd64-gce builder.
> See http://build.golang.org/log/a9bb175233f9489b31ec9e04f684a23ad7fbf451
>
> https://codereview.appspot.com/106460044/

My mistake, fixed in 106560044.

-- 
Aram Hăvărneanu

Expand All Messages | Collapse All Messages