[
https://issues.apache.org/jira/browse/ARROW-2275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16392994#comment-16392994
]
ASF GitHub Bot commented on ARROW-2275:
---------------------------------------
wesm closed pull request #1717: ARROW-2275: [C++] Guard against bad use of
Buffer.mutable_data()
URL: https://github.com/apache/arrow/pull/1717
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/cpp/src/arrow/buffer.cc b/cpp/src/arrow/buffer.cc
index 29e2c242a..e32e02c9f 100644
--- a/cpp/src/arrow/buffer.cc
+++ b/cpp/src/arrow/buffer.cc
@@ -70,6 +70,14 @@ Status Buffer::FromString(const std::string& data,
std::shared_ptr<Buffer>* out)
return FromString(data, default_memory_pool(), out);
}
+#ifndef NDEBUG
+// DCHECK macros aren't allowed in public include files
+uint8_t* Buffer::mutable_data() {
+ DCHECK(is_mutable());
+ return mutable_data_;
+}
+#endif
+
PoolBuffer::PoolBuffer(MemoryPool* pool) : ResizableBuffer(nullptr, 0) {
if (pool == nullptr) {
pool = default_memory_pool();
diff --git a/cpp/src/arrow/buffer.h b/cpp/src/arrow/buffer.h
index cf25ccd03..ad11ff943 100644
--- a/cpp/src/arrow/buffer.h
+++ b/cpp/src/arrow/buffer.h
@@ -54,7 +54,11 @@ class ARROW_EXPORT Buffer {
///
/// \note The passed memory must be kept alive through some other means
Buffer(const uint8_t* data, int64_t size)
- : is_mutable_(false), data_(data), size_(size), capacity_(size) {}
+ : is_mutable_(false),
+ data_(data),
+ mutable_data_(NULLPTR),
+ size_(size),
+ capacity_(size) {}
/// \brief Construct from std::string without copying memory
///
@@ -113,7 +117,11 @@ class ARROW_EXPORT Buffer {
int64_t capacity() const { return capacity_; }
const uint8_t* data() const { return data_; }
+#ifdef NDEBUG
uint8_t* mutable_data() { return mutable_data_; }
+#else
+ uint8_t* mutable_data();
+#endif
int64_t size() const { return size_; }
diff --git a/cpp/src/arrow/tensor.h b/cpp/src/arrow/tensor.h
index 4e4c6b8d5..699dc0393 100644
--- a/cpp/src/arrow/tensor.h
+++ b/cpp/src/arrow/tensor.h
@@ -71,7 +71,7 @@ class ARROW_EXPORT Tensor {
std::shared_ptr<Buffer> data() const { return data_; }
const uint8_t* raw_data() const { return data_->data(); }
- uint8_t* raw_data() { return data_->mutable_data(); }
+ uint8_t* raw_mutable_data() { return data_->mutable_data(); }
const std::vector<int64_t>& shape() const { return shape_; }
const std::vector<int64_t>& strides() const { return strides_; }
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> [C++] Buffer::mutable_data_ member uninitialized
> ------------------------------------------------
>
> Key: ARROW-2275
> URL: https://issues.apache.org/jira/browse/ARROW-2275
> Project: Apache Arrow
> Issue Type: Bug
> Components: C++
> Affects Versions: 0.8.0
> Reporter: Antoine Pitrou
> Priority: Minor
> Labels: pull-request-available
> Fix For: 0.9.0
>
>
> For immutable buffers (i.e. most of them), the {{mutable_data_}} member is
> uninitialized. If the user calls {{mutable_data()}} by mistake on such a
> buffer, they will get a bogus pointer back.
> This is exacerbated by the Tensor API whose const and non-const
> {{raw_data()}} methods return different things...
> (also an idea: add a DCHECK for mutability before returning from
> {{mutable_data()}}?)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
