bug: Panic on 32-bit platforms

[hairyhenderson]

There are a number of field alignment issues in this package, which cause panics on 32-bit platforms. I built a quick Docker image to run the tests:

Dockerfile.test:

FROM golang:1.22

WORKDIR /app

COPY . .

$ docker build --platform=linux/386 -t test_32bit -f Dockerfile.test .
[...]
$ docker run --platform=linux/386 --rm test_32bit go test ./...
?       github.com/hack-pad/hackpadfs/fstest    [no test files]
?       github.com/hack-pad/hackpadfs/internal/mounttest        [no test files]
?       github.com/hack-pad/hackpadfs/internal/assert   [no test files]
?       github.com/hack-pad/hackpadfs/internal/pathlock [no test files]
?       github.com/hack-pad/hackpadfs/keyvalue/blob     [no test files]
--- FAIL: TestChmod (0.00s)
panic: unaligned 64-bit atomic operation [recovered]
        panic: unaligned 64-bit atomic operation

goroutine 23 [running]:
testing.tRunner.func1.2({0x81d5b80, 0x82378b0})
        /usr/local/go/src/testing/testing.go:1631 +0x2a3
testing.tRunner.func1()
        /usr/local/go/src/testing/testing.go:1634 +0x434
panic({0x81d5b80, 0x82378b0})
        /usr/local/go/src/runtime/panic.go:770 +0x103
runtime/internal/atomic.panicUnaligned()
        /usr/local/go/src/runtime/internal/atomic/unaligned.go:8 +0x2d
runtime/internal/atomic.Load64(0x886609c)
        /usr/local/go/src/runtime/internal/atomic/atomic_386.s:225 +0x10
github.com/hack-pad/hackpadfs/keyvalue/blob.(*Bytes).Len(0x8866090)
        /app/keyvalue/blob/bytes.go:56 +0x26
[...]

This particular failure is due to the blob.Bytes type, which is misaligned:

type Bytes struct {
	bytes  []byte		// 24 bytes on 64-bit, 12 bytes on 32-bit
	length int64		// 8 bytes
	mu     *sync.Mutex	// 8 bytes on 64-bit, 4 bytes on 32-bit
}

If we run fieldalignment on this, we see (on 64-bit):

$ fieldalignment ./keyvalue/blob/  
/Users/hairyhenderson/go/src/github.com/hack-pad/hackpadfs/keyvalue/blob/bytes.go:23:12: struct with 40 pointer bytes could be 16

And on 32-bit:

$ go install golang.org/x/tools/go/analysis/passes/fieldalignment/cmd/fieldalignment@latest
[...]
$ fieldalignment ./keyvalue/blob/  
/app/keyvalue/blob/bytes.go:23:12: struct with 24 pointer bytes could be 8

So it's not optimally-aligned, but that's not actually the root issue. The root issue is a known bug on 32-bit systems on many atomic.* functions, namely:

On ARM, 386, and 32-bit MIPS, it is the caller's responsibility to arrange for 64-bit alignment of 64-bit words accessed atomically via the primitive atomic functions (types Int64 and Uint64 are automatically aligned). [...]

Thankfully, fixing this automatically with fieldalignment works fine (it moves mu to the beginning so we get a 32-byte or 16-byte offset before length, thus aligning length on a 64-bit/8-byte boundary as it needs to be).

I'm going to issue a PR that enables the fieldalignment linter (which is built into the govet linter in golangci-lint), and fixes the found problems.

[JohnStarich]

Thank you for the issue and PR! I’ll take a look soon.

I’ll be honest, I didn’t realize things like this weren’t handled automatically by Go. Really appreciate you bringing this to our attention.

[hairyhenderson]

@JohnStarich no worries. I've got a PR in progress on my fork hairyhenderson#4 that I'll re-upstream once I get fully working. There's a bunch of other lint failures that'll need to be fixed separately though. I started on that a few days back in #42, but that was a sort of half-hearted effort 😉

[hairyhenderson]

@JohnStarich ok, I've got #44 up and ready to review - it fixes the bug 😁

[JohnStarich]

Thanks again for your time and effort on this one! Your change has been released in v0.2.2 🚀

[hairyhenderson]

Thanks for the quick review and release @JohnStarich! 🙇‍♂️