Repository: bmatsuo/lmdb-go (c)

Issue:
lmdb: possible CGO pointer argument panic using bytes.Buffer
My experimental raft backend at github.com/bmatsuo/raft-mdb issues a panic due to the new cgo pointer restrictions. The problem occurs when a bytes.Buffer with a small amount of data is written using the Bytes method.

```
$ go test
--- FAIL: TestMDB_Logs (0.00s)
panic: runtime error: cgo argument has Go pointer to Go pointer [recovered]
	panic: runtime error: cgo argument has Go pointer to Go pointer

goroutine 11 [running]:
panic(0x896ec0, 0xc82000bc00)
	/usr/lib/go/src/runtime/panic.go:464 +0x3e6
testing.tRunner.func1(0xc8200805a0)
	/usr/lib/go/src/testing/testing.go:467 +0x192
panic(0x896ec0, 0xc82000bc00)
	/usr/lib/go/src/runtime/panic.go:426 +0x4e9
github.com/bmatsuo/lmdb-go/lmdb._cgoCheckPointer0(0x7d23c0, 0xc8200541e4, 0x0, 0x0, 0x0, 0xc82000bba8)
	??:0 +0x4d
github.com/bmatsuo/lmdb-go/lmdb.(*Txn).Put(0xc8200c1f40, 0xc800000002, 0xc82000bba8, 0x8, 0x8, 0xc8200541e4, 0x1f, 0x40, 0x0, 0x0, ...)
	/home/b/src/github.com/bmatsuo/lmdb-go/lmdb/txn.go:297 +0x19d
github.com/bmatsuo/raft-mdb.(*MDBStore).StoreLogs.func1(0xc8200c1f40, 0x0, 0x0)
	/home/b/src/github.com/bmatsuo/raft-mdb/mdb_store.go:180 +0x31d
github.com/bmatsuo/lmdb-go/lmdb.(*Env).run(0xc8200280f0, 0x489301, 0x0, 0xc820049b90, 0x0, 0x0)
	/home/b/src/github.com/bmatsuo/lmdb-go/lmdb/env.go:461 +0x16a
github.com/bmatsuo/lmdb-go/lmdb.(*Env).Update(0xc8200280f0, 0xc820049b90, 0x0, 0x0)
	/home/b/src/github.com/bmatsuo/lmdb-go/lmdb/env.go:437 +0x45
github.com/bmatsuo/raft-mdb.(*MDBStore).StoreLogs(0xc820116600, 0xc820049bf0, 0x1, 0x1, 0x0, 0x0)
	/home/b/src/github.com/bmatsuo/raft-mdb/mdb_store.go:186 +0x63
github.com/bmatsuo/raft-mdb.(*MDBStore).StoreLog(0xc820116600, 0xc820015c80, 0x0, 0x0)
	/home/b/src/github.com/bmatsuo/raft-mdb/mdb_store.go:164 +0x84
github.com/bmatsuo/raft-mdb.TestMDB_Logs(0xc8200805a0)
	/home/b/src/github.com/bmatsuo/raft-mdb/mdb_store_test.go:167 +0x944
testing.tRunner(0xc8200805a0, 0xd85f40)
	/usr/lib/go/src/testing/testing.go:473 +0x98
created by testing.RunTests
	/usr/lib/go/src/testing/testing.go:582 +0x892
exit status 2
FAIL	github.com/bmatsuo/raft-mdb	0.006s
```

Pull request:
Fix cgo argument check panic for small slices from bytes.Buffer (and similar types)
Fixes #56 

The `valBytes()` function has been modified to return a non-empty slice of bytes for any (possibly empty) input slice. The length returned is always the true length of the input.

This is a hack that allows the cgo tool to correctly identify the argument type as the slice and not the bytes.Buffer (or whatever containing type causes these types of panics).

Patch:
diff --git a/lmdb/cursor.go b/lmdb/cursor.go
index 1dd8035..ec877c7 100644
--- a/lmdb/cursor.go
+++ b/lmdb/cursor.go
@@ -7,7 +7,10 @@ package lmdb
 #include "lmdbgo.h"
 */
 import "C"
