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.

Sign up for GitHub

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

x/tools/go/ssa: multiple assign ssa.FieldAddr instr order error #55086

Closed
visualfc opened this issue Sep 15, 2022 · 4 comments
Closed

x/tools/go/ssa: multiple assign ssa.FieldAddr instr order error #55086

visualfc opened this issue Sep 15, 2022 · 4 comments
Assignees
@timothy-king
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@visualfc
Copy link

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

$ go version
go version go1.18.4 darwin/amd64

Does this issue reproduce with the latest release?

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

go env Output
$ go env

go version -m $(which ssadump) Output
$ go version -m $(which ssadump)
/Users/vfc/go/bin/ssadump: go1.18.4
	path	golang.org/x/tools/cmd/ssadump
	mod	golang.org/x/tools	(devel)
	dep	golang.org/x/mod	v0.6.0-dev.0.20220419223038-86c51ed26bb4	h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
	dep	golang.org/x/sys	v0.0.0-20220722155257-8c9f86f7a55f	h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s=
	build	-compiler=gc
	build	CGO_ENABLED=1
	build	CGO_CFLAGS="-g -O2"
	build	CGO_CPPFLAGS=
	build	CGO_CXXFLAGS="-g -O2"
	build	CGO_LDFLAGS="-g -O2"
	build	GOARCH=amd64
	build	GOOS=darwin
	build	GOAMD64=v1
	build	vcs=git
	build	vcs.revision=9d7fa4a49fed984f3980240b804508eff5988360
	build	vcs.time=2022-09-05T11:27:21Z
	build	vcs.modified=false

What did you do?

main.go

package main

type P struct{ i int }

func main() {
	var m int
	var p *P
	defer func() {
		recover()
		println(m)
	}()
	m, p.i = 3, 2
}

$ ssadump -build=F main.go

func init(): ....
func main$1(): ...

# Name: command-line-arguments.main
# Package: command-line-arguments
# Location: /Users/vfc/code/vtest/main.go:5:6
# Recover: 1
func main():
0:                                                                entry P:0 S:0
	t0 = new int (m)                                                   *int
	t1 = make closure main$1 [t0]                                    func()
	defer t1()
	t2 = &nil:*P.i [#0]                                                *int
	*t0 = 3:int
	*t2 = 2:int
	rundefers
	return
1:                                                              recover P:0 S:0
	return

What did you expect to see?

the func main() instr order *t0 = 3:int before of t2 = &nil:*P.i [#0]
run line m, p.i = 3, 2 p.i is panic, but befor m is set, result is 3.

	*t0 = 3:int
	t2 = &nil:*P.i [#0]                                                *int
	*t2 = 2:int

What did you see instead?

the the func main() instr order error, the *ssa.FieldAddr &nil:*P.i [#0] before of *t0 = 3:int.
run line m, p.i = 3, 2 p.i is panic, m not set, result is 0.

	t2 = &nil:*P.i [#0]                                                *int
	*t0 = 3:int
	*t2 = 2:int
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 15, 2022
@gopherbot gopherbot added this to the Unreleased milestone Sep 15, 2022
@timothy-king
Copy link
Contributor

https://go.dev/play/p/5Wwf8VHN0lD does print 3.

AFAICT I believe printing both 0 and 3 are legal outputs according to the spec https://go.dev/ref/spec#Order_of_evaluation . I think this is most analogous to:

However, the order of those events compared to the evaluation and indexing of x and the evaluation of y is not specified.

@mdempsky are you aware whether both orders are legal? Or is only one?

@mdempsky
Copy link
Member

mdempsky commented Sep 15, 2022 •
edited

@timothy-king It has to print 3. The other relevant clause here is in "Assignment statements":

The assignment proceeds in two phases. First, the operands of index expressions and pointer indirections (including implicit pointer indirections in selectors) on the left and the expressions on the right are all evaluated in the usual order. Second, the assignments are carried out in left-to-right order.

So the expressions p (as an operand of a pointer indirection) and 3 and 2 (as expressions on the right) are all evaluated in phase 1 in an unspecified order (unspecified because there aren't any calls or receive expressions).

Then in phase 2, the assignments m = (3) and then (nil).i = (2) happen sequentially.

@timothy-king
Copy link
Contributor

Thanks. This is a bug then.

@timothy-king timothy-king self-assigned this Sep 15, 2022
@timothy-king timothy-king added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 15, 2022
@gopherbot
Copy link

Change https://go.dev/cl/442655 mentions this issue: go/ssa: emit field lvals on demand

@golang golang locked and limited conversation to collaborators Oct 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@timothy-king timothy-king

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@mdempsky @visualfc @timothy-king @gopherbot