This is an automated email from the ASF dual-hosted git repository.
uwe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new e0328b0 ARROW-2023: [C++] Fix ASAN failure on malformed / empty
stream input, enable ASAN builds, add more dev docs
e0328b0 is described below
commit e0328b0af5ea80116d266bb270260a9f9605ebcc
Author: Wes McKinney <[email protected]>
AuthorDate: Tue Feb 27 10:38:08 2018 +0100
ARROW-2023: [C++] Fix ASAN failure on malformed / empty stream input,
enable ASAN builds, add more dev docs
More to do here, see discussion in
https://issues.apache.org/jira/browse/ARROW-1589
Author: Wes McKinney <[email protected]>
Closes #1503 from wesm/ARROW-2023 and squashes the following commits:
e3a590e <Wes McKinney> Do not invoke memcpy unless number of bytes to read
greater than 0. Return more informative error message
90787ec <Wes McKinney> Add ASAN instructions, llvm-symbolizer instructions
for fuzzers. Remove Apache Kudu cruft from run-test.sh
53d2c92 <Wes McKinney> Add missing lsan-suppressions.txt to enable ASAN to
run with unittests
69cb5b7 <Wes McKinney> Start C++ stream reader tests for malformed inputs
---
cpp/CMakeLists.txt | 7 ++++---
cpp/README.md | 25 ++++++++++++++++++++++++-
cpp/build-support/lsan-suppressions.txt | 19 +++++++++++++++++++
cpp/build-support/run-test.sh | 6 ------
cpp/src/arrow/io/memory.cc | 6 ++++--
cpp/src/arrow/ipc/ipc-read-write-test.cc | 16 ++++++++++++++++
cpp/src/arrow/ipc/message.cc | 5 ++++-
7 files changed, 71 insertions(+), 13 deletions(-)
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index b922141..8c0e956 100644
--- a/cpp/CMakeLists.txt
+++ b/cpp/CMakeLists.txt
@@ -291,8 +291,6 @@ include(ThirdpartyToolchain)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_COMMON_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${ARROW_CXXFLAGS}")
-message(STATUS "CMAKE_CXX_FLAGS: ${CMAKE_CXX_FLAGS}")
-
if ("${COMPILER_FAMILY}" STREQUAL "clang")
# Using Clang with ccache causes a bunch of spurious warnings that are
# purportedly fixed in the next version of ccache. See the following for
details:
@@ -305,7 +303,7 @@ endif()
# ASAN / TSAN / UBSAN
if(ARROW_FUZZING)
- set(ARROW_USE_COVERAGE ON)
+ set(ARROW_USE_COVERAGE ON)
endif()
include(san-config)
@@ -333,6 +331,9 @@ if ("${ARROW_GENERATE_COVERAGE}")
endif()
endif()
+# CMAKE_CXX_FLAGS now fully assembled
+message(STATUS "CMAKE_CXX_FLAGS: ${CMAKE_CXX_FLAGS}")
+
# set compile output directory
string (TOLOWER ${CMAKE_BUILD_TYPE} BUILD_SUBDIR_NAME)
diff --git a/cpp/README.md b/cpp/README.md
index 1daf863..daeeade 100644
--- a/cpp/README.md
+++ b/cpp/README.md
@@ -99,7 +99,14 @@ and benchmarks or `make runbenchmark` to run only the
benchmark tests.
Benchmark logs will be placed in the build directory under
`build/benchmark-logs`.
-## Building/Running fuzzers
+### Testing with LLVM AddressSanitizer
+
+To use AddressSanitizer (ASAN) to find bad memory accesses or leaks with LLVM,
+pass `-DARROW_USE_ASAN=ON` when building. You must use clang to compile with
+ASAN, and `ARROW_USE_ASAN` is mutually-exclusive with the valgrind option
+`ARROW_TEST_MEMCHECK`.
+
+### Building/Running fuzzers
Fuzzers can help finding unhandled exceptions and problems with untrusted
input that
may lead to crashes, security issues and undefined behavior. They do this by
@@ -128,6 +135,22 @@ stack trace as well as the input data. After a problem was
found this way, it sh
be reported and fixed. Usually, the fuzzing process cannot be continued until
the
fix is applied, since the fuzzer usually converts to the problem again.
+If you build fuzzers with ASAN, you need to set the `ASAN_SYMBOLIZER_PATH`
+environment variable to the absolute path of `llvm-symbolizer`, which is a tool
+that ships with LLVM.
+
+```shell
+export ASAN_SYMBOLIZER_PATH=$(type -p llvm-symbolizer)
+```
+
+Note that some fuzzer builds currently reject paths with a version qualifier
+(like `llvm-sanitizer-5.0`). To overcome this, set an appropriate symlink
+(here, when using LLVM 5.0):
+
+```shell
+ln -sf /usr/bin/llvm-sanitizer-5.0 /usr/bin/llvm-sanitizer
+```
+
There are some problems that may occur during the compilation process:
- libfuzzer was not distributed with your LLVM: `ld: file not found:
.../libLLVMFuzzer.a`
diff --git a/cpp/build-support/lsan-suppressions.txt
b/cpp/build-support/lsan-suppressions.txt
new file mode 100644
index 0000000..927afb3
--- /dev/null
+++ b/cpp/build-support/lsan-suppressions.txt
@@ -0,0 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# False positive from atexit() registration in libc
+leak:*__new_exitfn*
diff --git a/cpp/build-support/run-test.sh b/cpp/build-support/run-test.sh
index b4da4f3..656ab7b 100755
--- a/cpp/build-support/run-test.sh
+++ b/cpp/build-support/run-test.sh
@@ -88,12 +88,6 @@ function setup_sanitizers() {
# Set up suppressions for LeakSanitizer
LSAN_OPTIONS="$LSAN_OPTIONS
suppressions=$ROOT/build-support/lsan-suppressions.txt"
export LSAN_OPTIONS
-
- # Suppressions require symbolization. We'll default to using the symbolizer
in
- # thirdparty.
- if [ -z "$ASAN_SYMBOLIZER_PATH" ]; then
- export ASAN_SYMBOLIZER_PATH=$(find $NATIVE_TOOLCHAIN/llvm-3.7.0/bin -name
llvm-symbolizer)
- fi
}
function run_test() {
diff --git a/cpp/src/arrow/io/memory.cc b/cpp/src/arrow/io/memory.cc
index ecdf26f..45d6bdb 100644
--- a/cpp/src/arrow/io/memory.cc
+++ b/cpp/src/arrow/io/memory.cc
@@ -257,9 +257,11 @@ Status BufferReader::Tell(int64_t* position) const {
bool BufferReader::supports_zero_copy() const { return true; }
Status BufferReader::Read(int64_t nbytes, int64_t* bytes_read, void* buffer) {
- memcpy(buffer, data_ + position_, nbytes);
*bytes_read = std::min(nbytes, size_ - position_);
- position_ += *bytes_read;
+ if (*bytes_read) {
+ memcpy(buffer, data_ + position_, *bytes_read);
+ position_ += *bytes_read;
+ }
return Status::OK();
}
diff --git a/cpp/src/arrow/ipc/ipc-read-write-test.cc
b/cpp/src/arrow/ipc/ipc-read-write-test.cc
index 1fcbdac..d877e99 100644
--- a/cpp/src/arrow/ipc/ipc-read-write-test.cc
+++ b/cpp/src/arrow/ipc/ipc-read-write-test.cc
@@ -755,5 +755,21 @@ TEST_F(TestTensorRoundTrip, NonContiguous) {
CheckTensorRoundTrip(tensor);
}
+TEST(TestRecordBatchStreamReader, MalformedInput) {
+ const std::string empty_str = "";
+ const std::string garbage_str = "12345678";
+
+ auto empty = std::make_shared<Buffer>(empty_str);
+ auto garbage = std::make_shared<Buffer>(garbage_str);
+
+ std::shared_ptr<RecordBatchReader> batch_reader;
+
+ io::BufferReader empty_reader(empty);
+ ASSERT_RAISES(Invalid, RecordBatchStreamReader::Open(&empty_reader,
&batch_reader));
+
+ io::BufferReader garbage_reader(garbage);
+ ASSERT_RAISES(Invalid, RecordBatchStreamReader::Open(&garbage_reader,
&batch_reader));
+}
+
} // namespace ipc
} // namespace arrow
diff --git a/cpp/src/arrow/ipc/message.cc b/cpp/src/arrow/ipc/message.cc
index 1835cef..fd74756 100644
--- a/cpp/src/arrow/ipc/message.cc
+++ b/cpp/src/arrow/ipc/message.cc
@@ -227,7 +227,10 @@ Status ReadMessage(io::InputStream* file,
std::unique_ptr<Message>* message) {
std::shared_ptr<Buffer> metadata;
RETURN_NOT_OK(file->Read(message_length, &metadata));
if (metadata->size() != message_length) {
- return Status::IOError("Unexpected end of stream trying to read message");
+ std::stringstream ss;
+ ss << "Expected to read " << message_length << " metadata bytes, but "
+ << "only read " << metadata->size();
+ return Status::Invalid(ss.str());
}
return Message::ReadFrom(metadata, file, message);
--
To stop receiving notification emails like this one, please contact
[email protected].
