code review 117920043: cmd/addr2line, cmd/objdump: fix on amd64 Plan 9

cmd/addr2line, cmd/objdump: fix on amd64 Plan 9

Fix virtual address of the start of the text segment
on amd64 Plan 9.

This issue has been partially fixed in cmd/add2line,
as part of CL 106460044, but we forgot to report the
change to cmd/objdump.

In the meantime, we also fixed the textStart address
in both cmd/add2line and cmd/objdump.

[0intro, 2014-07-18 09:15:12]

Hello rsc (cc: ality, aram, golang-codereviews@googlegroups.com, jas, mischief),

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

[mischief, 2014-07-18 09:30:18]

this is is a small improvement, as now the file:lineno shows up. but the symbols
are still busted.

bmo% go test -v
=== RUN TestObjDump
--- PASS: TestObjDump (8.12s)
=== RUN TestDisasm
--- FAIL: TestDisasm (10.69s)
	objdump_test.go:189: disassembly missing 'JMP main.main(SB)'
	objdump_test.go:189: disassembly missing 'CALL fmt.Println(SB)'
	objdump_test.go:194: full disassembly:
		TEXT main.main(SB) /sys/src/go/src/cmd/objdump/testdata/fmthello.go
			fmthello.go:5	0x200030	0000			ADDL AL, 0(AX)
			fmthello.go:5	0x200032	0000			ADDL AL, 0(AX)
			fmthello.go:5	0x200034	0000			ADDL AL, 0(AX)
			fmthello.go:5	0x200036	0000			ADDL AL, 0(AX)
			fmthello.go:5	0x200038	488b0c25e85c4000	MOVQ _privates(SB), CX
			fmthello.go:5	0x200040	488b8900000000		MOVQ 0(CX), CX
			fmthello.go:5	0x200047	483b21			CMPQ 0(CX), SP
			fmthello.go:5	0x20004a	7707			JA 0x200053
			fmthello.go:5	0x20004c	e8b7960200		CALL 0x229708
			fmthello.go:6	0x200051	ebe5			JMP 0x200038
			fmthello.go:6	0x200053	4883ec70		SUBQ $0x70, SP
			fmthello.go:6	0x200057	488d1c2540172d00	LEAQ go.string.*+35104(SB), BX
			fmthello.go:6	0x20005f	488b2b			MOVQ 0(BX), BP
			fmthello.go:6	0x200062	48896c2438		MOVQ BP, 0x38(SP)
			fmthello.go:6	0x200067	488b6b08		MOVQ 0x8(BX), BP
			fmthello.go:6	0x20006b	48896c2440		MOVQ BP, 0x40(SP)
			fmthello.go:6	0x200070	488d7c2448		LEAQ 0x48(SP), DI
			fmthello.go:6	0x200075	31c0			XORL AX, AX
			fmthello.go:6	0x200077	48ab			STOSQ AX, ES:0(DI)
			fmthello.go:6	0x200079	48ab			STOSQ AX, ES:0(DI)
			fmthello.go:6	0x20007b	488d5c2448		LEAQ 0x48(SP), BX
			fmthello.go:6	0x200080	4883fb00		CMPQ $0x0, BX
			fmthello.go:6	0x200084	7464			JE 0x2000ea
			fmthello.go:6	0x200086	48895c2458		MOVQ BX, 0x58(SP)
			fmthello.go:6	0x20008b	48c744246001000000	MOVQ $0x1, 0x60(SP)
			fmthello.go:6	0x200094	48c744246801000000	MOVQ $0x1, 0x68(SP)
			fmthello.go:6	0x20009d	48c70424408e2900	MOVQ $type.*+74272(SB), 0(SP)
			fmthello.go:6	0x2000a5	488d5c2438		LEAQ 0x38(SP), BX
			fmthello.go:6	0x2000aa	48895c2408		MOVQ BX, 0x8(SP)
			fmthello.go:6	0x2000af	e8b4280200		CALL 0x222968
			fmthello.go:6	0x2000b4	488b442458		MOVQ 0x58(SP), AX
			fmthello.go:6	0x2000b9	488d5c2410		LEAQ 0x10(SP), BX
			fmthello.go:6	0x2000be	4889c7			MOVQ AX, DI
			fmthello.go:6	0x2000c1	4889de			MOVQ BX, SI
			fmthello.go:6	0x2000c4	48a5			MOVSQ DS:0(SI), ES:0(DI)
			fmthello.go:6	0x2000c6	48a5			MOVSQ DS:0(SI), ES:0(DI)
			fmthello.go:6	0x2000c8	48890424		MOVQ AX, 0(SP)
			fmthello.go:6	0x2000cc	488b5c2460		MOVQ 0x60(SP), BX
			fmthello.go:6	0x2000d1	48895c2408		MOVQ BX, 0x8(SP)
			fmthello.go:6	0x2000d6	488b5c2468		MOVQ 0x68(SP), BX
			fmthello.go:6	0x2000db	48895c2410		MOVQ BX, 0x10(SP)
			fmthello.go:7	0x2000e0	e8c3e40200		CALL 0x22e5a8
			fmthello.go:6	0x2000e5	4883c470		ADDQ $0x70, SP
			fmthello.go:6	0x2000e9	c3			RET
			fmthello.go:6	0x2000ea	8903			MOVL AX, 0(BX)
			fmthello.go:6	0x2000ec	eb98			JMP 0x200086
			fmthello.go:6	0x2000ee	0000			ADDL AL, 0(AX)
