runtime: jsoniter stack corruption with 32-bit arches, potential GC issues with 64-bit arches

[bboreham]

Go version

go1.24rc2 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='0'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GO386='sse2'
GOARCH='386'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/bryan/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/bryan/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m32 -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1397494606=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/tmp/jsoniter/go.mod'
GOMODCACHE='/home/bryan/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/bryan'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/bryan/pkg/mod/golang.org/toolchain@v0.0.1-go1.24rc2.linux-amd64'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/home/bryan/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/bryan/pkg/mod/golang.org/toolchain@v0.0.1-go1.24rc2.linux-amd64/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24rc2'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

When GOARCH was set to 386, and trying out the release candidates of Go 1.24, a unit test failed in the project Prometheus: https://github.com/prometheus/prometheus/actions/runs/12932314085/job/36070245239?pr=15707

I have simplified to just one use of the 'jsoniter' library.
https://go.dev/play/p/gtfdrAhV48r?v=gotip

(That link gets a different error: could not parse netrc (GOAUTH=netrc): $HOME is not defined, so I'll supply files too.)

go.mod
go.sum
jsoniter_test.go

What did you see happen?

$ GOARCH=386 go test
--- FAIL: TestJsonIter (0.00s)
    jsoniter_test.go:21: Result too short (28): "{\"item1\":[{\"type\":\"gauge\"}]}"
FAIL
exit status 1
FAIL    v1      0.001s

What did you expect to see?

The resulting JSON should be about 57 bytes long, with two items in it.
The test passes with GOARCH=amd64 and Go 1.24rc2, also with GOARCH=386 on Go 1.23.

[randall77]

I can reproduce.
@prattmic Possibly a swiss map iteration bug on 32-bit.
I will look more later today.

[randall77]

I suspect that https://github.com/modern-go/reflect2, which is imported by https://github.com/json-iterator/go, can not handle the switch to swissmaps. It reaches into internal details of the runtime with unsafe/linkname/etc. In fact I'm surprised it mostly works.
My immediate thought is that the hiter struct they use is now wrong. Maybe the new hiter is bigger?

[prattmic]

I kept the key and value fields in the same place to keep casts OK (reflect2 only accesses those fields AFAICT), but the new maps.Iter type is indeed 8-bytes bigger (64 vs 56 bytes) on 386, so we'll corrupt the stack. maps.Iter is smaller on amd64 (96 vs 104 bytes). The different pointer layout is likely a problem for the GC, even on amd64.

[randall77]

I opened a new issue against reflect2 here: modern-go/reflect2#32

Not sure what if anything the Go project should do about this. I'll leave this issue open in case anyone has some suggestions, but I'm inclined to say "tough luck". But reflect2 is quite popular, so if there was something simple we could do I would consider it.

[bboreham]

Thanks for the investigation and quick response.

FWIW I was aware that jsoniter is unmaintained, but I didn’t realise it was this problematic.

A little research shows that Kubernetes decided years ago to move off (although work remains).

[prattmic]

Thinking about potential for issues with GC, here are the overlaps on amd64:

type hiter struct {                       | type Iter struct {
        key         unsafe.Pointer        |     key         unsafe.Pointer
        value       unsafe.Pointer        |     elem        unsafe.Pointer
        t           unsafe.Pointer        |     typ         *abi.SwissMapType
        h           unsafe.Pointer        |     m           *Map
        buckets     unsafe.Pointer        |     entryOffset uint64
        bptr        unsafe.Pointer        |     dirOffset   uint64
        overflow    *[]unsafe.Pointer     |     clearSeq    uint64
        oldoverflow *[]unsafe.Pointer     |     globalDepth uint8
        startBucket uintptr               |     dirIdx      int
        offset      uint8                 |     tab         *table
        wrapped     bool                  |
        B           uint8                 |
        i           uint8                 |
        bucket      uintptr               |     group.data  unsafe.Pointer
        checkBucket uintptr               |     entryIdx    uint64
}                                         | }

(These structures are actually the same size. https://go.dev/cl/625275 added one more field to hiter, but that isn't reflected in reflect2, nor was it ever released.)

The pointer/non-pointer mismatches are:

Recorded as pointer, but not actually a pointer:

buckets     <-> entryOffset
bptr        <-> dirOffset
overflow    <-> clearSeq
oldoverflow <-> globalDepth

For these, the GC may throw if these look like pointers that aren't allocated. Also if they are non-zero but look too small to be a valid pointer [1]. The latter is more likely, all of these values will generally be quite small.

Recorded as non-pointer, but actually a pointer:

offset+wrapper+B+i <-> tab
bucket             <-> group.data

For these, if they aren't otherwise reachable then they could be freed early.

Both of these are normally reachable from the map itself, though I don't think reflect2 guarantees the map will still be reachable. (Edit: doh, the map is reachable from Iter). They will become reachable only from Iter if the table grows during iteration [2].

[1] I think? But now I can't find the code.
[2] I haven't double-checked, but I doubt the map can grow in json-iterator, as it wouldn't make sense for that package to mutate the map. But it may apply to other users of reflect2.

---

It looks like we could potentially work around this problem by rearranging the fields. Move tab and group up to align with a pointer.

Move entryOffset, dirOffset, clearSeq, globalDepth off of pointers. The only problem is that we have 5 non-pointer words, but hiter only has 4.

But globalDepth is small, so if we align it the top byte of a pointer, then that pointer will always look like a kernel address, which is probably OK.

[prattmic]

Release blocker until we make a decision.

[bboreham]

we have 5 non-pointer words, but hiter only has 4.

I count 6 in Iter - entryOffset, dirOffset, clearSeq, globalDepth, dirIdx, entryIdx.