storj / uplink

Storj network Go library
MIT License
115 stars 18 forks source link

Regression: "unaligned 64-bit atomic operation" on 32-bit systems #156

Closed arrogantrabbit closed 9 months ago

arrogantrabbit commented 9 months ago

Please look at this thread:

https://forum.duplicacy.com/t/duplicacy-3-2-2-crashes-on-arm-32-bit/8024/7

To reproduce: build 32-bit arm version of the library and try to use.

Regression:

The proposed solution is to use atomic.AlignedInt64 instead of int64 in the MemoryBackend struct, or reorder len and buf, ideally -- do both, to reduce waste of space on padding.

Quoting analysis from the linked post:

I believe this is an issue in storj.io/uplink, which was upgraded from v1.9.0 to v1.12.0 for CLI 3.2.1: https://github.com/gilbertchen/duplicacy/commit/7cc1b4222cb466f9707bd338245f7f22f608e5a7

This is the line caused the crash: https://github.com/storj/uplink/blob/c46ff1e3c01a1ef00000e5bf3ae48f4e887f8466/private/storage/streams/buffer/backend.go#L38C31-L38C31

This is the definition of MemoryBackend: https://github.com/storj/uplink/blob/c46ff1e3c01a1ef00000e5bf3ae48f4e887f8466/private/storage/streams/buffer/backend.go#L27-L31

If they can simply swap buf and len like this then the error will go away:

type MemoryBackend struct {
  len    int64
  buf    []byte
  closed bool
}
storj-gerrit[bot] commented 9 months ago

Change private/storage/streams/buffer: fix unaligned LoadInt64 mentions this issue.

egonelbre commented 9 months ago

@arrogantrabbit Thank you for the report and analysis. It seems we forgot to run staticcheck for 32bit arm, which catches such issues.

egonelbre commented 9 months ago

PS: I created v1.12.1 release that has this issue fixed.