Skip to content

Commit

Permalink
Merge pull request #201 from jrick/checkptr
Browse files Browse the repository at this point in the history
Fix unsafe pointer conversions caught by Go 1.14 checkptr
  • Loading branch information
gyuho committed Mar 19, 2020
2 parents da442c5 + 543c40a commit 2fc6815
Show file tree
Hide file tree
Showing 21 changed files with 136 additions and 124 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
19 changes: 0 additions & 19 deletions bolt_arm.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,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 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:https://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_mips64x.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,3 @@ const maxMapSize = 0x8000000000 // 512GB

// 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_mipsx.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,3 @@ const maxMapSize = 0x40000000 // 1GB

// 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_ppc.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,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_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_riscv64.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 = true
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
30 changes: 16 additions & 14 deletions bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,12 @@ func (b *Bucket) Bucket(name []byte) *Bucket {
func (b *Bucket) openBucket(value []byte) *Bucket {
var child = newBucket(b.tx)

// If unaligned load/stores are broken on this arch and value is
// unaligned simply clone to an aligned byte array.
unaligned := brokenUnaligned && uintptr(unsafe.Pointer(&value[0]))&3 != 0

// Unaligned access requires a copy to be made.
const unalignedMask = unsafe.Alignof(struct {
bucket
page
}{}) - 1
unaligned := uintptr(unsafe.Pointer(&value[0]))&unalignedMask != 0
if unaligned {
value = cloneBytes(value)
}
Expand Down Expand Up @@ -409,24 +411,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 @@ -447,13 +449,13 @@ func (b *Bucket) Stats() BucketStats {

// 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)
s.BranchInuse += used
used += uintptr(lastElement.pos + lastElement.ksize)
s.BranchInuse += int(used)
s.BranchOverflowN += int(p.overflow)
}

Expand Down Expand Up @@ -593,7 +595,7 @@ func (b *Bucket) inlineable() bool {
// our threshold for inline bucket size.
var size = pageHeaderSize
for _, inode := range n.inodes {
size += leafPageElementSize + len(inode.key) + len(inode.value)
size += leafPageElementSize + uintptr(len(inode.key)) + uintptr(len(inode.value))

if inode.flags&bucketLeafFlag != 0 {
return false
Expand All @@ -606,8 +608,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
2 changes: 1 addition & 1 deletion db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,7 @@ func ExampleDB_View() {
// John's last name is doe.
}

func ExampleDB_Begin_ReadOnly() {
func ExampleDB_Begin() {
// Open the database.
db, err := bolt.Open(tempfile(), 0666, nil)
if err != nil {
Expand Down
37 changes: 29 additions & 8 deletions freelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package bbolt

import (
"fmt"
"reflect"
"sort"
"unsafe"
)
Expand Down Expand Up @@ -71,7 +72,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 @@ -93,8 +94,24 @@ func (f *freelist) pending_count() int {
return count
}

// copyall copies into dst a list of all free ids and all pending ids in one sorted list.
// 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 _, txp := range f.pending {
m = append(m, txp.ids...)
}
sort.Sort(m)
fpgids := f.getFreePageIDs()
sz := len(fpgids) + len(m)
dst := *(*[]pgid)(unsafe.Pointer(&reflect.SliceHeader{
Data: uintptr(dstptr),
Len: sz,
Cap: sz,
}))
mergepgids(dst, fpgids, m)
}

func (f *freelist) copyall(dst []pgid) {
m := make(pgids, 0, f.pending_count())
for _, txp := range f.pending {
Expand Down Expand Up @@ -267,17 +284,21 @@ 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)) + unsafe.Sizeof(*p))))
}

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

// copy the ids, so we don't modify on the freelist page directly
idsCopy := make([]pgid, count)
Expand Down Expand Up @@ -315,11 +336,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)) + unsafe.Sizeof(*p)))
} 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)) + unsafe.Sizeof(*p))) = pgid(lenids)
f.copyallunsafe(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p) + 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 @@ -262,7 +262,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)) + unsafe.Sizeof(*page)))
ids[0] = 23
ids[1] = 50

Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
module go.etcd.io/bbolt

go 1.12

require golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +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=
Loading

0 comments on commit 2fc6815

Please sign in to comment.