This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new ab7a6af3eed GH-51649: [C++][Parquet] Tell encoding fuzzer to skip
inputs with invalid parameters (#51650)
ab7a6af3eed is described below
commit ab7a6af3eed42f3429369082a067280b05f7c4dd
Author: Antoine Pitrou <[email protected]>
AuthorDate: Tue Oct 6 08:48:00 2026 +0200
GH-51649: [C++][Parquet] Tell encoding fuzzer to skip inputs with invalid
parameters (#51650)
### Rationale for this change
`LLVMFuzzerTestOneInput` allows two return values: 0 and -1. While 0 allows
for the input to be (potentially) saved in the corpus, -1 tells the fuzzer not
to save it.
In the words of the [fine
documentation](https://llvm.org/docs/LibFuzzer.html#rejecting-unwanted-inputs):
> It may be desirable to reject some inputs, i.e. to not add them to the
corpus.
> For example, when fuzzing an API consisting of parsing and other logic,
one may want
> to allow only those inputs into the corpus that parse successfully.
> If the fuzz target returns -1 on a given input, libFuzzer will not add
that
> input to the corpus, regardless of what coverage it triggers.
### What changes are included in this PR?
1. Add a `FuzzStatus` type that allows telling the fuzzing engine whether
an input should be skipped (i.e. not saved in the corpus).
2. Skip inputs where the parameter header contains invalid values in the
Parquet encoding fuzzer
### Are these changes tested?
Manually, by running the fuzzer from the command line and watching logs
with `ARROW_FUZZING_VERBOSITY` enabled.
Without a seed corpus and trying to generate 100k inputs
(`./build/out/arrow/parquet-encoding-fuzz -seed=42 -runs=100000`), I get:
* before:
```
#100000 DONE cov: 1377 ft: 2265 corp: 46/3357b lim: 778 exec/s: 7142 rss:
201Mb
Done 100000 runs in 14 second(s)
```
* after:
```
#100000 DONE cov: 2051 ft: 3723 corp: 41/5725b lim: 832 exec/s: 50000
rss: 259Mb
Done 100000 runs in 2 second(s)
```
It seems that this PR allows the fuzzer to proceed much faster when
mutation generates such inputs. Despite running faster, it also reaches more
coverage (see [doc](https://llvm.org/docs/LibFuzzer.html#output) about the
`cov:` and `ft:` values).
(*caveat:* these measurements is with a fixed random seed, and without a
seed corpus)
(of course, ideally the fuzzer would not produce such inputs at all, but
ensuring that is [more
involved](https://llvm.org/docs/LibFuzzer.html#user-supplied-mutators))
### Are there any user-facing changes?
No.
### Was AI used for this PR?
In accordance to the [AI generation
guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code),
please disclose below whether and how AI was used in this PR.
**PR code and description written by:**
- [x] Human
- [ ] AI
**Reviewed before submission by:**
- [x] Human
- [ ] AI
- [ ] Not reviewed
* GitHub Issue: #51649
Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/csv/fuzz.cc | 3 +-
cpp/src/arrow/ipc/file_fuzz.cc | 3 +-
cpp/src/arrow/ipc/stream_fuzz.cc | 3 +-
cpp/src/arrow/util/fuzz_internal.cc | 22 +++++++++----
cpp/src/arrow/util/fuzz_internal.h | 29 ++++++++++++++++-
cpp/src/parquet/arrow/encoding_fuzz.cc | 3 +-
cpp/src/parquet/arrow/fuzz.cc | 3 +-
cpp/src/parquet/arrow/fuzz_encoding_internal.cc | 21 ++++++++++---
cpp/src/parquet/arrow/fuzz_encoding_internal.h | 4 ++-
cpp/src/parquet/decoder.cc | 41 +++++++++++++++++++------
cpp/src/parquet/encoding.h | 8 ++++-
cpp/src/parquet/encoding_test.cc | 2 ++
12 files changed, 109 insertions(+), 33 deletions(-)
diff --git a/cpp/src/arrow/csv/fuzz.cc b/cpp/src/arrow/csv/fuzz.cc
index 39148f1aacd..2219efa2cb5 100644
--- a/cpp/src/arrow/csv/fuzz.cc
+++ b/cpp/src/arrow/csv/fuzz.cc
@@ -121,6 +121,5 @@ Status FuzzCsvReader(const uint8_t* data, int64_t size) {
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
auto status = arrow::csv::FuzzCsvReader(data, static_cast<int64_t>(size));
- arrow::internal::LogFuzzStatus(status, data, size);
- return 0;
+ return arrow::internal::LogFuzzStatus(status, data, size);
}
diff --git a/cpp/src/arrow/ipc/file_fuzz.cc b/cpp/src/arrow/ipc/file_fuzz.cc
index c3ce9c2dea6..2d2a924befc 100644
--- a/cpp/src/arrow/ipc/file_fuzz.cc
+++ b/cpp/src/arrow/ipc/file_fuzz.cc
@@ -24,6 +24,5 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
auto status = arrow::ipc::internal::FuzzIpcFile(data,
static_cast<int64_t>(size));
- arrow::internal::LogFuzzStatus(status, data, static_cast<int64_t>(size));
- return 0;
+ return arrow::internal::LogFuzzStatus(status, data,
static_cast<int64_t>(size));
}
diff --git a/cpp/src/arrow/ipc/stream_fuzz.cc b/cpp/src/arrow/ipc/stream_fuzz.cc
index 04b12863bed..8aaef771260 100644
--- a/cpp/src/arrow/ipc/stream_fuzz.cc
+++ b/cpp/src/arrow/ipc/stream_fuzz.cc
@@ -24,6 +24,5 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
auto status = arrow::ipc::internal::FuzzIpcStream(data,
static_cast<int64_t>(size));
- arrow::internal::LogFuzzStatus(status, data, static_cast<int64_t>(size));
- return 0;
+ return arrow::internal::LogFuzzStatus(status, data,
static_cast<int64_t>(size));
}
diff --git a/cpp/src/arrow/util/fuzz_internal.cc
b/cpp/src/arrow/util/fuzz_internal.cc
index 28d210333dd..813652f49ae 100644
--- a/cpp/src/arrow/util/fuzz_internal.cc
+++ b/cpp/src/arrow/util/fuzz_internal.cc
@@ -34,7 +34,7 @@ MemoryPool* fuzzing_memory_pool() {
return pool.get();
}
-void LogFuzzStatus(const Status& st, const uint8_t* data, int64_t size) {
+int LogFuzzStatus(const FuzzStatus& st, const uint8_t* data, int64_t size) {
static const int kVerbosity = []() {
auto maybe_env_value =
GetEnvVarInteger("ARROW_FUZZING_VERBOSITY", /*min_value=*/0,
/*max_value=*/1);
@@ -48,13 +48,23 @@ void LogFuzzStatus(const Status& st, const uint8_t* data,
int64_t size) {
return 0;
}();
- if (!st.ok() && kVerbosity >= 1) {
- ARROW_LOG(WARNING) << "Fuzzing input with size=" << size
- << " failed: " << st.ToString();
- } else if (st.IsOutOfMemory()) {
+ const auto& reason = FuzzReason(st);
+ bool skip_input = std::holds_alternative<SkipFuzzInput>(st);
+ if (skip_input) {
+ if (kVerbosity >= 1) {
+ ARROW_LOG(WARNING) << "Skipping input with size=" << size << ": "
+ << reason.ToString();
+ }
+ } else if (reason.IsOutOfMemory()) {
ARROW_LOG(WARNING) << "Fuzzing input with size=" << size
- << " hit allocation failure: " << st.ToString();
+ << " hit allocation failure: " << reason.ToString();
+ } else {
+ if (!reason.ok() && kVerbosity >= 1) {
+ ARROW_LOG(WARNING) << "Fuzzing input with size=" << size
+ << " failed: " << reason.ToString();
+ }
}
+ return skip_input ? -1 : 0;
}
} // namespace arrow::internal
diff --git a/cpp/src/arrow/util/fuzz_internal.h
b/cpp/src/arrow/util/fuzz_internal.h
index 5280b7ec1ff..685570e5cf7 100644
--- a/cpp/src/arrow/util/fuzz_internal.h
+++ b/cpp/src/arrow/util/fuzz_internal.h
@@ -18,12 +18,33 @@
#pragma once
#include <cstdint>
+#include <variant>
+#include "arrow/status.h"
#include "arrow/type_fwd.h"
#include "arrow/util/macros.h"
namespace arrow::internal {
+// A tag used to signal the fuzzing engine that this input should not be saved
+// into the corpus.
+struct SkipFuzzInput {
+ Status reason;
+};
+
+// The Status alternative holds a regular fuzzing success or failure,
+// while the SkipFuzzInput alternative holds a setup failure (e.g. invalid
+// fuzzing parameters encoded in the payload).
+using FuzzStatus = std::variant<Status, SkipFuzzInput>;
+
+inline const Status& FuzzReason(const FuzzStatus& st) {
+ struct Visitor {
+ const Status& operator()(const SkipFuzzInput& v) { return v.reason; }
+ const Status& operator()(const Status& v) { return v; }
+ };
+ return std::visit(Visitor{}, st);
+}
+
// The default rss_limit_mb on OSS-Fuzz is 2560 MB and we want to fail
allocations
// before that limit is reached, otherwise the fuzz target gets killed
(GH-48105).
constexpr int64_t kFuzzingMemoryLimit = 2200LL * 1000 * 1000;
@@ -32,6 +53,12 @@ constexpr int64_t kFuzzingMemoryLimit = 2200LL * 1000 * 1000;
ARROW_EXPORT MemoryPool* fuzzing_memory_pool();
/// Optionally log the outcome of fuzzing an input
-ARROW_EXPORT void LogFuzzStatus(const Status&, const uint8_t* data, int64_t
size);
+///
+/// Returns the integer code to return from LLVMFuzzerTestOneInput.
+ARROW_EXPORT int LogFuzzStatus(const FuzzStatus&, const uint8_t* data, int64_t
size);
+
+inline int LogFuzzStatus(const Status& status, const uint8_t* data, int64_t
size) {
+ return LogFuzzStatus(FuzzStatus{status}, data, size);
+}
} // namespace arrow::internal
diff --git a/cpp/src/parquet/arrow/encoding_fuzz.cc
b/cpp/src/parquet/arrow/encoding_fuzz.cc
index 64095795e28..3a968a3df87 100644
--- a/cpp/src/parquet/arrow/encoding_fuzz.cc
+++ b/cpp/src/parquet/arrow/encoding_fuzz.cc
@@ -22,6 +22,5 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
auto status =
parquet::fuzzing::internal::FuzzEncoding(data,
static_cast<int64_t>(size));
- arrow::internal::LogFuzzStatus(status, data, static_cast<int64_t>(size));
- return 0;
+ return arrow::internal::LogFuzzStatus(status, data,
static_cast<int64_t>(size));
}
diff --git a/cpp/src/parquet/arrow/fuzz.cc b/cpp/src/parquet/arrow/fuzz.cc
index c2d8021a5a5..9cf26eb1f2b 100644
--- a/cpp/src/parquet/arrow/fuzz.cc
+++ b/cpp/src/parquet/arrow/fuzz.cc
@@ -21,6 +21,5 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
auto status = parquet::fuzzing::internal::FuzzReader(data,
static_cast<int64_t>(size));
- arrow::internal::LogFuzzStatus(status, data, static_cast<int64_t>(size));
- return 0;
+ return arrow::internal::LogFuzzStatus(status, data,
static_cast<int64_t>(size));
}
diff --git a/cpp/src/parquet/arrow/fuzz_encoding_internal.cc
b/cpp/src/parquet/arrow/fuzz_encoding_internal.cc
index a0077391167..9b8917897c0 100644
--- a/cpp/src/parquet/arrow/fuzz_encoding_internal.cc
+++ b/cpp/src/parquet/arrow/fuzz_encoding_internal.cc
@@ -23,8 +23,10 @@
#include <functional>
#include <limits>
#include <new>
+#include <optional>
#include <sstream>
#include <string_view>
+#include <typeinfo>
#include "arrow/array.h"
#include "arrow/array/builder_binary.h"
@@ -52,6 +54,7 @@ using ::arrow::MemoryPool;
using ::arrow::Result;
using ::arrow::Status;
using ::arrow::TypedBufferBuilder;
+using ::arrow::internal::FuzzStatus;
using ::parquet::arrow::FileReader;
ColumnDescriptor MakeColumnDescriptor(Type::type type, int type_length) {
@@ -140,6 +143,10 @@ std::string FuzzEncodingHeader::Serialize() const {
static_cast<Encoding::type>(ph.roundtrip_encoding_id),
static_cast<Type::type>(ph.type_id),
ph.type_length,
ph.num_values);
+ if (!IsEncodingSupported(header.type, header.source_encoding) ||
+ !IsEncodingSupported(header.type, header.roundtrip_encoding)) {
+ return invalid_payload();
+ }
if ((header.type == Type::FIXED_LEN_BYTE_ARRAY) ? (header.type_length <= 0)
: (header.type_length !=
-1)) {
return invalid_payload();
@@ -492,13 +499,17 @@ struct TypedFuzzEncoding {
} // namespace
-Status FuzzEncoding(const uint8_t* data, int64_t size) {
+FuzzStatus FuzzEncoding(const uint8_t* data, int64_t size) {
constexpr auto kInt32Max = std::numeric_limits<int32_t>::max();
- ARROW_ASSIGN_OR_RAISE(const auto parse_result,
- FuzzEncodingHeader::Parse(std::span(data, size)));
- const auto header = parse_result.first;
- const auto encoded_data = parse_result.second;
+ auto maybe_parse_result = FuzzEncodingHeader::Parse(std::span(data, size));
+ if (!maybe_parse_result.ok()) {
+ // If the fuzz encoding header is invalid, we won't save this input
+ // in the corpus, because it didn't exercise anything interesting.
+ return ::arrow::internal::SkipFuzzInput{maybe_parse_result.status()};
+ }
+ const auto header = maybe_parse_result->first;
+ const auto encoded_data = maybe_parse_result->second;
if (encoded_data.size() > static_cast<size_t>(kInt32Max)) {
// Unlikely but who knows?
return Status::Invalid("Fuzz payload too large");
diff --git a/cpp/src/parquet/arrow/fuzz_encoding_internal.h
b/cpp/src/parquet/arrow/fuzz_encoding_internal.h
index 92d6b2834cc..8ce17eadc41 100644
--- a/cpp/src/parquet/arrow/fuzz_encoding_internal.h
+++ b/cpp/src/parquet/arrow/fuzz_encoding_internal.h
@@ -27,6 +27,7 @@
#include "arrow/result.h"
#include "arrow/status.h"
#include "arrow/type_fwd.h"
+#include "arrow/util/fuzz_internal.h"
#include "arrow/util/macros.h"
#include "parquet/platform.h"
#include "parquet/types.h"
@@ -76,7 +77,8 @@ struct FuzzEncodingHeader {
};
/// Fuzz a payload encoded as explained in FuzzEncodingHeader
-PARQUET_EXPORT ::arrow::Status FuzzEncoding(const uint8_t* data, int64_t size);
+PARQUET_EXPORT ::arrow::internal::FuzzStatus FuzzEncoding(const uint8_t* data,
+ int64_t size);
PARQUET_EXPORT ColumnDescriptor MakeColumnDescriptor(Type::type type,
int type_length = -1);
diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc
index 47f2b46ebaa..c63d01bb16d 100644
--- a/cpp/src/parquet/decoder.cc
+++ b/cpp/src/parquet/decoder.cc
@@ -2557,27 +2557,50 @@ std::unique_ptr<Decoder> MakeDictDecoder(Type::type
type_num,
} // namespace detail
-std::vector<Encoding::type> SupportedEncodings(Type::type physical_type) {
+namespace {
+
+// Discriminant allows definition of distinct functions for different
initializers.
+template <auto Discriminant, typename T>
+const std::vector<T>& ConstVectorRef(std::initializer_list<T> init) {
+ static const auto vec = std::vector<T>(std::move(init));
+ return vec;
+}
+
+const std::vector<Encoding::type>& SupportedEncodingsRef(Type::type
physical_type) {
switch (physical_type) {
case Type::BOOLEAN:
- return {Encoding::PLAIN, Encoding::RLE};
+ return ConstVectorRef<Type::BOOLEAN>({Encoding::PLAIN, Encoding::RLE});
case Type::INT32:
case Type::INT64:
- return {Encoding::PLAIN, Encoding::DELTA_BINARY_PACKED,
- Encoding::BYTE_STREAM_SPLIT};
+ return ConstVectorRef<Type::INT32>(
+ {Encoding::PLAIN, Encoding::DELTA_BINARY_PACKED,
Encoding::BYTE_STREAM_SPLIT});
case Type::INT96:
- return {Encoding::PLAIN};
+ return ConstVectorRef<Type::INT96>({Encoding::PLAIN});
case Type::FLOAT:
case Type::DOUBLE:
- return {Encoding::PLAIN, Encoding::BYTE_STREAM_SPLIT};
+ return ConstVectorRef<Type::FLOAT>({Encoding::PLAIN,
Encoding::BYTE_STREAM_SPLIT});
case Type::FIXED_LEN_BYTE_ARRAY:
- return {Encoding::PLAIN, Encoding::BYTE_STREAM_SPLIT,
Encoding::DELTA_BYTE_ARRAY};
+ return ConstVectorRef<Type::FIXED_LEN_BYTE_ARRAY>(
+ {Encoding::PLAIN, Encoding::BYTE_STREAM_SPLIT,
Encoding::DELTA_BYTE_ARRAY});
case Type::BYTE_ARRAY:
- return {Encoding::PLAIN, Encoding::DELTA_LENGTH_BYTE_ARRAY,
- Encoding::DELTA_BYTE_ARRAY};
+ return ConstVectorRef<Type::BYTE_ARRAY>({Encoding::PLAIN,
+
Encoding::DELTA_LENGTH_BYTE_ARRAY,
+ Encoding::DELTA_BYTE_ARRAY});
default:
throw ParquetException("Invalid physical type");
}
}
+} // namespace
+
+const std::vector<Encoding::type>& SupportedEncodings(Type::type
physical_type) {
+ return SupportedEncodingsRef(physical_type);
+}
+
+bool IsEncodingSupported(Type::type physical_type, Encoding::type encoding) {
+ const auto& supported_encodings = SupportedEncodingsRef(physical_type);
+ return std::find(supported_encodings.begin(), supported_encodings.end(),
encoding) !=
+ supported_encodings.end();
+}
+
} // namespace parquet
diff --git a/cpp/src/parquet/encoding.h b/cpp/src/parquet/encoding.h
index 9a0cc55aa18..157d67264c5 100644
--- a/cpp/src/parquet/encoding.h
+++ b/cpp/src/parquet/encoding.h
@@ -482,6 +482,12 @@ std::unique_ptr<typename EncodingTraits<DType>::Decoder>
MakeTypedDecoder(
///
/// Only non-dictionary encodings are returned.
PARQUET_EXPORT
-std::vector<Encoding::type> SupportedEncodings(Type::type physical_type);
+const std::vector<Encoding::type>& SupportedEncodings(Type::type
physical_type);
+
+/// Return whether the encoding is supported for the given physical type
+///
+/// Dictionary encodings return false.
+PARQUET_EXPORT
+bool IsEncodingSupported(Type::type physical_type, Encoding::type encoding);
} // namespace parquet
diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc
index 74b422cb606..6aa9256aa8a 100644
--- a/cpp/src/parquet/encoding_test.cc
+++ b/cpp/src/parquet/encoding_test.cc
@@ -77,8 +77,10 @@ void TestSupportedEncodingsConsistentWith(
if (std::find(supported_encodings.begin(), supported_encodings.end(),
encoding) !=
supported_encodings.end()) {
ASSERT_NO_THROW(func(type, encoding, descr));
+ ASSERT_TRUE(IsEncodingSupported(type, encoding));
} else {
ASSERT_THROW(func(type, encoding, descr), ParquetException);
+ ASSERT_FALSE(IsEncodingSupported(type, encoding));
}
}
}
