fix(arrow/cdata): Avoid calling unsafe.Slice on zero-length pointers (#513)

### Rationale for this change

Slices from FFI may have an arbitrary pointer when the length is zero,
but this is not allowed in Go, where the pointer must always be valid. I
believe this fixes #28.

### What changes are included in this PR?

I changed the instances of `unsafe.Slice` in `arrow/cdata` I could find
to be robust when used with length zero.

### Are these changes tested?

No, I don't have a Go setup at all.

### Are there any user-facing changes?

No.

---------

Co-authored-by: Matt Topol <zotthewizard@gmail.com>

Author: orlp

arrow/cdata/cdata.go

@@ -370,10 +370,10 @@ func (imp *cimporter) importChild(parent *cimporter, src *CArrowArray) error {
 
 // import any child arrays for lists, structs, and so on.
 func (imp *cimporter) doImportChildren() error {
-	children := unsafe.Slice(imp.arr.children, imp.arr.n_children)
-
-	if len(children) > 0 {
-		imp.children = make([]cimporter, len(children))
+	var children []*CArrowArray
+	if imp.arr.n_children > 0 {
+		children = unsafe.Slice(imp.arr.children, imp.arr.n_children)
+		imp.children = make([]cimporter, imp.arr.n_children)
 	}
 
 	// handle the cases
@@ -711,10 +711,12 @@ func (imp *cimporter) importBinaryViewLike() (err error) {
 		return
 	}
 
-	dataBufferSizes := unsafe.Slice((*int64)(unsafe.Pointer(imp.cbuffers[len(buffers)])), len(buffers)-2)
-	for i, size := range dataBufferSizes {
-		if buffers[i+2], err = imp.importVariableValuesBuffer(i+2, 1, size); err != nil {
-			return
+	if len(buffers) > 2 {
+		dataBufferSizes := unsafe.Slice((*int64)(unsafe.Pointer(imp.cbuffers[len(buffers)])), len(buffers)-2)
+		for i, size := range dataBufferSizes {
+			if buffers[i+2], err = imp.importVariableValuesBuffer(i+2, 1, size); err != nil {
+				return
+			}
 		}
 	}
 
@@ -866,7 +868,7 @@ func (imp *cimporter) checkNumBuffers(n int64) error {
 func (imp *cimporter) importBuffer(bufferID int, sz int64) (*memory.Buffer, error) {
 	// this is not a copy, we're just having a slice which points at the data
 	// it's still owned by the C.ArrowArray object and its backing C++ object.
-	if imp.cbuffers[bufferID] == nil {
+	if imp.cbuffers[bufferID] == nil || sz == 0 {
 		if sz != 0 {
 			return nil, errors.New("invalid buffer")
 		}

arrow/cdata/cdata_fulltest.c

@@ -335,8 +335,13 @@ void export_int32_array(const int32_t*, int64_t, struct ArrowArray*);
 
 static void release_str_array(struct ArrowArray* array) {
     assert(array->n_buffers == 3);
+    if (array->buffers[0] != NULL) {
+        free((void*) array->buffers[0]);
+    }
     free((void*) array->buffers[1]);
-    free((void*) array->buffers[2]);
+    if (array->buffers[2] != NULL && array->buffers[2] != (void*)0x1) {
+        free((void*) array->buffers[2]);
+    }
     free(array->buffers);
     array->release = NULL;
 }
@@ -361,6 +366,29 @@ void export_str_array(const char* data, const int32_t* offsets, int64_t nitems,
     out->buffers[2] = data;
 }
 
+void export_str_array_with_nulls(int64_t nitems, struct ArrowArray* out) {
+    *out = (struct ArrowArray) {
+        .length = nitems,
+        .offset = 0,
+        .null_count = nitems,
+        .n_buffers = 3,
+        .n_children = 0,
+        .children = NULL,
+        .dictionary = NULL,
+        // bookkeeping
+        .release = &release_str_array
+    };
+
+    out->buffers = (const void**)malloc(sizeof(void*) * out->n_buffers);
+    assert(out->buffers != NULL);
+    int64_t bitmap_nbytes = (nitems + 7) / 8;
+    out->buffers[0] = malloc(bitmap_nbytes);
+    memset((void*)out->buffers[0], 0, bitmap_nbytes);
+    out->buffers[1] = malloc((nitems + 1) * sizeof(int32_t));
+    memset((void*)out->buffers[1], 0, (nitems + 1) * sizeof(int32_t));
+    out->buffers[2] = (void*)0x1;
+}
+
 static int next_record(struct ArrowArrayStream* st, struct ArrowArray* out) {
     struct streamcounter* cnter = (struct streamcounter*)(st->private_data);
     if (cnter->n == cnter->max) {

arrow/cdata/cdata_test.go

@@ -737,6 +737,20 @@ func TestNestedArrays(t *testing.T) {
 	}
 }
 
+func TestStrArrayAllNulls(t *testing.T) {
+	arr := exportStrArrayWithNulls(1000)
+	carr, err := ImportCArrayWithType(&arr, arrow.BinaryTypes.String)
+	require.NoError(t, err)
+	defer carr.Release()
+
+	buffer := carr.Data().Buffers()[2]
+	assert.NotNil(t, buffer)
+	bs := buffer.Bytes()
+	assert.Equal(t, 1000, carr.Len())
+	assert.Equal(t, 1000, carr.NullN())
+	assert.Empty(t, bs)
+}
+
 func TestRecordBatch(t *testing.T) {
 	mem := mallocator.NewMallocator()
 	defer mem.AssertSize(t, 0)

arrow/cdata/cdata_test_framework.go

@@ -53,6 +53,7 @@ package cdata
 // }
 // void export_int32_type(struct ArrowSchema* schema);
 // void export_int32_array(const int32_t*, int64_t, struct ArrowArray*);
+// void export_str_array_with_nulls(int64_t nitems, struct ArrowArray* out);
 // int test1_is_released();
 // void test_primitive(struct ArrowSchema* schema, const char* fmt);
 // void free_malloced_schemas(struct ArrowSchema**);
@@ -98,6 +99,12 @@ func exportInt32TypeSchema() CArrowSchema {
 	return s
 }
 
+func exportStrArrayWithNulls(nitems int64) CArrowArray {
+	var arr CArrowArray
+	C.export_str_array_with_nulls(C.int64_t(nitems), &arr)
+	return arr
+}
+
 func schemaIsReleased(s *CArrowSchema) bool {
 	return C.ArrowSchemaIsReleased(s) == 1
 }