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

Fix unaligned load error on 32-bit architectures #438

Merged
merged 2 commits into from
Mar 7, 2024

Conversation

x0wllaar
Copy link
Contributor

@x0wllaar x0wllaar commented Feb 26, 2024

On some 32-bit architectures, 64-bit atomic operations panic when the value is not aligned properly.

In this package, this causes netConn operations to panic when compiling with GOARCH=386, since netConn does atomic operations with int64 values in the netConn struct (namely, with readExpired and writeExpired):

runtime/internal/atomic.panicUnaligned()
        .../go1.22/src/runtime/internal/atomic/unaligned.go:8 +0x2d
runtime/internal/atomic.Load64(0x90a011c)
        .../go1.22/src/runtime/internal/atomic/atomic_386.s:225 +0x10
nhooyr.io/websocket.(*netConn).read(0x90a00f0, {0x9180000, 0x8000, 0x8000})
        .../go/pkg/mod/nhooyr.io/websocket@v1.8.10/netconn.go:157 +0x2a
nhooyr.io/websocket.(*netConn).Read(0x90a00f0, {0x9180000, 0x8000, 0x8000})
        .../go/pkg/mod/nhooyr.io/websocket@v1.8.10/netconn.go:145 +0xb1

!!!REST OF THE CALLSTACK SNIPPED!!!

This commit fixes this by moving readExpired and writeExpired to the beginning of the struct, which makes them properly aligned.

I didn't test, but I think this error will also occur on 32-bit ARM (Raspberry Pi Zero) and MIPS32 (a bunch of routers use that, which is a real use case for me), see https://pkg.go.dev/sync/atomic#pkg-note-BUG and grafana/loki#6944

@x0wllaar x0wllaar force-pushed the fix-netconn-alignment branch 2 times, most recently from 3a71544 to 4eda6c3 Compare February 26, 2024 21:34
On some 32-bit architectures, 64-bit atomic operations panic when the
value is not aligned properly.

In this package, this causes netConn operations to panic when compiling
with GOARCH=386, since netConn does atomic operations with int64 values
in the netConn struct (namely, with readExpired and writeExpired).

This commit fixes this by moving readExpired and writeExpired to the
beginning of the struct, which makes them properly aligned.
@x0wllaar x0wllaar force-pushed the fix-netconn-alignment branch from 4eda6c3 to 4c27331 Compare February 26, 2024 21:36
@nhooyr
Copy link
Contributor

nhooyr commented Mar 7, 2024

Ah darn. Could have sworn I fixed this in the past. Thanks @x0wllaar

@nhooyr nhooyr enabled auto-merge March 7, 2024 19:39
@nhooyr nhooyr merged commit 65663d1 into coder:master Mar 7, 2024
4 checks passed
nhooyr added a commit that referenced this pull request Mar 7, 2024
Fix unaligned load error on 32-bit architectures

Closes #432
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants