oschwald / maxminddb-golang

MaxMind DB Reader for Go
ISC License
615 stars 101 forks source link

Test segfault with Golang 1.12.5 #55

Closed eclipseo closed 5 years ago

eclipseo commented 5 years ago

Golang 1.12.5, on Fedora Rawhide x86_64, maxminddb 1.3.0:

Testing    in: /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/src
         PATH: /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/bin:/builddir/.local/bin:/builddir/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/sbin
       GOPATH: /builddir/build/BUILD/maxminddb-golang-1.3.0/_build:/usr/share/gocode
  GO111MODULE: off
      command: go test -buildmode pie -compiler gc -ldflags "-X github.com/oschwald/maxminddb-golang/version=1.3.0 -extldflags '-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld '"
      testing: github.com/oschwald/maxminddb-golang
github.com/oschwald/maxminddb-golang
unexpected fault address 0x7febf45bb623
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x1 addr=0x7febf45bb623 pc=0x563fe18a7806]

goroutine 52 [running]:
runtime.throw(0x563fe1aeb66f, 0x5)
    /usr/lib/golang/src/runtime/panic.go:617 +0x74 fp=0xc0000a9890 sp=0xc0000a9860 pc=0x563fe1877b94
runtime.sigpanic()
    /usr/lib/golang/src/runtime/signal_unix.go:397 +0x405 fp=0xc0000a98c0 sp=0xc0000a9890 pc=0x563fe188cdb5
runtime.memmove(0xc000019890, 0x7febf45bb623, 0xc)
    /usr/lib/golang/src/runtime/memmove_amd64.s:173 +0x136 fp=0xc0000a98c8 sp=0xc0000a98c0 pc=0x563fe18a7806
runtime.slicebytetostring(0x0, 0x7febf45bb623, 0xc, 0x7381, 0xc000019890, 0xc)
    /usr/lib/golang/src/runtime/string.go:106 +0x82 fp=0xc0000a98f8 sp=0xc0000a98c8 pc=0x563fe1891a12
github.com/oschwald/maxminddb-golang.(*decoder).decodeMap(0xc0000a9c68, 0x2, 0x5901, 0x563fe1c02e00, 0xc0004653b0, 0x15, 0x1, 0x58e4, 0x0, 0x0)
    /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/src/github.com/oschwald/maxminddb-golang/decoder.go:495 +0x28f fp=0xc0000a99b0 sp=0xc0000a98f8 pc=0x563fe1ad87cf
github.com/oschwald/maxminddb-golang.(*decoder).unmarshalMap(0xc0000a9c68, 0x2, 0x58e5, 0x563fe1c01840, 0xc0004693f0, 0x194, 0x1, 0x2, 0x58e5, 0x0)
    /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/src/github.com/oschwald/maxminddb-golang/decoder.go:324 +0x3d5 fp=0xc0000a9ab8 sp=0xc0000a99b0 pc=0x563fe1ad6e15
github.com/oschwald/maxminddb-golang.(*decoder).decodeFromType(0xc0000a9c68, 0x7, 0x2, 0x58e5, 0x563fe1be3fe0, 0xc0004693f0, 0x16, 0x1, 0xc0003c7320, 0xc0000a9bb0, ...)
    /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/src/github.com/oschwald/maxminddb-golang/decoder.go:123 +0x580 fp=0xc0000a9b38 sp=0xc0000a9ab8 pc=0x563fe1ad4d10
github.com/oschwald/maxminddb-golang.(*decoder).decode(0xc0000a9c68, 0x58e4, 0x563fe1be3fe0, 0xc0004693f0, 0x16, 0x0, 0x58e4, 0x0, 0x0)
    /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/src/github.com/oschwald/maxminddb-golang/decoder.go:54 +0xee fp=0xc0000a9bc0 sp=0xc0000a9b38 pc=0x563fe1ad415e
