Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: line numbers attached to value spill/unspills (StoreReg/LoadReg/Copy) put line numbers in funny places #18902

Closed
dr2chase opened this issue Feb 2, 2017 · 4 comments

Comments

@dr2chase
Copy link
Contributor

dr2chase commented Feb 2, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.8

What operating system and processor architecture are you using (go env)?

darwin/amd64

What did you do?

This is extracted from a larger debugging problem to be self-contained, so it does not run.
When debugging, it can happen that when you set a breakpoint on a line, you'll actually end up stopping "somewhere else" because value unspills are tagged with the source line of the value, but may be located far from the rest of the code for that line.

https://play.golang.org/p/nyZdpigPAa

This can be seen in the assembly language output, either from go build -gcflags -S baddwarf.go:

	0x0077 00119 (.../tmp/baddwarf.go:116)	MOVQ	8(DX), R8
	0x007b 00123 (.../tmp/baddwarf.go:116)	TESTB	AL, (R8)
	0x007e 00126 (.../tmp/baddwarf.go:117)	MOVL	$1, AX
	0x0083 00131 (.../tmp/baddwarf.go:118)	MOVQ	$22, CX
	0x008a 00138 (.../tmp/baddwarf.go:121)	MOVQ	CX, "".shift+72(SP)
	0x008f 00143 (.../tmp/baddwarf.go:138)	MOVQ	R8, ""..autotmp_37+4272(SP)
	0x0097 00151 (.../tmp/baddwarf.go:142)	MOVQ	SI, ""..autotmp_38+4264(SP)
	0x009f 00159 (.../tmp/baddwarf.go:118)	TESTB	AL, AL
	0x00a1 00161 (.../tmp/baddwarf.go:118)	JEQ	$0, 1027
	0x00a7 00167 (.../tmp/baddwarf.go:120)	LEAQ	"".pos+144(SP), DI
	0x00af 00175 (.../tmp/baddwarf.go:121)	MOVQ	CX, AX
	0x00b2 00178 (.../tmp/baddwarf.go:120)	MOVQ	$512, CX
	0x00b9 00185 (.../tmp/baddwarf.go:121)	MOVQ	AX, R9
	0x00bc 00188 (.../tmp/baddwarf.go:120)	MOVQ	$0, AX
	0x00be 00190 (.../tmp/baddwarf.go:120)	REP
	0x00bf 00191 (.../tmp/baddwarf.go:120)	STOSQ

or (and this helps with spotting the cause) GOSSAFUNC='(*gcSortBuf).flush' go build baddwarf.go:

v37	00079 (.../tmp/baddwarf.go:116)	MOVQ	8(DX), R8
v38	00080 (.../tmp/baddwarf.go:116)	TESTB	AX, (R8)
v309	00081 (.../tmp/baddwarf.go:117)	MOVL	$1, AX
v299	00082 (.../tmp/baddwarf.go:118)	MOVQ	$22, CX
v175	00083 (.../tmp/baddwarf.go:121)	MOVQ	CX, "".shift-4208(SP)
v180	00084 (.../tmp/baddwarf.go:138)	MOVQ	R8, ""..autotmp_37-8(SP)
v61	00085 (.../tmp/baddwarf.go:142)	MOVQ	SI, ""..autotmp_38-16(SP)
v266	00086 (.../tmp/baddwarf.go:118)	TESTB	AX, AX
b9	00087 (.../tmp/baddwarf.go:118)	JEQ	$0, 259
v50	00088 (.../tmp/baddwarf.go:120)	VARDEF	"".pos-4136(SP)
v70	00089 (.../tmp/baddwarf.go:120)	LEAQ	"".pos-4136(SP), DI
v242	00090 (.../tmp/baddwarf.go:121)	MOVQ	CX, AX
v203	00091 (.../tmp/baddwarf.go:120)	MOVQ	$512, CX
v214	00092 (.../tmp/baddwarf.go:121)	MOVQ	AX, R9
v147	00093 (.../tmp/baddwarf.go:120)	MOVQ	$0, AX
v51	00094 (.../tmp/baddwarf.go:120)	REP
v51	00095 (.../tmp/baddwarf.go:120)	STOSQ

which can be seen to come from v175, v180, v61,v242,v214

v175 = StoreReg <uint> v401 : shift[uint]
v180 = StoreReg <*uint8> v188 : .autotmp_37[*uint8]
v61 = StoreReg <*uint8> v234 : .autotmp_38[*uint8]
v242 = Copy <uint> v401 : AX
v214 = Copy <uint> v242 : R9

It might make more sense to give these regalloc-inserted instructions the same line numbers as their predecessors or some other sort of "not really that line" annotation.

@randall77
Copy link
Contributor

Restores could be given the line of the value the restore is triggered for.
Restores during edge processing are a special case, not sure what their line should be.

@josharian josharian changed the title Line numbers attached to value spill/unspills (StoreReg/LoadReg/Copy) put line numbers in funny places cmd/compile: line numbers attached to value spill/unspills (StoreReg/LoadReg/Copy) put line numbers in funny places Feb 2, 2017
@dr2chase
Copy link
Contributor Author

dr2chase commented Feb 2, 2017

I'm trying both copying the preceding line and src.NoXPos, and both seem to give a decent gdb debugging experience. They may mean the same thing; there's no difference in the line number table sizes.

@dr2chase dr2chase self-assigned this Feb 2, 2017
@gopherbot
Copy link

CL https://golang.org/cl/36207 mentions this issue.

@bradfitz bradfitz added this to the Go1.9Maybe milestone Mar 21, 2017
@gopherbot
Copy link

Change https://golang.org/cl/74357 mentions this issue: cmd/compile: adjust expectations of test for issue 18902

gopherbot pushed a commit that referenced this issue Oct 30, 2017
The test for #18902 reads the assembly stream to be sure
that the line number does not change too often (this is an
indication that debugging the code will be unpleasant and
that the compiler is probably getting line numbers "wrong").

It checks that it is getting "enough" input, but the
compiler has gotten enough better since the test was written
that it now fails for lack of enough input.  The old
threshould was 200 instructions, the new one is 150 (the
minimum observed input is on arm64 with 184 instructions).

Fixes #22494.

Change-Id: Ibba7e9ff4ab6a7be369e5dd5859d150b7db94653
Reviewed-on: https://go-review.googlesource.com/74357
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
@golang golang locked and limited conversation to collaborators Oct 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants