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

runtime: invalid stack pointer when parsing XML with atomic test coverage #16948

Closed
iangudger opened this issue Sep 1, 2016 · 6 comments

Comments

Projects
None yet
4 participants
@iangudger
Copy link
Contributor

commented Sep 1, 2016

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

devel +f8555ea (HEAD)

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

Linux AMD64

What did you do?

The most minimal reproduction I can do tonight is these two files:

xmlparsing.go

package xmlparsing

import (
    "encoding/xml"
)

type Data struct {
}

type Packet struct {
    Serial string `xml:"serial,attr"`
    Data   []Data `xml:"data"`
}

func DecodePacket(raw []byte) (packet Packet, err error) {
    err = xml.Unmarshal(raw, &packet)
    return
}

xmlparsing.go

package xmlparsing

import (
    "testing"
)

var deltaData = `<group serial=""></group>`

func TestDecodePacket(t *testing.T) {
    DecodePacket([]byte(deltaData))
}

run go test -covermode=atomic

What did you expect to see?

$ go test -covermode=atomic
PASS
coverage: 100.0% of statements
ok      local/xmlparsing    0.012s

This is the output from Go 1.7 on the same system.

What did you see instead?

$ go test -covermode=atomic
runtime: bad pointer in frame local/xmlparsing.DecodePacket at 0xc420044f00: 0x19
fatal error: invalid stack pointer

runtime stack:
runtime.throw(0x52c7a7, 0x15)
    /go/src/runtime/panic.go:566 +0x95 fp=0x7fff66fd37f0 sp=0x7fff66fd37d0
runtime.adjustpointers(0xc420044f00, 0x7fff66fd38d8, 0x7fff66fd3c28, 0x5775c0)
    /go/src/runtime/stack.go:601 +0x269 fp=0x7fff66fd3860 sp=0x7fff66fd37f0
runtime.adjustframe(0x7fff66fd3b30, 0x7fff66fd3c28, 0x7fff66fd3901)
    /go/src/runtime/stack.go:673 +0x5af fp=0x7fff66fd3908 sp=0x7fff66fd3860
runtime.gentraceback(0xffffffffffffffff, 0xc420044d68, 0x0, 0xc420001860, 0x0, 0x0, 0x7fffffff, 0x532198, 0x7fff66fd3c28, 0x0, ...)
    /go/src/runtime/traceback.go:378 +0x109c fp=0x7fff66fd3b90 sp=0x7fff66fd3908
runtime.copystack(0xc420001860, 0x1000, 0x1)
    /go/src/runtime/stack.go:902 +0x381 fp=0x7fff66fd3d60 sp=0x7fff66fd3b90
runtime.newstack()
    /go/src/runtime/stack.go:1070 +0x35b fp=0x7fff66fd3ee0 sp=0x7fff66fd3d60
runtime.morestack()
    /go/src/runtime/asm_amd64.s:366 +0x7f fp=0x7fff66fd3ee8 sp=0x7fff66fd3ee0

goroutine 5 [copystack]:
encoding/xml.(*Decoder).unmarshal(0xc4200a02c0, 0x50e240, 0x19, 0x199, 0x0, 0x199, 0x725d4501)
    /go/src/encoding/xml/read.go:269 fp=0xc420044d70 sp=0xc420044d68
encoding/xml.(*Decoder).DecodeElement(0xc4200a02c0, 0x4f8340, 0x19, 0x0, 0x1, 0x7fd4ecb621e0)
    /go/src/encoding/xml/read.go:133 +0x10b fp=0xc420044dc0 sp=0xc420044d70
encoding/xml.(*Decoder).Decode(0xc4200a02c0, 0x4f8340, 0x19, 0xc420029e60, 0xc420012501)
    /go/src/encoding/xml/read.go:121 +0x48 fp=0xc420044e00 sp=0xc420044dc0
encoding/xml.Unmarshal(0xc42000e400, 0x19, 0x20, 0x4f8340, 0x19, 0x19, 0x20)
    /go/src/encoding/xml/read.go:115 +0x1a2 fp=0xc420044ec8 sp=0xc420044e00
