Merge pull request #201 from jrick/checkptr

Fix unsafe pointer conversions caught by Go 1.14 checkptr

bolt_386.go

@@ -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

bolt_amd64.go

@@ -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

bolt_arm.go

@@ -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://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
-}

bolt_arm64.go

@@ -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

bolt_mips64x.go

@@ -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

bolt_mipsx.go

@@ -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

bolt_ppc.go

@@ -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

bolt_ppc64.go

@@ -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

bolt_ppc64le.go

@@ -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

bolt_riscv64.go

@@ -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

bolt_s390x.go

@@ -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

bucket.go

@@ -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)
 	}
@@ -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.
@@ -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)
 		}
 
@@ -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
@@ -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.

db_test.go

@@ -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 {

freelist.go

@@ -2,6 +2,7 @@ package bbolt
 
 import (
 	"fmt"
+	"reflect"
 	"sort"
 	"unsafe"
 )
@@ -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
@@ -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 {
@@ -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)
@@ -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

freelist_test.go

@@ -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
 

go.mod

@@ -1,3 +1,5 @@
 module go.etcd.io/bbolt
 
 go 1.12
+
+require golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5

go.sum

@@ -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=