Skip to content

Commit

Permalink
Fix unsafe pointer conversions caught by Go 1.14 checkptr
Browse files Browse the repository at this point in the history
Port of etcd-io/bbolt@543c40a
Now tests don't crush with -race flag
Also found new race, reported to etcd/bolt: etcd-io#213
  • Loading branch information
AskAlexSharov committed Apr 2, 2020
1 parent 17bc3f8 commit 304a272
Show file tree
Hide file tree
Showing 15 changed files with 162 additions and 120 deletions.
3 changes: 0 additions & 3 deletions bolt_386.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,3 @@ const maxMapSize = 0x7FFFFFFF // 2GB

// maxAllocSize is the size used when creating array pointers.
const maxAllocSize = 0xFFFFFFF

// Are unaligned load/stores broken on this arch?
var brokenUnaligned = false
3 changes: 0 additions & 3 deletions bolt_amd64.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,3 @@ const maxMapSize = 0xFFFFFFFFFFFF // 256TB

// maxAllocSize is the size used when creating array pointers.
const maxAllocSize = 0x7FFFFFFF

// Are unaligned load/stores broken on this arch?
var brokenUnaligned = false
21 changes: 0 additions & 21 deletions bolt_arm.go
Original file line number Diff line number Diff line change
@@ -1,28 +1,7 @@
package bolt

import "unsafe"

// maxMapSize represents the largest mmap size supported by Bolt.
const maxMapSize = 0x7FFFFFFF // 2GB

// maxAllocSize is the size used when creating array pointers.
const maxAllocSize = 0xFFFFFFF

// Are unaligned load/stores broken on this arch?
var brokenUnaligned bool

func init() {
// Simple check to see whether this arch handles unaligned load/stores
// correctly.

// ARM9 and older devices require load/stores to be from/to aligned
// addresses. If not, the lower 2 bits are cleared and that address is
// read in a jumbled up order.

// See http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html

raw := [6]byte{0xfe, 0xef, 0x11, 0x22, 0x22, 0x11}
val := *(*uint32)(unsafe.Pointer(uintptr(unsafe.Pointer(&raw)) + 2))

brokenUnaligned = val != 0x11222211
}
3 changes: 0 additions & 3 deletions bolt_arm64.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,3 @@ const maxMapSize = 0xFFFFFFFFFFFF // 256TB

// maxAllocSize is the size used when creating array pointers.
const maxAllocSize = 0x7FFFFFFF

// Are unaligned load/stores broken on this arch?
var brokenUnaligned = false
3 changes: 0 additions & 3 deletions bolt_ppc64.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,3 @@ const maxMapSize = 0xFFFFFFFFFFFF // 256TB

// maxAllocSize is the size used when creating array pointers.
const maxAllocSize = 0x7FFFFFFF

// Are unaligned load/stores broken on this arch?
var brokenUnaligned = false
3 changes: 0 additions & 3 deletions bolt_ppc64le.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,3 @@ const maxMapSize = 0xFFFFFFFFFFFF // 256TB

// maxAllocSize is the size used when creating array pointers.
const maxAllocSize = 0x7FFFFFFF

// Are unaligned load/stores broken on this arch?
var brokenUnaligned = false
3 changes: 0 additions & 3 deletions bolt_s390x.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,3 @@ const maxMapSize = 0xFFFFFFFFFFFF // 256TB

// maxAllocSize is the size used when creating array pointers.
const maxAllocSize = 0x7FFFFFFF

