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: wrong pcln entry for instruction #49436

Closed
aarzilli opened this issue Nov 8, 2021 · 4 comments
Closed

cmd/compile: wrong pcln entry for instruction #49436

aarzilli opened this issue Nov 8, 2021 · 4 comments
Labels
NeedsInvestigation
Milestone

Comments

@aarzilli
Copy link
Contributor

@aarzilli aarzilli commented Nov 8, 2021

go version devel go1.18-9e6ad46bcc Sun Nov 7 04:57:22 2021 +0000 linux/amd64

Given https://play.golang.org/p/wsCMMD9Jk9J compiled with -gcflags='-N -l' the method PushBack compiles to:

TEXT main.(*List[go.shape.int_0]).PushBack(SB) ./buggy.go
...
  buggy.go:22		0x454a90		488b00			MOVQ 0(AX), AX					
  buggy.go:31		0x454a93		8400			TESTB AL, 0(AX)					
  buggy.go:22		0x454a95		c6400801		MOVB $0x1, 0x8(AX)				
...

The instruction 0x454a93 gets assigned to a line outside of the PushBack function, in this case it's the call to PushBack but in more complex examples can be something completely unrelated. Can't make this happen without embedding and type parameters.

cc @dr2chase

@hanchaoqun
Copy link
Contributor

@hanchaoqun hanchaoqun commented Nov 8, 2021

The error seems occurred in the stenciling, when transformDot is doing the XDOT to DOT conversion, the NewSelectorExpr called in AddImplicitDots passed the wrong Pos parameter, that should use n.X.Pos instead of base.Pos n.Pos instead of base.Pos.

dot := ir.NewSelectorExpr(base.Pos, ir.ODOT, n.X, path[c].field.Sym)

after noder2 (*List["".List.PushBack.V]).PushBack
...
.   .   .   AS tc(1) # pcln.go:22:22
.   .   .   .   XDOT main.Circular bool tc(1) # pcln.go:22:12
.   .   .   .   .   XDOT main.root PTR-*ExtNode["".List.PushBack.V] tc(1) # pcln.go:22:7
.   .   .   .   .   .   NAME-main.list Class:PPARAM Offset:0 OnStack PTR-*List["".List.PushBack.V] tc(1) # pcln.go:19:7
.   .   .   .   LITERAL-true bool tc(1) # pcln.go:22:24
...
stenciled (*List[%2eshape.int_0]).PushBack
...
.   .   .   AS tc(1) # pcln.go:22:22
.   .   .   .   DOT main.Circular bool tc(1) # pcln.go:22:12
.   .   .   .   .   DOTPTR main.Node Implicit main.Node tc(1) # pcln.go:31:2
.   .   .   .   .   .   DOTPTR main.root PTR-*ExtNode[%2eshape.int_0] tc(1) # pcln.go:22:7
.   .   .   .   .   .   .   NAME-main.list Class:PPARAM DictIndex:2 Offset:0 OnStack Used PTR-*List[%2eshape.int_0] tc(1) # pcln.go:19:7
.   .   .   .   LITERAL-true bool tc(1) # pcln.go:22:24
...

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 8, 2021

Change https://golang.org/cl/361917 mentions this issue: cmd/compile: NewSelectorExpr used use n.X.Pos instead of base.Pos

@cagedmantis cagedmantis added the NeedsInvestigation label Nov 9, 2021
@cagedmantis cagedmantis added this to the Backlog milestone Nov 9, 2021
@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented Nov 9, 2021

/cc @randall77 @griesemer

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 9, 2021

Change https://golang.org/cl/362714 mentions this issue: cmd/compile: add line number test for #49436

gopherbot pushed a commit that referenced this issue Nov 9, 2021
This enhances the existing line number test
to allow a specific -gcflags (e.g., -G=3)
and to permit ignoring duplicate line numbers
(which is arguably a bug, but not THIS bug,
and it lowers the risk of a flaky test).

Limited to Linux/Darwin and amd64/arm64,
also tests with "unified" mangling.

And, using these new powers, adds a test.

Updates #49436.

Change-Id: I09c82e6a08d53edd5a752522a827e872d3e16e0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/362714
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

4 participants