johannesboyne / gofakes3

A simple fake AWS S3 object storage (used for local test-runs against AWS S3 APIs)
MIT License
361 stars 84 forks source link

panic using unaligned 64-bit atomics on 32-bit architectures #47

Closed jrhy closed 3 years ago

jrhy commented 3 years ago

Thanks for this super-handy project!

I found a bug on a Raspberry Pi, where tests fail like:

pi@raspberrypi:~/inst/gofakes3 $ go test -run=TestCreateObjectWithMissingContentLength
log output redirected to "/tmp/gofakes3-639390565.log"
--- FAIL: TestCreateObjectWithMissingContentLength (0.01s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x3f2dac]

goroutine 6 [running]:
testing.tRunner.func1.1(0x465c10, 0x7c6ad0)
    /home/pi/sdk/go1.15.7/src/testing/testing.go:1072 +0x264
testing.tRunner.func1(0x2401500)
    /home/pi/sdk/go1.15.7/src/testing/testing.go:1075 +0x364
panic(0x465c10, 0x7c6ad0)
    /home/pi/sdk/go1.15.7/src/runtime/panic.go:969 +0x158
github.com/johannesboyne/gofakes3_test.TestCreateObjectWithMissingContentLength(0x2401500)
    /home/pi/inst/gofakes3/gofakes3_test.go:197 +0x228
testing.tRunner(0x2401500, 0x4f4298)
    /home/pi/sdk/go1.15.7/src/testing/testing.go:1123 +0xbc
created by testing.(*T).Run
    /home/pi/sdk/go1.15.7/src/testing/testing.go:1168 +0x220
exit status 2
FAIL    github.com/johannesboyne/gofakes3   0.029s

Per sync/atomic#BUG, 64-bit atomic variables must be 64-bit aligned, but to embed such field in a struct on most 32-bit architectures, the field must be at the beginning of the struct to guarantee 64-bit alignment. Moving the requestID field to the beginning of the GoFakeS3 struct fixes this.

johannesboyne commented 3 years ago

Hi @jrhy good catch and thanks a lot for the contribution to fix it directly!