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

Panic: unaligned 64-bit atomic operation #11

Open
gibmat opened this issue Aug 12, 2022 · 0 comments
Open

Panic: unaligned 64-bit atomic operation #11

gibmat opened this issue Aug 12, 2022 · 0 comments

Comments

@gibmat
Copy link

gibmat commented Aug 12, 2022

When built on a 32-bit architecture, the tests fail with unaligned panics:

----------------------------------------------------------------------
PANIC: dilated_test.go:93: dilatedClockSuite.TestAdvance

... Panic: unaligned 64-bit atomic operation (PC=0x807FAA2)

/usr/lib/go-1.18/src/runtime/panic.go:838
  in gopanic
/usr/lib/go-1.18/src/runtime/internal/atomic/unaligned.go:8
  in panicUnaligned
/usr/lib/go-1.18/src/runtime/internal/atomic/atomic_386.s:225
  in Load64
dilated.go:50
  in dilationClock.nowWithOffset
dilated.go:114
  in dilatedWallTimer.start
dilated.go:108
  in newDilatedWallTimer
dilated.go:58
  in dilationClock.After
dilated_test.go:96
  in dilatedClockSuite.TestAdvance
/usr/lib/go-1.18/src/reflect/value.go:339
  in Value.Call
/usr/lib/go-1.18/src/runtime/asm_386.s:1326
  in goexit

The following patch fixes the issue, by ensuring that the variable used in atomic operations is at the beginning of the struct to guarantee proper alignment:

diff --git a/testclock/dilated.go b/testclock/dilated.go
index 4333b11..0b4d764 100644
--- a/testclock/dilated.go
+++ b/testclock/dilated.go
@@ -24,11 +24,14 @@ func NewDilatedWallClock(realSecondDuration time.Duration) AdvanceableClock {
 }
 
 type dilationClock struct {
+	// offsetAtomic is the current dilated offset to allow for time jumps/advances.
+	// Must be the first item in the struct to ensure proper 64-bit alignment as
+	// documented at https://pkg.go.dev/sync/atomic#pkg-note-BUG
+	offsetAtomic int64
+
 	epoch              time.Time
 	realSecondDuration time.Duration
 
-	// offsetAtomic is the current dilated offset to allow for time jumps/advances.
-	offsetAtomic int64
 	// offsetChanged is a channel that is closed when timers need to be signaled
 	// that there is a offset change coming.
 	offsetChanged chan any
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

No branches or pull requests

1 participant