github.com/oschwald/maxminddb-golang.(*verifier).verifyDataSection(0xc0000a9d80, 0xc000382f90, 0x0, 0x0)
    /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/src/github.com/oschwald/maxminddb-golang/verifier.go:137 +0x18f fp=0xc0000a9d30 sp=0xc0000a9bc0 pc=0x563fe1adce9f
github.com/oschwald/maxminddb-golang.(*verifier).verifyDatabase(0xc0000a9d80, 0x0, 0x0)
    /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/src/github.com/oschwald/maxminddb-golang/verifier.go:94 +0x93 fp=0xc0000a9d68 sp=0xc0000a9d30 pc=0x563fe1adc913
github.com/oschwald/maxminddb-golang.(*Reader).Verify(0xc0002daa00, 0xc0002c0400, 0x0)
    /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/src/github.com/oschwald/maxminddb-golang/verifier.go:18 +0x71 fp=0xc0000a9d98 sp=0xc0000a9d68 pc=0x563fe1adbf21
github.com/oschwald/maxminddb-golang.TestVerifyOnGoodDatabases(0xc0002c0400)
    /builddir/build/BUILD/maxminddb-golang-1.3.0/_build/src/github.com/oschwald/maxminddb-golang/verifier_test.go:35 +0x111 fp=0xc0000a9fa8 sp=0xc0000a9d98 pc=0x563fe1ae8521
testing.tRunner(0xc0002c0400, 0x563fe1c61588)
    /usr/lib/golang/src/testing/testing.go:865 +0xc2 fp=0xc0000a9fd0 sp=0xc0000a9fa8 pc=0x563fe191ff72
runtime.goexit()
    /usr/lib/golang/src/runtime/asm_amd64.s:1337 +0x1 fp=0xc0000a9fd8 sp=0xc0000a9fd0 pc=0x563fe18a65b1
created by testing.(*T).Run
    /usr/lib/golang/src/testing/testing.go:916 +0x35c

goroutine 1 [chan receive]:
testing.(*T).Run(0xc0002c0400, 0x563fe1af1c31, 0x19, 0x563fe1c61588, 0x563fe18c1301)
    /usr/lib/golang/src/testing/testing.go:917 +0x383
testing.runTests.func1(0xc00015c000)
    /usr/lib/golang/src/testing/testing.go:1157 +0x7a
testing.tRunner(0xc00015c000, 0xc0000a7e30)
    /usr/lib/golang/src/testing/testing.go:865 +0xc2
testing.runTests(0xc000096380, 0x563fe1e3b7c0, 0x23, 0x23, 0x0)
    /usr/lib/golang/src/testing/testing.go:1155 +0x2ab
testing.(*M).Run(0xc00015a000, 0x0)
    /usr/lib/golang/src/testing/testing.go:1072 +0x164
main.main()
    _testmain.go:120 +0x140
exit status 2
FAIL    github.com/oschwald/maxminddb-golang    0.045s
oschwald commented 5 years ago

I am not sure why this is happening. Based on my testing, this issue appeared with Go 1.12. It seems related to memory mapping as the same test passes if I load the database into memory instead. It would be interesting to bisect Go to see which commit caused the issue.

oschwald commented 5 years ago

The bisect of go wasn't quite as bad as I imagined. The offending commit seems to be https://github.com/golang/go/commit/9a8372f8bd5a39d2476bfa9247407b51f9193b9e. I'll likely need to dig into this later as most of the changes are beyond me.

oschwald commented 5 years ago

Aha, the real issue seems to be the use of runtime.SetFinalizer. Using that was probably a bad decision on my part. I'll need to think about this a bit more.

oschwald commented 5 years ago

This turned out to be fairly easy to fix. From the Go 1.12 release notes:

The compiler's live variable analysis has improved. This may mean that finalizers will be executed sooner in this release than in previous releases. If that is a problem, consider the appropriate addition of a runtime.KeepAlive call.

I added a runtime.KeepAlive call to the Verify method and that seemed to resolve the issue.