runtime: crash running x = x #8097
Labels
Milestone
Comments
Yep, definitely a bug. arr is not marked as live at the GC call. There's something
strange going on with the VARDEF declarations. Almost like the compiler thinks there
are two independent arr variables. It even copies arr onto itself at the end. I'll
keep looking.
liveness: foo
bb#0 pred= succ=
00151 (issue8097.go:18) TEXT "".foo+0(SB),$0-0
live=
00152 (issue8097.go:18) FUNCDATA $2,"".gcargs·2+0(SB)
00153 (issue8097.go:18) FUNCDATA $3,"".gclocals·3+0(SB)
00154 (issue8097.go:18) TYPE "".arr+0(FP),$24
00156 (issue8097.go:18) VARDEF ,"".arr+0(FP)
varkill=arr
00157 (issue8097.go:18) VARDEF ,"".arr+0(FP)
varkill=arr
00161 (issue8097.go:18) VARDEF ,"".arr+0(FP)
varkill=arr
00162 (issue8097.go:18) VARDEF ,"".arr+0(FP)
varkill=arr
00166 (issue8097.go:19) MOVQ $type.[1]int+0(SB),(SP)
00167 (issue8097.go:19) PCDATA $0,$16
00200 (issue8097.go:19) PCDATA $1,$0
00168 (issue8097.go:19) CALL ,runtime.new+0(SB)
live=
00169 (issue8097.go:19) PCDATA $0,$-1
00170 (issue8097.go:19) MOVQ 8(SP),AX
00173 (issue8097.go:19) NOP ,
00174 (issue8097.go:19) MOVQ "".statictmp_0012+0(SB),BP
00175 (issue8097.go:19) MOVQ BP,(AX)
00178 (issue8097.go:19) NOP ,
00179 (issue8097.go:19) VARDEF ,"".arr+0(FP)
varkill=arr
00181 (issue8097.go:19) MOVQ AX,"".arr+0(FP)
00182 (issue8097.go:19) MOVQ $1,"".arr+8(FP)
00183 (issue8097.go:19) MOVQ $1,"".arr+16(FP)
00184 (issue8097.go:21) PCDATA $0,$0
00199 (issue8097.go:21) PCDATA $1,$0
00185 (issue8097.go:21) CALL ,runtime.GC+0(SB)
live=
00186 (issue8097.go:21) PCDATA $0,$-1
00187 (issue8097.go:23) VARDEF ,"".arr+0(FP)
varkill=arr
00188 (issue8097.go:23) MOVQ "".arr+0(FP),BX
uevar=arr
00189 (issue8097.go:23) MOVQ BX,"".arr+0(FP)
00190 (issue8097.go:23) MOVQ "".arr+8(FP),BX
uevar=arr
00191 (issue8097.go:23) MOVQ BX,"".arr+8(FP)
00192 (issue8097.go:23) MOVQ "".arr+16(FP),BX
uevar=arr
00193 (issue8097.go:23) MOVQ BX,"".arr+16(FP)
00194 (issue8097.go:23) RET ,
uevar=arr
end
varkill=arr
Owner changed to @randall77. |
So it looks like the compiler thinks it is inserting a VARDEF in the middle of this
assignment:
arr = arr
but there is no "middle" here, so we get
VARDEF arr
arr = arr
so the compiler assumes that arr is dead preceding the VARDEF, which isn't true in this
case.
We need to either
1) detect x=x assignments and skip them.
2) convert x=x to tmp=x;x=tmp
I've hacked up the former (just to skip inserting the VARDEF) for slices, CL 92610046.
It fixes the bug, but there's a side-effect: the "variable recorded live on entry" check
triggers in some other parts of the standard library. I'll track that down - I'll bet
some other part of the compiler assume x=x assignments have a VARDEF in them.
|
Am looking into this. Owner changed to @rsc. |
CL https://golang.org/cl/102820043 mentions this issue. |
|
This issue was closed by revision 948b2c7. Status changed to Fixed. |
|
This issue was closed by revision 9dd062b. |
CL https://golang.org/cl/103750043 mentions this issue. |
|
This issue was closed by revision 89d46fe. Status changed to Fixed. |
wheatman
pushed a commit
to wheatman/go-akaros
that referenced
this issue
Jun 25, 2018
The 'nodarg' function is used to obtain a Node* representing a function argument or result. It returned a brand new Node*, but that violates the guarantee in most places in the compiler that two Node*s refer to the same variable if and only if they are the same Node* pointer. Reestablish that invariant by making nodarg return a preexisting named variable if present. Having fixed that, avoid any copy during x=x in componentgen, because the VARDEF we emit before the copy marks the lhs x as dead incorrectly. The change in walk.c avoids modifying the result of nodarg. This was the only place in the compiler that did so. Fixes golang#8097. LGTM=r, khr R=golang-codereviews, r, khr CC=golang-codereviews, iant https://golang.org/cl/102820043
wheatman
pushed a commit
to wheatman/go-akaros
that referenced
this issue
Jun 25, 2018
Breaks 386 and arm builds. The obvious reason is that this CL only edited 6g/gsubr.c and failed to edit 5g/gsubr.c and 8g/gsubr.c. However, the obvious CL applying the same edit to those files (CL 101900043) causes mysterious build failures in various of the standard package tests, usually involving reflect. Something deep and subtle is broken but only on the 32-bit systems. Undo this CL for now. ««« original CL description cmd/gc: fix x=x crash The 'nodarg' function is used to obtain a Node* representing a function argument or result. It returned a brand new Node*, but that violates the guarantee in most places in the compiler that two Node*s refer to the same variable if and only if they are the same Node* pointer. Reestablish that invariant by making nodarg return a preexisting named variable if present. Having fixed that, avoid any copy during x=x in componentgen, because the VARDEF we emit before the copy marks the lhs x as dead incorrectly. The change in walk.c avoids modifying the result of nodarg. This was the only place in the compiler that did so. Fixes golang#8097. LGTM=r, khr R=golang-codereviews, r, khr CC=golang-codereviews, iant https://golang.org/cl/102820043 »»» TBR=r CC=golang-codereviews, khr https://golang.org/cl/95660043
wheatman
pushed a commit
to wheatman/go-akaros
that referenced
this issue
Jun 25, 2018
[Same as CL 102820043 except applied changes to 6g/gsubr.c also to 5g/gsubr.c and 8g/gsubr.c. The problem I had last night trying to do that was that 8g's copy of nodarg has different (but equivalent) control flow and I was pasting the new code into the wrong place.] Description from CL 102820043: The 'nodarg' function is used to obtain a Node* representing a function argument or result. It returned a brand new Node*, but that violates the guarantee in most places in the compiler that two Node*s refer to the same variable if and only if they are the same Node* pointer. Reestablish that invariant by making nodarg return a preexisting named variable if present. Having fixed that, avoid any copy during x=x in componentgen, because the VARDEF we emit before the copy marks the lhs x as dead incorrectly. The change in walk.c avoids modifying the result of nodarg. This was the only place in the compiler that did so. Fixes golang#8097. LGTM=khr R=golang-codereviews, khr CC=golang-codereviews, iant, khr, r https://golang.org/cl/103750043
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
go version go1.3beta2 linux/amd64 and go1.3beta2 +77632b0a1c94 Sun May 25 08:38:59 2014 +1000 This test code doesn't work on go1.3beta2: package gc_test import ( "fmt" "runtime" "testing" ) func TestGc(t *testing.T) { fmt.Println(runtime.Version()) arr := foo() if arr[0] != 1 { t.Fatalf("want 1, got %d", arr[0]) } } func foo() (arr []int) { arr = []int{1} runtime.GC() return arr } result: go test go1.3beta2 --- FAIL: TestGc (0.00 seconds) gc_test.go:15: want 1, got 833357874160 FAIL The array object `arr` was garbage collected. When I used the go1.2.2, this test code worked fine. I noticed that when I removed the named result `arr` on func `foo`, this test code passed: func foo() []int { arr := []int{1} runtime.GC() return arr } Thanks.The text was updated successfully, but these errors were encountered: