Skip to content

Commit 232d8fc

Browse files
gyuhogyuho
authored
Merge pull request etcd-io#220 from jrick/memfix
Fix incorrect unsafe usage
2 parents a8af23b + 044f3bd commit 232d8fc

File tree

7 files changed

+172
-106
lines changed

7 files changed

+172
-106
lines changed

‎freelist.go

Lines changed: 24 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package bbolt
22

33
import (
44
"fmt"
5-
"reflect"
65
"sort"
76
"unsafe"
87
)
@@ -94,24 +93,8 @@ func (f *freelist) pending_count() int {
9493
return count
9594
}
9695

97-
// copyallunsafe copies a list of all free ids and all pending ids in one sorted list.
96+
// copyall copies a list of all free ids and all pending ids in one sorted list.
9897
// f.count returns the minimum length required for dst.
99-
func (f *freelist) copyallunsafe(dstptr unsafe.Pointer) { // dstptr is []pgid data pointer
100-
m := make(pgids, 0, f.pending_count())
101-
for _, txp := range f.pending {
102-
m = append(m, txp.ids...)
103-
}
104-
sort.Sort(m)
105-
fpgids := f.getFreePageIDs()
106-
sz := len(fpgids) + len(m)
107-
dst := *(*[]pgid)(unsafe.Pointer(&reflect.SliceHeader{
108-
Data: uintptr(dstptr),
109-
Len: sz,
110-
Cap: sz,
111-
}))
112-
mergepgids(dst, fpgids, m)
113-
}
114-
11598
func (f *freelist) copyall(dst []pgid) {
11699
m := make(pgids, 0, f.pending_count())
117100
for _, txp := range f.pending {
@@ -284,21 +267,23 @@ func (f *freelist) read(p *page) {
284267
}
285268
// If the page.count is at the max uint16 value (64k) then it's considered
286269
// an overflow and the size of the freelist is stored as the first element.
287-
var idx, count uintptr = 0, uintptr(p.count)
270+
var idx, count = 0, int(p.count)
288271
if count == 0xFFFF {
289272
idx = 1
290-
count = uintptr(*(*pgid)(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p))))
273+
c := *(*pgid)(unsafeAdd(unsafe.Pointer(p), unsafe.Sizeof(*p)))
274+
count = int(c)
275+
if count < 0 {
276+
panic(fmt.Sprintf("leading element count %d overflows int", c))
277+
}
291278
}
292279

293280
// Copy the list of page ids from the freelist.
294281
if count == 0 {
295282
f.ids = nil
296283
} else {
297-
ids := *(*[]pgid)(unsafe.Pointer(&reflect.SliceHeader{
298-
Data: uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p) + idx*unsafe.Sizeof(pgid(0)),
299-
Len: int(count),
300-
Cap: int(count),
301-
}))
284+
var ids []pgid
285+
data := unsafeIndex(unsafe.Pointer(p), unsafe.Sizeof(*p), unsafe.Sizeof(ids[0]), idx)
286+
unsafeSlice(unsafe.Pointer(&ids), data, count)
302287

303288
// copy the ids, so we don't modify on the freelist page directly
304289
idsCopy := make([]pgid, count)
@@ -331,16 +316,22 @@ func (f *freelist) write(p *page) error {
331316

332317
// The page.count can only hold up to 64k elements so if we overflow that
333318
// number then we handle it by putting the size in the first element.
334-
lenids := f.count()
335-
if lenids == 0 {
336-
p.count = uint16(lenids)
337-
} else if lenids < 0xFFFF {
338-
p.count = uint16(lenids)
339-
f.copyallunsafe(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p)))
319+
l := f.count()
320+
if l == 0 {
321+
p.count = uint16(l)
322+
} else if l < 0xFFFF {
323+
p.count = uint16(l)
324+
var ids []pgid
325+
data := unsafeAdd(unsafe.Pointer(p), unsafe.Sizeof(*p))
326+
unsafeSlice(unsafe.Pointer(&ids), data, l)
327+
f.copyall(ids)
340328
} else {
341329
p.count = 0xFFFF
342-
*(*pgid)(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p))) = pgid(lenids)
343-
f.copyallunsafe(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p) + unsafe.Sizeof(pgid(0))))
330+
var ids []pgid
331+
data := unsafeAdd(unsafe.Pointer(p), unsafe.Sizeof(*p))
332+
unsafeSlice(unsafe.Pointer(&ids), data, l+1)
333+
ids[0] = pgid(l)
334+
f.copyall(ids[1:])
344335
}
345336

346337
return nil

‎manydbs_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package bbolt
2+
3+
import (
4+
"fmt"
5+
"io/ioutil"
6+
"math/rand"
7+
"os"
8+
"path/filepath"
9+
"testing"
10+
)
11+
12+
func createDb(t *testing.T) (*DB, func()) {
13+
// First, create a temporary directory to be used for the duration of
14+
// this test.
15+
tempDirName, err := ioutil.TempDir("", "bboltmemtest")
16+
if err != nil {
17+
t.Fatalf("error creating temp dir: %v", err)
18+
}
19+
path := filepath.Join(tempDirName, "testdb.db")
20+
21+
bdb, err := Open(path, 0600, nil)
22+
if err != nil {
23+
t.Fatalf("error creating bbolt db: %v", err)
24+
}
25+
26+
cleanup := func() {
27+
bdb.Close()
28+
os.RemoveAll(tempDirName)
29+
}
30+
31+
return bdb, cleanup
32+
}
33+
34+
func createAndPutKeys(t *testing.T) {
35+
t.Parallel()
36+
37+
db, cleanup := createDb(t)
38+
defer cleanup()
39+
40+
bucketName := []byte("bucket")
41+
42+
for i := 0; i < 100; i++ {
43+
err := db.Update(func(tx *Tx) error {
44+
nodes, err := tx.CreateBucketIfNotExists(bucketName)
45+
if err != nil {
46+
return err
47+
}
48+
49+
var key [16]byte
50+
rand.Read(key[:])
51+
if err := nodes.Put(key[:], nil); err != nil {
52+
return err
53+
}
54+
55+
return nil
56+
})
57+
if err != nil {
58+
t.Fatal(err)
59+
}
60+
}
61+
}
62+
63+
func TestManyDBs(t *testing.T) {
64+
for i := 0; i < 100; i++ {
65+
t.Run(fmt.Sprintf("%d", i), createAndPutKeys)
66+
}
67+
}

‎node.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package bbolt
33
import (
44
"bytes"
55
"fmt"
6-
"reflect"
76
"sort"
87
"unsafe"
98
)
@@ -208,36 +207,32 @@ func (n *node) write(p *page) {
208207
}
209208

210209
// Loop over each item and write it to the page.
211-
bp := uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p) + n.pageElementSize()*uintptr(len(n.inodes))
210+
// off tracks the offset into p of the start of the next data.
211+
off := unsafe.Sizeof(*p) + n.pageElementSize()*uintptr(len(n.inodes))
212212
for i, item := range n.inodes {
213213
_assert(len(item.key) > 0, "write: zero-length inode key")
214214

215+
// Create a slice to write into of needed size and advance
216+
// byte pointer for next iteration.
217+
sz := len(item.key) + len(item.value)
218+
b := unsafeByteSlice(unsafe.Pointer(p), off, 0, sz)
219+
off += uintptr(sz)
220+
215221
// Write the page element.
216222
if n.isLeaf {
217223
elem := p.leafPageElement(uint16(i))
218-
elem.pos = uint32(bp - uintptr(unsafe.Pointer(elem)))
224+
elem.pos = uint32(uintptr(unsafe.Pointer(&b[0])) - uintptr(unsafe.Pointer(elem)))
219225
elem.flags = item.flags
220226
elem.ksize = uint32(len(item.key))
221227
elem.vsize = uint32(len(item.value))
222228
} else {
223229
elem := p.branchPageElement(uint16(i))
224-
elem.pos = uint32(bp - uintptr(unsafe.Pointer(elem)))
230+
elem.pos = uint32(uintptr(unsafe.Pointer(&b[0])) - uintptr(unsafe.Pointer(elem)))
225231
elem.ksize = uint32(len(item.key))
226232
elem.pgid = item.pgid
227233
_assert(elem.pgid != p.id, "write: circular dependency occurred")
228234
}
229235

230-
// Create a slice to write into of needed size and advance
231-
// byte pointer for next iteration.
232-
klen, vlen := len(item.key), len(item.value)
233-
sz := klen + vlen
234-
b := *(*[]byte)(unsafe.Pointer(&reflect.SliceHeader{
235-
Data: bp,
236-
Len: sz,
237-
Cap: sz,
238-
}))
239-
bp += uintptr(sz)
240-
241236
// Write data for the element to the end of the page.
242237
l := copy(b, item.key)
243238
copy(b[l:], item.value)

‎node_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ func TestNode_read_LeafPage(t *testing.T) {
4444
nodes[1] = leafPageElement{flags: 0, pos: 23, ksize: 10, vsize: 3} // pos = sizeof(leafPageElement) + 3 + 4
4545

4646
// Write data for the nodes at the end.
47-
data := (*[4096]byte)(unsafe.Pointer(&nodes[2]))
48-
copy(data[:], "barfooz")
49-
copy(data[7:], "helloworldbye")
47+
const s = "barfoozhelloworldbye"
48+
data := unsafeByteSlice(unsafe.Pointer(&nodes[2]), 0, 0, len(s))
49+
copy(data, s)
5050

5151
// Deserialize page into a leaf.
5252
n := &node{}

‎page.go

Lines changed: 21 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package bbolt
33
import (
44
"fmt"
55
"os"
6-
"reflect"
76
"sort"
87
"unsafe"
98
)
@@ -51,52 +50,46 @@ func (p *page) typ() string {
5150

5251
// meta returns a pointer to the metadata section of the page.
5352
func (p *page) meta() *meta {
54-
return (*meta)(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p)))
53+
return (*meta)(unsafeAdd(unsafe.Pointer(p), unsafe.Sizeof(*p)))
5554
}
5655

5756
// leafPageElement retrieves the leaf node by index
5857
func (p *page) leafPageElement(index uint16) *leafPageElement {
59-
off := uintptr(index) * leafPageElementSize
60-
return (*leafPageElement)(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p) + off))
58+
return (*leafPageElement)(unsafeIndex(unsafe.Pointer(p), unsafe.Sizeof(*p),
59+
leafPageElementSize, int(index)))
6160
}
6261

6362
// leafPageElements retrieves a list of leaf nodes.
6463
func (p *page) leafPageElements() []leafPageElement {
6564
if p.count == 0 {
6665
return nil
6766
}
68-
return *(*[]leafPageElement)(unsafe.Pointer(&reflect.SliceHeader{
69-
Data: uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p),
70-
Len: int(p.count),
71-
Cap: int(p.count),
72-
}))
67+
var elems []leafPageElement
68+
data := unsafeAdd(unsafe.Pointer(p), unsafe.Sizeof(*p))
69+
unsafeSlice(unsafe.Pointer(&elems), data, int(p.count))
70+
return elems
7371
}
7472

7573
// branchPageElement retrieves the branch node by index
7674
func (p *page) branchPageElement(index uint16) *branchPageElement {
77-
off := uintptr(index) * unsafe.Sizeof(branchPageElement{})
78-
return (*branchPageElement)(unsafe.Pointer(uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p) + off))
75+
return (*branchPageElement)(unsafeIndex(unsafe.Pointer(p), unsafe.Sizeof(*p),
76+
unsafe.Sizeof(branchPageElement{}), int(index)))
7977
}
8078

8179
// branchPageElements retrieves a list of branch nodes.
8280
func (p *page) branchPageElements() []branchPageElement {
8381
if p.count == 0 {
8482
return nil
8583
}
86-
return *(*[]branchPageElement)(unsafe.Pointer(&reflect.SliceHeader{
87-
Data: uintptr(unsafe.Pointer(p)) + unsafe.Sizeof(*p),
88-
Len: int(p.count),
89-
Cap: int(p.count),
90-
}))
84+
var elems []branchPageElement
85+
data := unsafeAdd(unsafe.Pointer(p), unsafe.Sizeof(*p))
86+
unsafeSlice(unsafe.Pointer(&elems), data, int(p.count))
87+
return elems
9188
}
9289

9390
// dump writes n bytes of the page to STDERR as hex output.
9491
func (p *page) hexdump(n int) {
95-
buf := *(*[]byte)(unsafe.Pointer(&reflect.SliceHeader{
96-
Data: uintptr(unsafe.Pointer(p)),
97-
Len: n,
98-
Cap: n,
99-
}))
92+
buf := unsafeByteSlice(unsafe.Pointer(p), 0, 0, n)
10093
fmt.Fprintf(os.Stderr, "%x\n", buf)
10194
}
10295

@@ -115,11 +108,7 @@ type branchPageElement struct {
115108

116109
// key returns a byte slice of the node key.
117110
func (n *branchPageElement) key() []byte {
118-
return *(*[]byte)(unsafe.Pointer(&reflect.SliceHeader{
119-
Data: uintptr(unsafe.Pointer(n)) + uintptr(n.pos),
120-
Len: int(n.ksize),
121-
Cap: int(n.ksize),
122-
}))
111+
return unsafeByteSlice(unsafe.Pointer(n), 0, int(n.pos), int(n.pos)+int(n.ksize))
123112
}
124113

125114
// leafPageElement represents a node on a leaf page.
@@ -132,20 +121,16 @@ type leafPageElement struct {
132121

133122
// key returns a byte slice of the node key.
134123
func (n *leafPageElement) key() []byte {
135-
return *(*[]byte)(unsafe.Pointer(&reflect.SliceHeader{
136-
Data: uintptr(unsafe.Pointer(n)) + uintptr(n.pos),
137-
Len: int(n.ksize),
138-
Cap: int(n.ksize),
139-
}))
124+
i := int(n.pos)
125+
j := i + int(n.ksize)
126+
return unsafeByteSlice(unsafe.Pointer(n), 0, i, j)
140127
}
141128

142129
// value returns a byte slice of the node value.
143130
func (n *leafPageElement) value() []byte {
144-
return *(*[]byte)(unsafe.Pointer(&reflect.SliceHeader{
145-
Data: uintptr(unsafe.Pointer(n)) + uintptr(n.pos) + uintptr(n.ksize),
146-
Len: int(n.vsize),
147-
Cap: int(n.vsize),
148-
}))
131+
i := int(n.pos) + int(n.ksize)
132+
j := i + int(n.vsize)
133+
return unsafeByteSlice(unsafe.Pointer(n), 0, i, j)
149134
}
150135

151136
// PageInfo represents human readable information about a page.

0 commit comments

Comments
 (0)