local/xmlparsing.DecodePacket(0xc42000e400, 0x19, 0x20, 0x0, 0x0, 0x0, 0x0, 0x0, 0x57c7d08a, 0xc4079c941d)
    local/xmlparsing/_test/_obj_test/xmlparsing.go:19 +0xb1 fp=0xc420044f18 sp=0xc420044ec8
local/xmlparsing.TestDecodePacket(0xc42008c180)
    /go/src/local/xmlparsing/xmlparsing_test.go:10 +0x64 fp=0xc420044f78 sp=0xc420044f18
testing.tRunner(0xc42008c180, 0x5320b0)
    /go/src/testing/testing.go:610 +0x81 fp=0xc420044fa0 sp=0xc420044f78
runtime.goexit()
    /go/src/runtime/asm_amd64.s:2086 +0x1 fp=0xc420044fa8 sp=0xc420044fa0
created by testing.(*T).Run
    /go/src/testing/testing.go:646 +0x2eb

goroutine 1 [chan receive]:
runtime.gopark(0x532400, 0xc42006c298, 0x52aa91, 0xc, 0xc42003fb17, 0x3)
    /go/src/runtime/proc.go:259 +0x13a fp=0xc42003fab0 sp=0xc42003fa80
runtime.goparkunlock(0xc42006c298, 0x52aa91, 0xc, 0x5bfa17, 0x3)
    /go/src/runtime/proc.go:265 +0x5e fp=0xc42003faf0 sp=0xc42003fab0
runtime.chanrecv(0x4fdb40, 0xc42006c240, 0x0, 0xc42003fb01, 0x46851b)
    /go/src/runtime/chan.go:496 +0x2de fp=0xc42003fb78 sp=0xc42003faf0
runtime.chanrecv1(0x4fdb40, 0xc42006c240, 0x0)
    /go/src/runtime/chan.go:378 +0x35 fp=0xc42003fbb0 sp=0xc42003fb78
testing.(*T).Run(0xc42008c0c0, 0x52b53a, 0x10, 0x5320b0, 0xc42003fc98)
    /go/src/testing/testing.go:647 +0x315 fp=0xc42003fc58 sp=0xc42003fbb0
testing.RunTests.func1(0xc42008c0c0)
    /go/src/testing/testing.go:793 +0x67 fp=0xc42003fca8 sp=0xc42003fc58
testing.tRunner(0xc42008c0c0, 0xc42003fd88)
    /go/src/testing/testing.go:610 +0x81 fp=0xc42003fcd0 sp=0xc42003fca8
testing.RunTests(0x5320b8, 0x5b8a90, 0x1, 0x1, 0x40ebb0)
    /go/src/testing/testing.go:799 +0x2f5 fp=0xc42003fdb8 sp=0xc42003fcd0
testing.(*M).Run(0xc42003fee8, 0x52e718)
    /go/src/testing/testing.go:743 +0x85 fp=0xc42003fe40 sp=0xc42003fdb8
main.main()
    local/xmlparsing/_test/_testmain.go:100 +0x1bd fp=0xc42003ff48 sp=0xc42003fe40
runtime.main()
    /go/src/runtime/proc.go:183 +0x1f4 fp=0xc42003ffa0 sp=0xc42003ff48
runtime.goexit()
    /go/src/runtime/asm_amd64.s:2086 +0x1 fp=0xc42003ffa8 sp=0xc42003ffa0

goroutine 2 [force gc (idle)]:
runtime.gopark(0x532400, 0x5bf020, 0x52b2bd, 0xf, 0x532314, 0x1)
    /go/src/runtime/proc.go:259 +0x13a fp=0xc420028748 sp=0xc420028718
runtime.goparkunlock(0x5bf020, 0x52b2bd, 0xf, 0xc420000114, 0x1)
    /go/src/runtime/proc.go:265 +0x5e fp=0xc420028788 sp=0xc420028748
runtime.forcegchelper()
    /go/src/runtime/proc.go:224 +0x9e fp=0xc4200287c0 sp=0xc420028788
