Skip to content

Commit

Permalink
DRAFT: [C++] C Data Interface: check imported buffer for non-null
Browse files Browse the repository at this point in the history
The C data interface may expose null data pointers for zero-sized buffers.
Make sure that all buffer pointers remain non-null internally.

Followup to GH-14805.
  • Loading branch information
pitrou committed Jan 17, 2023
1 parent c525b57 commit e67c687
Showing 1 changed file with 22 additions and 6 deletions.
28 changes: 22 additions & 6 deletions cpp/src/arrow/c/bridge.cc
Expand Up @@ -31,6 +31,7 @@
#include "arrow/c/util_internal.h"
#include "arrow/extension_type.h"
#include "arrow/memory_pool.h"
#include "arrow/memory_pool_internal.h" // for kZeroSizeArea
#include "arrow/record_batch.h"
#include "arrow/result.h"
#include "arrow/stl_allocator.h"
Expand Down Expand Up @@ -60,6 +61,8 @@ using internal::SchemaExportTraits;

using internal::ToChars;

using memory_pool::internal::kZeroSizeArea;

namespace {

Status ExportingNotImplemented(const DataType& type) {
Expand Down Expand Up @@ -1265,7 +1268,8 @@ class ImportedBuffer : public Buffer {
};

struct ArrayImporter {
explicit ArrayImporter(const std::shared_ptr<DataType>& type) : type_(type) {}
explicit ArrayImporter(const std::shared_ptr<DataType>& type)
: type_(type), zero_size_buffer_(std::make_shared<Buffer>(kZeroSizeArea, 0)) {}

Status Import(struct ArrowArray* src) {
if (ArrowArrayIsReleased(src)) {
Expand Down Expand Up @@ -1529,7 +1533,7 @@ struct ArrayImporter {
}

Status ImportNullBitmap(int32_t buffer_id = 0) {
RETURN_NOT_OK(ImportBitsBuffer(buffer_id));
RETURN_NOT_OK(ImportBitsBuffer(buffer_id, /*is_null_bitmap=*/true));
if (data_->null_count > 0 && data_->buffers[buffer_id] == nullptr) {
return Status::Invalid(
"ArrowArray struct has null bitmap buffer but non-zero null_count ",
Expand All @@ -1538,10 +1542,10 @@ struct ArrayImporter {
return Status::OK();
}

Status ImportBitsBuffer(int32_t buffer_id) {
Status ImportBitsBuffer(int32_t buffer_id, bool is_null_bitmap = false) {
// Compute visible size of buffer
int64_t buffer_size = bit_util::BytesForBits(c_struct_->length + c_struct_->offset);
return ImportBuffer(buffer_id, buffer_size);
return ImportBuffer(buffer_id, buffer_size, is_null_bitmap);
}

Status ImportFixedSizeBuffer(int32_t buffer_id, int64_t byte_width) {
Expand All @@ -1567,13 +1571,22 @@ struct ArrayImporter {
return ImportBuffer(buffer_id, buffer_size);
}

Status ImportBuffer(int32_t buffer_id, int64_t buffer_size) {
Status ImportBuffer(int32_t buffer_id, int64_t buffer_size,
bool is_null_bitmap = false) {
std::shared_ptr<Buffer>* out = &data_->buffers[buffer_id];
auto data = reinterpret_cast<const uint8_t*>(c_struct_->buffers[buffer_id]);
if (data != nullptr) {
*out = std::make_shared<ImportedBuffer>(data, buffer_size, import_);
} else {
} else if (is_null_bitmap) {
out->reset();
} else {
// Ensure that imported buffers are never null (except for the null bitmap)
if (buffer_size != 0) {
return Status::Invalid(
"ArrowArrayStruct contains null data pointer "
"for a buffer with non-zero computed size");
}
*out = zero_size_buffer_;
}
return Status::OK();
}
Expand All @@ -1585,6 +1598,9 @@ struct ArrayImporter {
std::shared_ptr<ImportedArrayData> import_;
std::shared_ptr<ArrayData> data_;
std::vector<ArrayImporter> child_importers_;

// For imported null buffer pointers
std::shared_ptr<Buffer> zero_size_buffer_;
};

} // namespace
Expand Down

0 comments on commit e67c687

Please sign in to comment.