// Are unaligned load/stores broken on this arch?
var brokenUnaligned = false
28 changes: 14 additions & 14 deletions bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,24 +554,24 @@ func (b *Bucket) Stats() BucketStats {

if p.count != 0 {
// If page has any elements, add all element headers.
used += leafPageElementSize * int(p.count-1)
used += leafPageElementSize * uintptr(p.count-1)

// Add all element key, value sizes.
// The computation takes advantage of the fact that the position
// of the last element's key/value equals to the total of the sizes
// of all previous elements' keys and values.
// It also includes the last element's header.
lastElement := p.leafPageElement(p.count - 1)
used += int(lastElement.pos + lastElement.ksize + lastElement.vsize)
used += uintptr(lastElement.pos + lastElement.ksize + lastElement.vsize)
}

if b.root == 0 {
// For inlined bucket just update the inline stats
s.InlineBucketInuse += used
s.InlineBucketInuse += int(used)
} else {
// For non-inlined bucket update all the leaf stats
s.LeafPageN++
s.LeafInuse += used
s.LeafInuse += int(used)
s.LeafOverflowN += int(p.overflow)

// Collect stats from sub-buckets.
Expand All @@ -588,31 +588,31 @@ func (b *Bucket) Stats() BucketStats {
}
} else if (p.flags & branchPageFlag) != 0 {
s.BranchPageN++
var used int
var used uintptr
if b.enum {
lastElement := p.branchPageElementX(p.count - 1)

// used totals the used bytes for the page
// Add header and all element headers.
used = pageHeaderSize + (branchPageElementSizeX * int(p.count-1))
used = pageHeaderSize + (branchPageElementSizeX * uintptr(p.count-1))

// Add size of all keys and values.
// Again, use the fact that last element's position equals to
// the total of key, value sizes of all previous elements.
used += int(lastElement.pos + lastElement.ksize)
used += uintptr(lastElement.pos + lastElement.ksize)
} else {
lastElement := p.branchPageElement(p.count - 1)

// used totals the used bytes for the page
// Add header and all element headers.
used = pageHeaderSize + (branchPageElementSize * int(p.count-1))
used = pageHeaderSize + (branchPageElementSize * uintptr(p.count-1))

// Add size of all keys and values.
// Again, use the fact that last element's position equals to
// the total of key, value sizes of all previous elements.
used += int(lastElement.pos + lastElement.ksize)
used += uintptr(lastElement.pos + lastElement.ksize)
}
s.BranchInuse += used
s.BranchInuse += int(used)
s.BranchOverflowN += int(p.overflow)
}

Expand Down Expand Up @@ -758,7 +758,7 @@ func (b *Bucket) inlineable() bool {
var size = pageHeaderSize
var prefix []byte
for i, inode := range n.inodes {
size += leafPageElementSize + len(inode.key) + len(inode.value)
size += leafPageElementSize + uintptr(len(inode.key)) + uintptr(len(inode.value))
if !b.tx.db.KeysPrefixCompressionDisable {
if prefix == nil {
prefix = inode.key
Expand All @@ -775,7 +775,7 @@ func (b *Bucket) inlineable() bool {
}
if inode.flags&bucketLeafFlag != 0 {
return false
} else if size-len(prefix)*i > b.maxInlineBucketSize() {
} else if size-uintptr(len(prefix)*i) > b.maxInlineBucketSize() {
return false
}
}
Expand All @@ -784,8 +784,8 @@ func (b *Bucket) inlineable() bool {
}

// Returns the maximum total size of a bucket to make it a candidate for inlining.
func (b *Bucket) maxInlineBucketSize() int {
return b.tx.db.pageSize / 4
func (b *Bucket) maxInlineBucketSize() uintptr {
return uintptr(b.tx.db.pageSize / 4)
}

// write allocates and writes a bucket to a byte slice.
Expand Down
36 changes: 29 additions & 7 deletions freelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package bolt

import (
"fmt"
"reflect"
"sort"
"unsafe"
)
Expand Down Expand Up @@ -29,7 +30,7 @@ func (f *freelist) size() int {
// The first element will be used to store the count. See freelist.write.
n++
}
return pageHeaderSize + (int(unsafe.Sizeof(pgid(0))) * n)
return int(pageHeaderSize) + (int(unsafe.Sizeof(pgid(0))) * n)
}

// count returns count of pages on the freelist
Expand All @@ -51,6 +52,23 @@ func (f *freelist) pending_count() int {
return count
}

// copyallunsafe copies a list of all free ids and all pending ids in one sorted list.
// f.count returns the minimum length required for dst.
func (f *freelist) copyallunsafe(dstptr unsafe.Pointer) { // dstptr is []pgid data pointer
m := make(pgids, 0, f.pending_count())
for _, list := range f.pending {
m = append(m, list...)
}
sort.Sort(m)
sz := len(f.ids) + len(m)
dst := *(*[]pgid)(unsafe.Pointer(&reflect.SliceHeader{
Data: uintptr(dstptr),
Len: sz,
Cap: sz,
}))
mergepgids(dst, f.ids, m)
}

// copyall copies into dst a list of all free ids and all pending ids in one sorted list.
// f.count returns the minimum length required for dst.
func (f *freelist) copyall(dst []pgid) {
Expand Down Expand Up @@ -163,17 +181,21 @@ func (f *freelist) freed(pgid pgid) bool {
func (f *freelist) read(p *page) {
// If the page.count is at the max uint16 value (64k) then it's considered
// an overflow and the size of the freelist is stored as the first element.
idx, count := 0, int(p.count)
var idx, count uintptr = 0, uintptr(p.count)
if count == 0xFFFF {
idx = 1
count = int(((*[maxAllocSize]pgid)(unsafe.Pointer(&p.ptr)))[0])
count = uintptr(*(*pgid)(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + pageHeaderSize)))
}

// Copy the list of page ids from the freelist.
if count == 0 {
f.ids = nil
} else {
ids := ((*[maxAllocSize]pgid)(unsafe.Pointer(&p.ptr)))[idx:count]
ids := *(*[]pgid)(unsafe.Pointer(&reflect.SliceHeader{
Data: uintptr(unsafe.Pointer(p)) + pageHeaderSize + idx*unsafe.Sizeof(pgid(0)),
Len: int(count),
Cap: int(count),
}))
f.ids = make([]pgid, len(ids))
copy(f.ids, ids)

Expand Down Expand Up @@ -201,11 +223,11 @@ func (f *freelist) write(p *page) error {
p.count = uint16(lenids)
} else if lenids < 0xFFFF {
p.count = uint16(lenids)
f.copyall(((*[maxAllocSize]pgid)(unsafe.Pointer(&p.ptr)))[:])
f.copyallunsafe(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + pageHeaderSize))
} else {
p.count = 0xFFFF
((*[maxAllocSize]pgid)(unsafe.Pointer(&p.ptr)))[0] = pgid(lenids)
f.copyall(((*[maxAllocSize]pgid)(unsafe.Pointer(&p.ptr)))[1:])
*(*pgid)(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + pageHeaderSize)) = pgid(lenids)
f.copyallunsafe(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + pageHeaderSize + unsafe.Sizeof(pgid(0))))
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion freelist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestFreelist_read(t *testing.T) {
page.count = 2

// Insert 2 page ids.
ids := (*[3]pgid)(unsafe.Pointer(&page.ptr))
ids := (*[3]pgid)(unsafe.Pointer(uintptr(unsafe.Pointer(page)) + pageHeaderSize))
ids[0] = 23
ids[1] = 50

Expand Down
4 changes: 1 addition & 3 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,4 +1,2 @@
golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5 h1:LfCXLvNmTYH9kEmVgqbnsWfruoXZIrh4YBgqVHtDvw0=
golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200331124033-c3d80250170d h1:nc5K6ox/4lTFbMVSL9WRR81ixkcwXThoiF6yf+R9scA=
golang.org/x/sys v0.0.0-20200331124033-c3d80250170d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Loading

0 comments on commit 304a272

Please sign in to comment.