runtime.goexit()
    /go/src/runtime/asm_amd64.s:2086 +0x1 fp=0xc4200287c8 sp=0xc4200287c0
created by runtime.init.3
    /go/src/runtime/proc.go:213 +0x35

goroutine 3 [GC sweep wait]:
runtime.gopark(0x532400, 0x5bf200, 0x52acc4, 0xd, 0x41c314, 0x1)
    /go/src/runtime/proc.go:259 +0x13a fp=0xc420028f38 sp=0xc420028f08
runtime.goparkunlock(0x5bf200, 0x52acc4, 0xd, 0x14, 0x1)
    /go/src/runtime/proc.go:265 +0x5e fp=0xc420028f78 sp=0xc420028f38
runtime.bgsweep(0xc4200180e0)
    /go/src/runtime/mgcsweep.go:63 +0xb6 fp=0xc420028fb8 sp=0xc420028f78
runtime.goexit()
    /go/src/runtime/asm_amd64.s:2086 +0x1 fp=0xc420028fc0 sp=0xc420028fb8
created by runtime.gcenable
    /go/src/runtime/mgc.go:195 +0x61

goroutine 4 [finalizer wait]:
runtime.gopark(0x532400, 0x5d9770, 0x52afed, 0xe, 0x14, 0x1)
    /go/src/runtime/proc.go:259 +0x13a fp=0xc420029708 sp=0xc4200296d8
runtime.goparkunlock(0x5d9770, 0x52afed, 0xe, 0x14, 0x1)
    /go/src/runtime/proc.go:265 +0x5e fp=0xc420029748 sp=0xc420029708
runtime.runfinq()
    /go/src/runtime/mfinal.go:158 +0xaf fp=0xc4200297c0 sp=0xc420029748
runtime.goexit()
    /go/src/runtime/asm_amd64.s:2086 +0x1 fp=0xc4200297c8 sp=0xc4200297c0
created by runtime.createfing
    /go/src/runtime/mfinal.go:139 +0x62
exit status 2
FAIL    local/xmlparsing    0.015s

I didn't find any unsafe code in either the testing or encoding/xml packages, so this might be a memory corruption issue.

@dsnet

This comment has been minimized.

Copy link
Member

commented Sep 1, 2016

I was seeing a similar error in a number of tests I was running and hadn't been able to produce a more minimal test case yet, so I'm glad that you came up with this one.

For the record, I'm pretty sure this is unrelated to both XML parsing and the covermode.

@dsnet dsnet added this to the Go1.8 milestone Sep 1, 2016

@dsnet

This comment has been minimized.

Copy link
Member

commented Sep 1, 2016

Git bisect reports the offending commit as b2e0e96.

/cc @cherrymui @dr2chase @randall77 @josharian

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2016

It seems the problem is that the inserted atomic op (now as intrinsic, which produces <v, mem>) confuses the scheduler and makes following stores scheduled before the load of the return value of runtime.newobject.

It was not a problem before this commit because the extra Zero op produces a mem, which happens to enforce the load is scheduled early. This Zero op is removed in this commit.

I am working on it.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 1, 2016

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

@gopherbot gopherbot closed this in 1c53a1b Sep 1, 2016

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2016

fatal error: invalid stack pointer

BTW, I feel that this error message is not clear enough. It sounds like the value of SP is screwed. Maybe invalid pointer on stack is better?

/cc @aclements

@gopherbot

This comment has been minimized.

Copy link

commented Sep 2, 2016

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

gopherbot pushed a commit that referenced this issue Sep 2, 2016

runtime: improve message when a bad pointer is found on the stack
Currently this message says "invalid stack pointer", which could be
interpreted as the value of SP being invalid. Change it to "invalid
pointer found on stack" to emphasize that it's a pointer on the stack
that's invalid.

Updates #16948.

Change-Id: I753624f8cc7e08cf13d3ea5d9c790cc4af9fa372
Reviewed-on: https://go-review.googlesource.com/28430
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

@golang golang locked and limited conversation to collaborators Sep 2, 2017

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.