-import "runtime"
+import (
+	"runtime"
+	"unsafe"
+)
 
 // These flags are used exclusively for Cursor.Get.
 const (
@@ -169,10 +172,9 @@ func (c *Cursor) getVal0(op uint) (key, val *C.MDB_val, err error) {
 func (c *Cursor) getVal1(setkey []byte, op uint) (key, val *C.MDB_val, err error) {
 	key = new(C.MDB_val)
 	val = new(C.MDB_val)
-	kdata, kn := valBytes(setkey)
 	ret := C.lmdbgo_mdb_cursor_get1(
 		c._c,
-		kdata, C.size_t(kn),
+		unsafe.Pointer(&setkey[0]), C.size_t(len(setkey)),
 		(*C.MDB_val)(key), (*C.MDB_val)(val),
 		C.MDB_cursor_op(op),
 	)
@@ -186,28 +188,36 @@ func (c *Cursor) getVal1(setkey []byte, op uint) (key, val *C.MDB_val, err error
 func (c *Cursor) getVal2(setkey, setval []byte, op uint) (key, val *C.MDB_val, err error) {
 	key = new(C.MDB_val)
 	val = new(C.MDB_val)
-	kdata, kn := valBytes(setkey)
-	vdata, vn := valBytes(setval)
 	ret := C.lmdbgo_mdb_cursor_get2(
 		c._c,
-		kdata, C.size_t(kn),
-		vdata, C.size_t(vn),
+		unsafe.Pointer(&setkey[0]), C.size_t(len(setkey)),
+		unsafe.Pointer(&setval[0]), C.size_t(len(setval)),
 		(*C.MDB_val)(key), (*C.MDB_val)(val),
 		C.MDB_cursor_op(op),
 	)
 	return key, val, operrno("mdb_cursor_get", ret)
 }
 
+func (c *Cursor) putNilKey(flags uint) error {
+	ret := C.lmdbgo_mdb_cursor_put2(c._c, nil, 0, nil, 0, C.uint(flags))
+	return operrno("mdb_cursor_put", ret)
+}
+
 // Put stores an item in the database.
 //
 // See mdb_cursor_put.
 func (c *Cursor) Put(key, val []byte, flags uint) error {
-	kdata, kn := valBytes(key)
-	vdata, vn := valBytes(val)
+	if len(key) == 0 {
+		return c.putNilKey(flags)
+	}
+	vn := len(val)
+	if vn == 0 {
+		val = []byte{0}
+	}
 	ret := C.lmdbgo_mdb_cursor_put2(
 		c._c,
-		kdata, C.size_t(kn),
-		vdata, C.size_t(vn),
+		unsafe.Pointer(&key[0]), C.size_t(len(key)),
+		unsafe.Pointer(&val[0]), C.size_t(len(val)),
 		C.uint(flags),
 	)
 	return operrno("mdb_cursor_put", ret)
@@ -217,11 +227,14 @@ func (c *Cursor) Put(key, val []byte, flags uint) error {
 // avoiding a memcopy.  The returned byte slice is only valid in txn's thread,
 // before it has terminated.
 func (c *Cursor) PutReserve(key []byte, n int, flags uint) ([]byte, error) {
-	kdata, kn := valBytes(key)
+	if len(key) == 0 {
+		return nil, c.putNilKey(flags)
+	}
+
 	val := &C.MDB_val{mv_size: C.size_t(n)}
 	ret := C.lmdbgo_mdb_cursor_put1(
 		c._c,
-		kdata, C.size_t(kn),
+		unsafe.Pointer(&key[0]), C.size_t(len(key)),
 		(*C.MDB_val)(val),
 		C.uint(flags|C.MDB_RESERVE),
 	)
@@ -238,13 +251,18 @@ func (c *Cursor) PutReserve(key []byte, n int, flags uint) ([]byte, error) {
 //
 // See mdb_cursor_put.
 func (c *Cursor) PutMulti(key []byte, page []byte, stride int, flags uint) error {
-	kdata, kn := valBytes(key)
-	vdata, _ := valBytes(page)
+	if len(key) == 0 {
+		return c.putNilKey(flags)
+	}
+	if len(page) == 0 {
+		page = []byte{0}
+	}
+
 	vn := WrapMulti(page, stride).Len()
 	ret := C.lmdbgo_mdb_cursor_putmulti(
 		c._c,
-		kdata, C.size_t(kn),
-		vdata, C.size_t(vn), C.size_t(stride),
+		unsafe.Pointer(&key[0]), C.size_t(len(key)),
+		unsafe.Pointer(&page[0]), C.size_t(vn), C.size_t(stride),
 		C.uint(flags|C.MDB_MULTIPLE),
 	)
 	return operrno("mdb_cursor_put", ret)
diff --git a/lmdb/txn.go b/lmdb/txn.go
index d821590..32d9848 100644
--- a/lmdb/txn.go
+++ b/lmdb/txn.go
@@ -278,7 +278,7 @@ func (txn *Txn) Get(dbi DBI, key []byte) ([]byte, error) {
 	val := new(C.MDB_val)
 	ret := C.lmdbgo_mdb_get(
 		txn._txn, C.MDB_dbi(dbi),
-		kdata, C.size_t(kn),
+		unsafe.Pointer(&kdata[0]), C.size_t(kn),
 		(*C.MDB_val)(val),
 	)
 	err := operrno("mdb_get", ret)
@@ -288,16 +288,29 @@ func (txn *Txn) Get(dbi DBI, key []byte) ([]byte, error) {
 	return txn.bytes(val), nil
 }
 
+func (txn *Txn) putNilKey(dbi DBI, flags uint) error {
+	// mdb_put with an empty key will always fail
+	ret := C.lmdbgo_mdb_put2(txn._txn, C.MDB_dbi(dbi), nil, 0, nil, 0, C.uint(flags))
+	return operrno("mdb_put", ret)
+}
+
 // Put stores an item in database dbi.
 //
 // See mdb_put.
 func (txn *Txn) Put(dbi DBI, key []byte, val []byte, flags uint) error {
-	kdata, kn := valBytes(key)
-	vdata, vn := valBytes(val)
+	kn := len(key)
+	if kn == 0 {
+		return txn.putNilKey(dbi, flags)
+	}
+	vn := len(val)
+	if vn == 0 {
+		val = []byte{0}
+	}
+
 	ret := C.lmdbgo_mdb_put2(
 		txn._txn, C.MDB_dbi(dbi),
-		kdata, C.size_t(kn),
-		vdata, C.size_t(vn),
+		unsafe.Pointer(&key[0]), C.size_t(kn),
+		unsafe.Pointer(&val[0]), C.size_t(vn),
 		C.uint(flags),
 	)
 	return operrno("mdb_put", ret)
@@ -307,11 +320,13 @@ func (txn *Txn) Put(dbi DBI, key []byte, val []byte, flags uint) error {
 // avoiding a memcopy.  The returned byte slice is only valid in txn's thread,
 // before it has terminated.
 func (txn *Txn) PutReserve(dbi DBI, key []byte, n int, flags uint) ([]byte, error) {
-	kdata, kn := valBytes(key)
+	if len(key) == 0 {
+		return nil, txn.putNilKey(dbi, flags)
+	}
 	val := &C.MDB_val{mv_size: C.size_t(n)}
 	ret := C.lmdbgo_mdb_put1(
 		txn._txn, C.MDB_dbi(dbi),
-		kdata, C.size_t(kn),
+		unsafe.Pointer(&key[0]), C.size_t(len(key)),
 		(*C.MDB_val)(val),
 		C.uint(flags|C.MDB_RESERVE),
 	)
@@ -331,8 +346,8 @@ func (txn *Txn) Del(dbi DBI, key, val []byte) error {
 	vdata, vn := valBytes(val)
 	ret := C.lmdbgo_mdb_del(
 		txn._txn, C.MDB_dbi(dbi),
-		kdata, C.size_t(kn),
-		vdata, C.size_t(vn),
+		unsafe.Pointer(&kdata[0]), C.size_t(kn),
+		unsafe.Pointer(&vdata[0]), C.size_t(vn),
 	)
 	return operrno("mdb_del", ret)
 }
diff --git a/lmdb/val.go b/lmdb/val.go
index 8dd0da7..a20586e 100644
--- a/lmdb/val.go
+++ b/lmdb/val.go
@@ -78,17 +78,19 @@ func (m *Multi) Page() []byte {
 	return m.page[:len(m.page):len(m.page)]
 }
 
-func valBytes(b []byte) (unsafe.Pointer, int) {
+var eb = []byte{0}
+
+func valBytes(b []byte) ([]byte, int) {
 	if len(b) == 0 {
-		return nil, 0
+		return eb, 0
 	}
-	return unsafe.Pointer(&b[0]), len(b)
+	return b, len(b)
 }
 
 func wrapVal(b []byte) *C.MDB_val {
-	ptr, n := valBytes(b)
+	p, n := valBytes(b)
 	return &C.MDB_val{
-		mv_data: unsafe.Pointer(ptr),
+		mv_data: unsafe.Pointer(&p[0]),
 		mv_size: C.size_t(n),
 	}
 }


Test patch:
diff --git a/lmdb/cursor_test.go b/lmdb/cursor_test.go
index 5a4d539..0d387c1 100644
--- a/lmdb/cursor_test.go
+++ b/lmdb/cursor_test.go
@@ -107,6 +107,59 @@ func TestCursor_Close(t *testing.T) {
 	}
 }
 
+func TestCursor_bytesBuffer(t *testing.T) {
+	env := setup(t)
+	defer clean(env, t)
+
+	db, err := openRoot(env, 0)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	err = env.Update(func(txn *Txn) (err error) {
+		cur, err := txn.OpenCursor(db)
+		if err != nil {
+			return err
+		}
+		defer cur.Close()
+		k := new(bytes.Buffer)
+		k.WriteString("hello")
+		v := new(bytes.Buffer)
+		v.WriteString("world")
+		return cur.Put(k.Bytes(), v.Bytes(), 0)
+	})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	err = env.View(func(txn *Txn) (err error) {
+		cur, err := txn.OpenCursor(db)
+		if err != nil {
+			return err
+		}
+		defer cur.Close()
+		k := new(bytes.Buffer)
+		k.WriteString("hello")
+		_k, v, err := cur.Get(k.Bytes(), nil, SetKey)
+		if err != nil {
+			return err
+		}
+		if !bytes.Equal(_k, k.Bytes()) {
+			return fmt.Errorf("unexpected key: %q", _k)
+		}
+		if !bytes.Equal(v, []byte("world")) {
+			return fmt.Errorf("unexpected value: %q", v)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+}
+
 func TestCursor_PutReserve(t *testing.T) {
 	env := setup(t)
 	defer clean(env, t)
diff --git a/lmdb/txn_test.go b/lmdb/txn_test.go
index f547e4f..ff9c3f3 100644
--- a/lmdb/txn_test.go
+++ b/lmdb/txn_test.go
@@ -214,6 +214,35 @@ func TestTxn_Del_dup(t *testing.T) {
 	}
 }
 
+func TestTexn_Put_emptyValue(t *testing.T) {
+	env := setup(t)
+	defer clean(env, t)
+
+	var db DBI
+	err := env.Update(func(txn *Txn) (err error) {
+		db, err = txn.OpenRoot(0)
+		if err != nil {
+			return err
+		}
+		err = txn.Put(db, []byte("k"), nil, 0)
+		if err != nil {
+			return err
+		}
+		v, err := txn.Get(db, []byte("k"))
+		if err != nil {
+			return err
+		}
+		if len(v) != 0 {
+			t.Errorf("value: %q (!= \"\")", v)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+}
+
 func TestTxn_PutReserve(t *testing.T) {
 	env := setup(t)
 	defer clean(env, t)
@@ -253,6 +282,46 @@ func TestTxn_PutReserve(t *testing.T) {
 	}
 }
 
+func TestTxn_bytesBuffer(t *testing.T) {
+	env := setup(t)
+	defer clean(env, t)
+
+	db, err := openRoot(env, 0)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	err = env.Update(func(txn *Txn) (err error) {
+		k := new(bytes.Buffer)
+		k.WriteString("hello")
+		v := new(bytes.Buffer)
+		v.WriteString("world")
+		return txn.Put(db, k.Bytes(), v.Bytes(), 0)
+	})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	err = env.View(func(txn *Txn) (err error) {
+		k := new(bytes.Buffer)
+		k.WriteString("hello")
+		v, err := txn.Get(db, k.Bytes())
+		if err != nil {
+			return err
+		}
+		if !bytes.Equal(v, []byte("world")) {
+			return fmt.Errorf("unexpected value: %q", v)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+}
+
 func TestTxn_Put_overwrite(t *testing.T) {
 	env := setup(t)
 	defer clean(env, t)
diff --git a/lmdb/val_test.go b/lmdb/val_test.go
index 4202f2e..493c6eb 100644
--- a/lmdb/val_test.go
+++ b/lmdb/val_test.go
@@ -46,8 +46,8 @@ func TestMultiVal_panic(t *testing.T) {
 
 func TestValBytes(t *testing.T) {
 	ptr, n := valBytes(nil)
-	if ptr != nil {
-		t.Errorf("unexpected non-nil pointer")
+	if len(ptr) == 0 {
+		t.Errorf("unexpected unaddressable slice")
 	}
 	if n != 0 {
 		t.Errorf("unexpected length: %d (expected 0)", n)
@@ -55,8 +55,8 @@ func TestValBytes(t *testing.T) {
 
 	b := []byte("abc")
 	ptr, n = valBytes(b)
-	if ptr == nil {
-		t.Errorf("unexpected nil pointer")
+	if len(ptr) == 0 {
+		t.Errorf("unexpected unaddressable slice")
 	}
 	if n != 3 {
 		t.Errorf("unexpected length: %d (expected %d)", n, len(b))