FAIL
exit status: 'objdump.test 18124: 1'
FAIL	cmd/objdump	18.899s
bmo%

[aram, 2014-07-18 10:13:00]

LGTM

[ality, 2014-07-18 23:51:50]

LGTM after changes noted below

https://codereview.appspot.com/117920043/diff/40001/src/cmd/objdump/main.go
File src/cmd/objdump/main.go (right):

https://codereview.appspot.com/117920043/diff/40001/src/cmd/objdump/main.go#n...
src/cmd/objdump/main.go:368: sym, err := findPlan9Symbol(obj, "text")
Delete these four lines and make textStart := obj.LoadAddress + obj.HdrSize
to fix the problem that mischief reported.

https://codereview.appspot.com/117920043/diff/40001/src/cmd/objdump/main.go#n...
src/cmd/objdump/main.go:455: return data[ssym.Value-(f.LoadAddress+f.HdrSize) :
esym.Value-(f.LoadAddress+f.HdrSize)], nil
Please make this

    textStart := f.LoadAddress + f.HdrSize
    return data[ssym.Value-textStart : esym.Value-textStart]

for uniformity.

[0intro, 2014-07-19 00:12:04]

Thanks. I've also reported the changes to cmd/addr2line.

https://codereview.appspot.com/117920043/diff/40001/src/cmd/objdump/main.go
File src/cmd/objdump/main.go (right):

https://codereview.appspot.com/117920043/diff/40001/src/cmd/objdump/main.go#n...
src/cmd/objdump/main.go:368: sym, err := findPlan9Symbol(obj, "text")
On 2014/07/18 23:51:50, ality wrote:
> Delete these four lines and make textStart := obj.LoadAddress + obj.HdrSize
> to fix the problem that mischief reported.

Done.

https://codereview.appspot.com/117920043/diff/40001/src/cmd/objdump/main.go#n...
src/cmd/objdump/main.go:455: return data[ssym.Value-(f.LoadAddress+f.HdrSize) :
esym.Value-(f.LoadAddress+f.HdrSize)], nil
On 2014/07/18 23:51:50, ality wrote:
> Please make this
> 
>     textStart := f.LoadAddress + f.HdrSize
>     return data[ssym.Value-textStart : esym.Value-textStart]
> 
> for uniformity.

Done.

[ality, 2014-07-19 00:39:24]

0intro@gmail.com once said:
> Thanks. I've also reported the changes to cmd/addr2line.

No problem. I forgot to mention that the problem is only
visible on amd64 because functions are aligned to 16 bytes
and the extra eight bytes in the 64-bit a.out file header
causes a gap in the beginning of the text section.

Cheers,
  Anthony

[mischief, 2014-07-19 07:34:07]

LGTM

cmd/objdump is passing now

[aram, 2014-07-19 09:15:14]

LGTM

[aram, 2014-07-19 09:16:40]

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

cmd/addr2line, cmd/objdump: fix on amd64 Plan 9

Fix virtual address of the start of the text segment
on amd64 Plan 9.

This issue has been partially fixed in cmd/add2line,
as part of CL 106460044, but we forgot to report the
change to cmd/objdump.

In the meantime, we also fixed the textStart address
in both cmd/add2line and cmd/objdump.

LGTM=aram, ality, mischief
R=rsc, mischief, aram, ality
CC=golang-codereviews, jas
https://codereview.appspot.com/117920043

Committer: Aram Hăvărneanu <aram@mgk.ro>