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

sync/atomic: addUint64 panics on ARM with unaligned pointers #23345

Closed
gernoteger opened this issue Jan 5, 2018 · 4 comments
Closed

sync/atomic: addUint64 panics on ARM with unaligned pointers #23345

gernoteger opened this issue Jan 5, 2018 · 4 comments

Comments

@gernoteger
Copy link

atomic.addUint64() panics if the pointer to its argument is not 64byte aligned.

it panics with:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x11438]

goroutine 1 [running]:
runtime/internal/atomic.Cas64(0x10410074, 0x1, 0x0, 0x2, 0x0, 0x7b728)
        /usr/local/go-1.9.2/src/runtime/internal/atomic/atomic_arm.go:110 +0x20
sync/atomic.addUint64(0x10410074, 0x1, 0x0, 0x3536c, 0x0)
        /usr/local/go-1.9.2/src/sync/atomic/64bit_arm.go:31 +0x4c

reproduce

package main

import "sync/atomic"

type X struct {
	foo int8
	bar uint64
}

func main() {
	x := X{bar: 1}
	atomic.AddUint64(&x.bar, 1)
	println("bar=", x.bar)
}

Hardware

I tested in on physical arm5/7 hardware. the arm5 code was executed on both targets, with equal results.

arm5:

uname -a
Linux tuxnet 2.6.31-626-g602af1c #131 PREEMPT Sat Jan 22 00:08:11 CET 2011 armv5tejl GNU/Linux

arm7:

uname -a
Linux rasp3 4.9.59-v7+ #1047 SMP Sun Oct 29 12:19:23 GMT 2017 armv7l GNU/Linux

compile options

This panics for:

  • GOOS=linux GOARCH=arm GOARM=5 with go 1.9.2
  • GOOS=linux GOARCH=arm GOARM=7 with go 1.9.2
  • GOOS=linux GOARCH=arm GOARM=7 with go 1.7.2

This works for:

  • GOOS=linux GOARCH=arm GOARM=7with go 1.71

so the behaviour is somewhat undefined, but became worse with 1.9.2.

Workaround

assure the atomic variabbles are aligned. E.g. putting bar first eliminates the issue

type X struct {
bar uint64
foo int8
}

This seems to be connected to #599.

What version of Go are you using (go version)?

1.7.1, 1.9.2

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

GOOS=linux GOARCH=arm GOARM=5
GOOS=linux GOARCH=arm GOARM=7

@artyom
Copy link
Member

artyom commented Jan 5, 2018

I think this case is already covered by the Bugs section of the package documentation:

On both ARM and x86-32, it is the caller's responsibility to arrange for 64-bit alignment of 64-bit words accessed atomically. The first word in a variable or in an allocated struct, array, or slice can be relied upon to be 64-bit aligned.

@dsnet
Copy link
Member

dsnet commented Jan 5, 2018

Closing as duplicate of #599.

@dsnet dsnet closed this as completed Jan 5, 2018
@gernoteger
Copy link
Author

to avoid: #11891

@mikioh mikioh changed the title atomic.addUint64 panics on ARM with unaligned pointers sync/atomic: addUint64 panics on ARM with unaligned pointers Jan 7, 2018
tyler-sommer added a commit to motki/core that referenced this issue May 9, 2018
This will hopefully fix a segfault under 32bit platforms caused
by unaligned access to 64bit values.

golang/go#23345
johannaratliff pushed a commit to cloudfoundry/go-diodes that referenced this issue Jul 17, 2018
writeIndex is written to atomically, which means it must be 64-bit
aligned on 32-bit 386 and ARM platforms. When the buffer slice is above
it, it becomes unaligned and segfaults on the call to
atomic.AddUint64().

See golang/go#23345
@brunoamancio
Copy link

brunoamancio commented Oct 6, 2018

I have the same issue with go 1.11 when running on linux, arm7, but the function atomic.StoreUint64.

svishwanath-tw added a commit to svishwanath-tw/desync that referenced this issue Jan 29, 2019
Alignment issues with 64 bit fields in the struct causes 32 bit builds of desync to not work (panic in sync/atomic package).
Re-arranging the members of the struct fixes this problem
Based on suggestions found at golang/go#599 and golang/go#23345.
daviddrysdale pushed a commit to google/certificate-transparency-go that referenced this issue Feb 25, 2019
Go doesn't always guarantee 64-bit alignment of 64-bit integer,
which leads to crashes when atomic operations are used on them
on certain platforms (e.g. ARM).

This problem is described here: https://golang.org/pkg/sync/atomic/#pkg-note-BUG

By putting these fields first, proper alignment will be guaranteed.

golang/go#599
golang/go#23345
@golang golang locked and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants