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: 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
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@aarzilli
Copy link
Contributor

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 commented Nov 8, 2021 •
edited

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.

go/src/cmd/compile/internal/typecheck/subr.go

Line 163 in 9e6ad46

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

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 Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 9, 2021
@cagedmantis cagedmantis added this to the Backlog milestone Nov 9, 2021
@cagedmantis
Copy link
Contributor

/cc @randall77 @griesemer

@gopherbot
Copy link

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
@dr2chase
cmd/compile: add line number test for #49436
a096316 
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>
@aarzilli aarzilli mentioned this issue Nov 11, 2021
Closed
@golang golang locked and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@cagedmantis @aarzilli @gopherbot @hanchaoqun