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: map lookup of boolean, then converted to interface{}, causes parameter smash #19275

Closed
dr2chase opened this issue Feb 24, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@dr2chase
Copy link
Contributor

commented Feb 24, 2017

Please answer these questions before submitting your issue. Thanks!

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

I isolated the failing commit:

drchase@drchase01:/tmp$ go version
go version devel +8958d8ce37 Thu Feb 2 21:06:28 2017 +0000 linux/amd64

I.e., "cmd/compile: skip convT2E for empty structs"

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

linux/amd64

The test works in the playground, but fails using go devel at 8958d8c or later.
https://play.golang.org/p/8idlRKU3Q8

drchase@drchase01:/tmp$ go run bad.go
FAIL, *n should be 99, not 8

In the generated code from GOSSAFUNC='(*SessionsInfo).test' go run bad.go
notice how &n is moved to (SP) at line 22, then smashed at line 27 before the call to runtime.mapaccess1_faststr on line 32.

   	00000 (/tmp/bad.go:16)	TEXT	"".(*SI).test(SB)
   	00001 (/tmp/bad.go:16)	FUNCDATA	$0, "".gcargs·0(SB)
   	00002 (/tmp/bad.go:16)	FUNCDATA	$1, "".gclocals·1(SB)
   	00003 (/tmp/bad.go:16)	TYPE	s(FP), type.*"".SI(SB)
   	00004 (/tmp/bad.go:16)	TYPE	name+8(FP), type.string(SB)
   	00005 (/tmp/bad.go:16)	TYPE	~r1+24(FP), type.*int(SB)
   	00006 (/tmp/bad.go:16)	TYPE	~r2+32(FP), type.error(SB)
   	00007 (/tmp/bad.go:16)	TYPE	.autotmp_2-32(SP), type.[2]interface {}(SB)
   	00008 (/tmp/bad.go:16)	TYPE	.autotmp_0-48(SP), type.[1]string(SB)
   	00009 (/tmp/bad.go:16)	TYPE	.autotmp_13-56(SP), type.*uint8(SB)
   	00010 (/tmp/bad.go:16)	TYPE	err.data-64(SP), type.*uint8(SB)
   	00011 (/tmp/bad.go:16)	TYPE	n-72(SP), type.*int(SB)
v84	00012 (/tmp/bad.go:17)	LEAQ	type.int(SB), AX
v14	00013 (/tmp/bad.go:17)	MOVQ	AX, (SP)
v15	00014 (/tmp/bad.go:17)	CALL	runtime.newobject(SB)
v17	00015 (/tmp/bad.go:17)	MOVQ	8(SP), AX
v138	00016 (/tmp/bad.go:17)	MOVQ	AX, "".n-72(SP)
v20	00017 (/tmp/bad.go:18)	MOVQ	$99, (AX)
v30	00018 (/tmp/bad.go:19)	MOVQ	"".statictmp_6+8(SB), CX
v34	00019 (/tmp/bad.go:19)	MOVQ	"".statictmp_6(SB), DX
v162	00020 (/tmp/bad.go:19)	MOVQ	DX, ""..autotmp_0-48(SP)
v25	00021 (/tmp/bad.go:19)	MOVQ	CX, ""..autotmp_0-40(SP)
v37	00022 (/tmp/bad.go:19)	MOVQ	AX, (SP)
v115	00023 (/tmp/bad.go:19)	MOVQ	"".s(FP), CX
v43	00024 (/tmp/bad.go:19)	MOVQ	(CX), DX
v45	00025 (/tmp/bad.go:19)	MOVQ	DX, 8(SP)
v159	00026 (/tmp/bad.go:19)	LEAQ	type.map[string]*"".PI(SB), DX
v48	00027 (/tmp/bad.go:19)	MOVQ	DX, (SP)
v158	00028 (/tmp/bad.go:19)	MOVQ	"".name+8(FP), BX
v31	00029 (/tmp/bad.go:19)	MOVQ	BX, 16(SP)
v154	00030 (/tmp/bad.go:19)	MOVQ	"".name+16(FP), SI
v51	00031 (/tmp/bad.go:19)	MOVQ	SI, 24(SP)
v52	00032 (/tmp/bad.go:19)	CALL	runtime.mapaccess1_faststr(SB)
v54	00033 (/tmp/bad.go:19)	MOVQ	32(SP), AX
v55	00034 (/tmp/bad.go:19)	MOVQ	(AX), AX
v70	00035 (/tmp/bad.go:19)	MOVBLZX	(AX), AX
v42	00036 (/tmp/bad.go:19)	LEAQ	type.bool(SB), CX
v78	00037 (/tmp/bad.go:19)	MOVQ	CX, 8(SP)
v90	00038 (/tmp/bad.go:19)	LEAQ	runtime.staticbytes(SB), DX
v62	00039 (/tmp/bad.go:19)	ADDQ	DX, AX
v65	00040 (/tmp/bad.go:19)	MOVQ	AX, 16(SP)
v123	00041 (/tmp/bad.go:19)	LEAQ	""..autotmp_0-48(SP), AX
v119	00042 (/tmp/bad.go:19)	MOVQ	AX, 24(SP)
v59	00043 (/tmp/bad.go:19)	MOVQ	$1, 32(SP)
v67	00044 (/tmp/bad.go:19)	MOVQ	$1, 40(SP)
v68	00045 (/tmp/bad.go:19)	CALL	"".addUpdate(SB)

Here is another failing program believed to have the same cause (similar boolean-to-interface conversion, failure introduced at same commit): https://play.golang.org/p/-Rus8Xs1Ze

@ianlancetaylor ianlancetaylor changed the title Map lookup of boolean, then converted to interface{}, causes parameter smash cmd/compile: map lookup of boolean, then converted to interface{}, causes parameter smash Feb 24, 2017

@ianlancetaylor ianlancetaylor added this to the Go1.9 milestone Feb 24, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Feb 24, 2017

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

@gopherbot gopherbot closed this in febafe6 Feb 25, 2017

@golang golang locked and limited conversation to collaborators Feb 25, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.