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 misaligned struct member used in atomic operation #5182

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dswarbrick
Copy link

This fixes a panic caused by attempting to atomically access a struct member which is not 64-bit aligned when running on 32-bit arch, due to the smaller sync.Map struct. For example:

$ GOARCH=386 go test -run TestCacheAdd
--- FAIL: TestCacheAdd (0.00s)
panic: unaligned 64-bit atomic operation [recovered]
        panic: unaligned 64-bit atomic operation

goroutine 18 [running]:
testing.tRunner.func1.2({0x82a03c0, 0x85de888})
        /usr/lib/go-1.21/src/testing/testing.go:1545 +0x2ab
testing.tRunner.func1()
        /usr/lib/go-1.21/src/testing/testing.go:1548 +0x43e
panic({0x82a03c0, 0x85de888})
        /usr/lib/go-1.21/src/runtime/panic.go:914 +0x1ec
runtime/internal/atomic.panicUnaligned()
        /usr/lib/go-1.21/src/runtime/internal/atomic/unaligned.go:8 +0x2d
runtime/internal/atomic.Xadd64(0xa4c625c, 0x1)
        /usr/lib/go-1.21/src/runtime/internal/atomic/atomic_386.s:125 +0x11
github.com/aws/aws-sdk-go/aws/crr.(*EndpointCache).Add(0xa4c6240, {{0x82c9971, 0x3}, {0xa4c83f0, 0x1, 0x1}})
        /home/daniel/src/aws-sdk-go/aws/crr/cache.go:89 +0x141
github.com/aws/aws-sdk-go/aws/crr.TestCacheAdd(0xa6904b0)
        /home/daniel/src/aws-sdk-go/aws/crr/cache_test.go:204 +0x144c
testing.tRunner(0xa6904b0, 0x82efa7c)
        /usr/lib/go-1.21/src/testing/testing.go:1595 +0x120
created by testing.(*T).Run in goroutine 1
        /usr/lib/go-1.21/src/testing/testing.go:1648 +0x3dd
exit status 2
FAIL    github.com/aws/aws-sdk-go/aws/crr       0.005s

This is essentially the same issue as aws/aws-sdk-go-v2#2518

Please consider adding testing with a 32-bit GOARCH (e.g. "386") to your CI testing pipeline to avoid such oversights in future.

This fixes a panic caused by attempting to atomically access a struct
member which is not 64-bit aligned when running on 32-bit arch, due to
the smaller sync.Map struct.

Signed-off-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
@dswarbrick
Copy link
Author

@lucix-aws PTAL

@lucix-aws
Copy link
Contributor

Fell of my radar. Will try to address next week. Patch is fine though.

@Tiwat2540k
Copy link

Hfcgb

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.

None yet

3 participants