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

Jump to bottom

cmd/compile: increased line number churn #20367

Closed
aarzilli opened this issue May 15, 2017 · 5 comments
Closed

cmd/compile: increased line number churn #20367

aarzilli opened this issue May 15, 2017 · 5 comments
Labels
FrozenDueToAge

Comments

@aarzilli
Copy link
Contributor

aarzilli commented May 15, 2017 •
edited

This problem has always existed with the SSA backend and my impression is that it got better during go1.9 development windows, however a recent commit, 00263a8, while trying to improve the situation actually made some things worse (although I'm sure it did actually improve numbering in other circumstances). For example in this program the function main.helloworld went from compiling (with -gcflags='-N -l') to:

TEXT main.helloworld(SB) /home/a/n/go/src/github.com/derekparker/delve/_fixtures/testnextprog.go
  testnextprog.go:13	0x481b20		64488b0c25f8ffffff	MOVQ FS:0xfffffff8, CX			
  testnextprog.go:13	0x481b29		483b6110		CMPQ 0x10(CX), SP			
  testnextprog.go:13	0x481b2d		0f8689000000		JBE 0x481bbc				
  testnextprog.go:13	0x481b33		4883ec68		SUBQ $0x68, SP				
  testnextprog.go:13	0x481b37		48896c2460		MOVQ BP, 0x60(SP)			
  testnextprog.go:13	0x481b3c		488d6c2460		LEAQ 0x60(SP), BP			
  testnextprog.go:14	0x481b41		48c744243800000000	MOVQ $0x0, 0x38(SP)			
  testnextprog.go:14	0x481b4a		48c744244000000000	MOVQ $0x0, 0x40(SP)			
  testnextprog.go:14	0x481b53		488d442438		LEAQ 0x38(SP), AX			
  testnextprog.go:14	0x481b58		4889442430		MOVQ AX, 0x30(SP)			
  testnextprog.go:14	0x481b5d		8400			TESTB AL, 0(AX)				
  testnextprog.go:14	0x481b5f		488d057aed0000		LEAQ 0xed7a(IP), AX			
  testnextprog.go:14	0x481b66		4889442438		MOVQ AX, 0x38(SP)			
  testnextprog.go:14	0x481b6b		488d05bec30300		LEAQ 0x3c3be(IP), AX			
  testnextprog.go:14	0x481b72		4889442440		MOVQ AX, 0x40(SP)			
  testnextprog.go:14	0x481b77		488b442430		MOVQ 0x30(SP), AX			
  testnextprog.go:14	0x481b7c		8400			TESTB AL, 0(AX)				
  testnextprog.go:14	0x481b7e		eb00			JMP 0x481b80				
  testnextprog.go:14	0x481b80		4889442448		MOVQ AX, 0x48(SP)			
  testnextprog.go:14	0x481b85		48c744245001000000	MOVQ $0x1, 0x50(SP)			
  testnextprog.go:14	0x481b8e		48c744245801000000	MOVQ $0x1, 0x58(SP)			
  testnextprog.go:14	0x481b97		48890424		MOVQ AX, 0(SP)				
  testnextprog.go:14	0x481b9b		48c744240801000000	MOVQ $0x1, 0x8(SP)			
  testnextprog.go:14	0x481ba4		48c744241001000000	MOVQ $0x1, 0x10(SP)			
  testnextprog.go:14	0x481bad		e8fe8dffff		CALL fmt.Println(SB)			
  testnextprog.go:15	0x481bb2		488b6c2460		MOVQ 0x60(SP), BP			
  testnextprog.go:15	0x481bb7		4883c468		ADDQ $0x68, SP				
  testnextprog.go:15	0x481bbb		c3			RET					
  testnextprog.go:13	0x481bbc		e86fd2fcff		CALL runtime.morestack_noctxt(SB)	
  testnextprog.go:13	0x481bc1		e95affffff		JMP main.helloworld(SB)

To compiling to:

TEXT main.helloworld(SB) /home/a/n/go/src/github.com/derekparker/delve/_fixtures/testnextprog.go
  testnextprog.go:13	0x481b20		64488b0c25f8ffffff	MOVQ FS:0xfffffff8, CX			
  testnextprog.go:13	0x481b29		483b6110		CMPQ 0x10(CX), SP			
  testnextprog.go:13	0x481b2d		0f8689000000		JBE 0x481bbc				
  testnextprog.go:13	0x481b33		4883ec68		SUBQ $0x68, SP				
  testnextprog.go:13	0x481b37		48896c2460		MOVQ BP, 0x60(SP)			
  testnextprog.go:13	0x481b3c		488d6c2460		LEAQ 0x60(SP), BP			
  testnextprog.go:14	0x481b41		48c744243800000000	MOVQ $0x0, 0x38(SP)			
  testnextprog.go:14	0x481b4a		48c744244000000000	MOVQ $0x0, 0x40(SP)			
  testnextprog.go:14	0x481b53		488d442438		LEAQ 0x38(SP), AX			
  testnextprog.go:14	0x481b58		4889442430		MOVQ AX, 0x30(SP)			
  testnextprog.go:14	0x481b5d		8400			TESTB AL, 0(AX)				
  testnextprog.go:13	0x481b5f		488d057aed0000		LEAQ 0xed7a(IP), AX			
  testnextprog.go:14	0x481b66		4889442438		MOVQ AX, 0x38(SP)			
  testnextprog.go:14	0x481b6b		488d059ec00300		LEAQ 0x3c09e(IP), AX			
  testnextprog.go:14	0x481b72		4889442440		MOVQ AX, 0x40(SP)			
  testnextprog.go:14	0x481b77		488b442430		MOVQ 0x30(SP), AX			
  testnextprog.go:14	0x481b7c		8400			TESTB AL, 0(AX)				
  testnextprog.go:14	0x481b7e		eb00			JMP 0x481b80				
  testnextprog.go:14	0x481b80		4889442448		MOVQ AX, 0x48(SP)			
  testnextprog.go:14	0x481b85		48c744245001000000	MOVQ $0x1, 0x50(SP)			
  testnextprog.go:14	0x481b8e		48c744245801000000	MOVQ $0x1, 0x58(SP)			
  testnextprog.go:14	0x481b97		48890424		MOVQ AX, 0(SP)				
  testnextprog.go:14	0x481b9b		48c744240801000000	MOVQ $0x1, 0x8(SP)			
  testnextprog.go:14	0x481ba4		48c744241001000000	MOVQ $0x1, 0x10(SP)			
  testnextprog.go:14	0x481bad		e8fe8dffff		CALL fmt.Println(SB)			
  testnextprog.go:15	0x481bb2		488b6c2460		MOVQ 0x60(SP), BP			
  testnextprog.go:15	0x481bb7		4883c468		ADDQ $0x68, SP				
  testnextprog.go:15	0x481bbb		c3			RET					
  testnextprog.go:13	0x481bbc		e86fd2fcff		CALL runtime.morestack_noctxt(SB)	
  testnextprog.go:13	0x481bc1		e95affffff		JMP main.helloworld(SB)

Note the line number of the LEA at 0x481b5f, there multiple examples of this happening in delve's test set.

cc @dr2chase
@josharian josharian changed the title cmd/compile: bad instruction line numbering cmd/compile: increased line number churn May 15, 2017
@dr2chase
Copy link
Contributor

Working on it, the problem comes from a poor combination of positioning "SB" (and "SP" and initial memory) and not positioning the LEAQ itself, which causes it to inherit the predecessor (SB) line number.

Minimal fix is to use src.NoXPos entryNewValue0, was wondering if it might not make more sense to use that for all the entryNewValueX generators. The reason for doing the larger-scope "fix" is that each of the entryNewValueX calls will stuff a "random" line number into the values in the entry Block (i.e., nonsensical for debugging), though the values so stuffed are all of the sort that get sunk down closer to their uses, which may or may not match their definitions.

@randall77
Copy link
Contributor

It certainly makes sense to use src.NoXPos for things like OpSP.
It's less clear for things like function addresses, zero values, etc. where they are associated with an original source line. Things that go in entry are always very CSEable so we might lose the line number anyway, but for the common case of, say:

var g int
func f() *int {
  return &g
}

It would be nice if the LEAQ had the line number of the return instruction, not the func declaration.
Now that I check, we currently don't get the line number of the LEAQ right anyway. Hmmm....

@josharian
Copy link
Contributor

Things that go in entry are always very CSEable so we might lose the line number anyway

Hmm. Tangent, but when we CSE values, which line number do we pick? Should we switch to NoXPos when we CSE two values from different lines?

@randall77
Copy link
Contributor

CSE chooses a value which dominates all the others in its equivalence class. Within a block, it is an arbitrary choice.

@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators May 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@josharian @aarzilli @dr2chase @randall77 @gopherbot