Misalignment causes panic on 386 with atomic int64

Issues Badger
1

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

c:\dev\temp\ll>go version
go version go1.15.6 windows/amd64

What operating system are you using?

Windows 10

What version of Badger are you using?

c:\dev\src\github.com\dgraph-io\badger>git log -n1
commit 1bebc261784de688acf135c6e0943c1feb04681f (HEAD -> master, origin/master, origin/HEAD)
Author: Manish R Jain <manish@dgraph.io>
Date:   Wed Jan 27 20:43:18 2021 -0800

Does this issue reproduce with the latest master?

Yes

Steps to Reproduce the issue

I have a simple test that writes/reads/closes the db.

What Badger options were set?

opts := badger.DefaultOptions(path).
        WithEncryptionKey(masterKey).
        WithValueLogFileSize(valueLogSize).
        WithIndexCacheSize(cacheSize)

What did you do?

Ran the test.

What did you expect to see?

Test should pass on all architectures.

What did you see instead?

Tests pass on amd64, but on 386 it panics:

=== RUN   TestBasic
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0x3f210b]

goroutine 13 [running]:
runtime/internal/atomic.Load64(0x1191fc84, 0x1, 0x3)
        c:/go/src/runtime/internal/atomic/asm_386.s:194 +0xb
github.com/dgraph-io/ristretto.(*sampledLFU).getMaxCost(...)
        C:/<snip>/vendor/github.com/dgraph-io/ristretto/policy.go:296
github.com/dgraph-io/ristretto.(*defaultPolicy).Add(0x1191fca0, 0x1, 0x0, 0xe0, 0x0, 0x0, 0x0, 0x0, 0x0)
        C:/<snip>/vendor/github.com/dgraph-io/ristretto/policy.go:140 +0xa2
github.com/dgraph-io/ristretto.(*Cache).processItems(0x11855240)
        C:/<snip>/vendor/github.com/dgraph-io/ristretto/cache.go:435 +0x288
created by github.com/dgraph-io/ristretto.NewCache
        C:/<snip>/vendor/github.com/dgraph-io/ristretto/cache.go:204 +0x303
exit status 2

Issue

The issue is here: ristretto/policy.go at master · dgraph-io/ristretto · GitHub

maxCost is not aligned on 64-bit address, and there is an issue with atomic - https://golang.org/pkg/sync/atomic/:
"
On ARM, x86-32, and 32-bit MIPS, 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.
"

Fix would be to align the members, or use mutex rather than atomic.

I created a minimal sample to repro the issue:
This code will panic. But move maxCost as first struct member, which guarantees alignment, and it will work. Failure is on 386 only.

package main

import (
	"fmt"
	"sync/atomic"
)

type sampledLFU struct {
	keyCosts map[uint64]int64
	maxCost  int64
}

func newSampledLFU(maxCost int64) *sampledLFU {
	return &sampledLFU{
		keyCosts: make(map[uint64]int64),
		maxCost:  maxCost,
	}
}

func (p *sampledLFU) getMaxCost() int64 {
	return atomic.LoadInt64(&(p.maxCost))
}

func main() {
	p := newSampledLFU(0xFFFFFFFFFFFFFFF)
	q := p.getMaxCost()
	fmt.Printf("maxCost=%v, loaded=%X\n", p.maxCost, q)
}
2

Hey @adamlicatta, thanks for reporting this. Can you please send a PR with the fix?

3

Hi @Naman, yep, will do.

1 Like
4

@adamlicatta Let us know on this thread when you have pushed the PR thru. Sometimes we miss things on github (I think the massive amounts of unmerged PRs is testament to that)

5

@chewxy, @naman, still working through the open source release process :-\

6

@chewxy, @naman, PR - structure alignment for 386 by anjalichandnani · Pull Request #249 · dgraph-io/ristretto · GitHub

1 Like
7

Thanks a lot @adamlicatta :tada: , for the PR. We have merged Fix crashing under ARMv6 by XIELongDragon · Pull Request #239 · dgraph-io/ristretto · GitHub with the comment from your PR.