Feat: fuzz tests #763

waldgange · 2025-05-28T13:28:46Z

This PR inroduces a new job to sanitizers workflow. The job builds Fuzz-tests and their dependencies instrumented with address sanitizer. This job is launched in parallel with other sanitizers after a PR is approved. The fuzz tests sources reside in src/standalones/s_bmqfuzz. Three tests are added so far:

s_bmqfuzz_eval.cpp targets BloombergLP::bmqeval::SimpleEvaluator::compile
s_bmqfuzz_parseutil.cpp targets BloombergLP::mqbcmd::ParseUtil::parse
s_bmqfuzz_bmqt_uri.cpp targets BloombergLP::bmqt::UriParser::parse

There is also a script for building them locally bin/build-fuzz.sh

bmq-oss-ci

Build 2700 of commit caff4b7 has completed with FAILURE

bmq-oss-ci

Build 2702 of commit 927b21f has completed with FAILURE

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

bmq-oss-ci

Build 2712 of commit c89f0a2 has completed with FAILURE

CMakeLists.txt

hallfox · 2025-06-02T19:03:17Z

bin/build-fuzz.sh

+CMAKE_OPTIONS=(\
+    -DBDE_BUILD_TARGET_64=ON \
+    -DBDE_BUILD_TARGET_SAFE=ON \
+    -DBDE_BUILD_TARGET_CPP20=ON \
+    -DCMAKE_BUILD_TYPE=Debug \
+    -DCMAKE_INSTALL_LIBDIR="lib" \
+    -DCMAKE_INSTALL_PREFIX="${DIR_INSTALL}" \
+    -DCMAKE_MODULE_PATH="${DIR_THIRDPARTY}/bde-tools/cmake;${DIR_THIRDPARTY}/bde-tools/BdeBuildSystem" \
+    -DCMAKE_PREFIX_PATH="${DIR_INSTALL}" \
+    -DCMAKE_TOOLCHAIN_FILE="${DIR_THIRDPARTY}/bde-tools/BdeBuildSystem/toolchains/linux/clang-default.cmake" \
+    -DCMAKE_CXX_FLAGS="-fsanitize=fuzzer-no-link" \
+    -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
+    -DINSTALL_TARGETS=fuzztests)


I think we should start moving these configs into CMakePresets.json, so we can manage all the commonalities between these build modes.

Can this be replaced with a preset called fuzz-tests?

added a preset for non-parametrized flags

hallfox · 2025-06-02T19:04:21Z

src/fuzz-tests/CMakeLists.txt

+
+file(GLOB sources "*fuzzer.cpp")
+foreach (source ${sources})
+  string(REGEX MATCH "\/([A-Za-z_]+)\.cpp$" _ ${source})


Why regex match if we glob all the .cpp files in the directory?

simplified this

hallfox · 2025-06-02T19:09:19Z

src/fuzz-tests/CMakeLists.txt

+  target_bmq_default_compiler_flags(${targetName})
+  target_include_directories(${targetName} PRIVATE ${fuzz-tests_INCLUDE_DIRS})
+  target_link_libraries(${targetName} PRIVATE ${fuzz-tests_PCDEPS})
+  target_link_options(${targetName} PRIVATE "-fsanitize=fuzzer")


Usually we set sanitizer flags in the toolchain file, we may want to provide one based off of the one we'd be using from https://github.com/bloomberg/bde-tools/tree/main/cmake/toolchains/linux

moved all the flags to docker/sanitizers/clang-libcxx-sanitizer.cmake

hallfox · 2025-06-02T19:10:42Z

src/fuzz-tests/CMakeLists.txt

+foreach (source ${sources})
+  string(REGEX MATCH "\/([A-Za-z_]+)\.cpp$" _ ${source})
+  set(targetName ${CMAKE_MATCH_1})
+  add_executable(${targetName} ${source})
+
+  target_bmq_default_compiler_flags(${targetName})
+  target_include_directories(${targetName} PRIVATE ${fuzz-tests_INCLUDE_DIRS})
+  target_link_libraries(${targetName} PRIVATE ${fuzz-tests_PCDEPS})
+  target_link_options(${targetName} PRIVATE "-fsanitize=fuzzer")
+
+  set_target_properties(${targetName}
+    PROPERTIES OUTPUT_NAME ${targetName})
+
+  # Add the current tutorial to the 'fuzztests' rule
+  add_dependencies("fuzztests" ${targetName})
+endforeach()


I think this is a nontrivial enough bit of CMake that it should be put in a function and documented

hallfox · 2025-06-02T19:12:53Z

CMakeLists.txt

+  if (installIT)
+    set(BMQ_TARGET_BMQ_NEEDED       YES)
+    set(BMQ_TARGET_MQB_NEEDED       YES)
+    set(BMQ_TARGET_BMQBRKR_NEEDED   YES)
+    set(BMQ_TARGET_BMQTOOL_NEEDED   YES)
+  endif()


Is this IT install rules related to fuzz testing? Can we have a separate PR for it if not?

Ok. It's not related directly to the fuzz tests

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

bmq-oss-ci

Build 2735 of commit db59087 has completed with FAILURE

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

bmq-oss-ci

Build 2739 of commit 9fb98f6 has completed with FAILURE

Co-authored-by: Taylor Foxhall <lxmachina@gmail.com> Signed-off-by: Anton Pryakhin <antonpryakhin@gmail.com>

bmq-oss-ci

Build 2749 of commit d7eb932 has completed with FAILURE

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

bmq-oss-ci

Build 2754 of commit e11b880 has completed with FAILURE

hallfox · 2025-06-17T21:29:15Z

docker/sanitizers/build_sanitizer.sh

+if [ "${FUZZER}" == "on" ]; then
+  export FUZZER_FLAG="fuzzer-no-link"
+else
+  export CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES="/usr/include;/usr/include/clang/${LLVM_VERSION}/include"


Why is this in the else branch?

The default standard library include paths contain libstdc++ headers. We don't want to change it for fuzz-tests. I didn't want to brake anything for the old workflows, so I desided to leave it in the else branch. However this doesn't look like correct include paths for libc++. We set this path properly later in the toolchain:

blazingmq/docker/sanitizers/clang-libcxx-sanitizer.cmake

Lines 48 to 52 in e1b5b1b

string(CONCAT TOOLCHAIN_CXX_FLAGS

"${TOOLCHAIN_CXX_FLAGS} "

"-stdlib=libc++ "

"-I${LIBCXX_BUILD_PATH}/include/c++/v1 "

)

So I've got rid of this branch at all and added a flag that removes default include paths for non-fuzz case

hallfox · 2025-06-17T21:34:03Z

docker/sanitizers/build_sanitizer.sh

+
+if [[ -z "${2}" || ("${2}" != "on" && "${2}" != "off") ]]; then
+    echo 'Error: Wrong fuzzer argument. It can be either "on" or "off"' >&2
+    echo '  (Usage: build_sanitizer.sh <sanitizer-name> <fuzzer>)' >&2


Can we add a section to the comment block at the top with a usage example?

docker/sanitizers/clang-libcxx-sanitizer.cmake

hallfox · 2025-06-17T21:51:22Z

docker/sanitizers/clang-libcxx-sanitizer.cmake

-       "-stdlib=libc++ "
-       "-I${LIBCXX_BUILD_PATH}/include/c++/v1 "
-      )
+if(DEFINED ENV{LIBCXX_BUILD_PATH})


In what build configuration are we using libc++? Just when fuzzers are disabled? I think we should be consistent about which standard library we use. It seems like we'd lose some instrumentation coverage if we don't custom build LLVM here. My understanding is we have this kind of matrix:

Sanitizer Fuzz? std choice

ASAN no libc++ instrumented

MSAN no libc++ instrumented

TSAN no libc++ instrumented

UBSAN no libc++ instrumented

ASAN yes libstd++

That makes me kind of think that the fuzz test builds are kind of exceptional and should probably have their own build rules

Correct. But these two workflows also have a lot in common - all dependencies, except the standard library, instumented with sanitizers. So it's very convenient to reuse this toolchain. Do we really want another toolchain for that?

Yeah I think that's fair

hallfox · 2025-06-17T22:36:10Z

src/fuzz-tests/CMakeLists.txt

+add_custom_target("fuzztests")
+
+find_package(BdeBuildSystem REQUIRED)
+bbs_read_metadata(PACKAGE fuzz-tests)


This is not a valid BDE package name, but BDE also doesn't really define a clear way for organizing a separate package with dedicated test executables. I'm going to recommend we invent the following conventions:

Fuzz tests should have the suffix .fuzz.cpp

This specific package of fuzz tests should live in src/standalones/s_bmqfuzz

(maybe we can consult with them on what their opinions about this would be?)

I've asked BDE team about fuzz tests. They build them in a slightly different way. They put each fuzz test to the test driver of the corresponding component. Here is an example. However they didn't integrate neither with BFuzz nor with oss-fuzz. They run their fuzz tests locally instead. What do you think? Should we do the same with our tests?

hallfox · 2025-06-17T22:39:05Z

src/fuzz-tests/CMakeLists.txt

+  add_dependencies("fuzztests" ${targetName})
+endfunction()
+
+file(GLOB sources "*fuzzer.cpp")


Probably should get this list from ${fuzz-tests_SOURCE_FILES}

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

Co-authored-by: Taylor Foxhall <lxmachina@gmail.com> Signed-off-by: Anton Pryakhin <antonpryakhin@gmail.com>

bmq-oss-ci

Build 2779 of commit 402061a has completed with FAILURE

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

bmq-oss-ci

Build 2780 of commit 67d8867 has completed with FAILURE

waldgange force-pushed the fuzztests branch from dfcf1a9 to caff4b7 Compare May 28, 2025 14:15

bmq-oss-ci bot reviewed May 28, 2025

View reviewed changes

waldgange force-pushed the fuzztests branch from caff4b7 to 927b21f Compare May 29, 2025 15:12

bmq-oss-ci bot reviewed May 29, 2025

View reviewed changes

10000

Feat: fuzz tests

c89f0a2

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

waldgange force-pushed the fuzztests branch from 927b21f to c89f0a2 Compare May 30, 2025 16:41

bmq-oss-ci bot reviewed May 30, 2025

View reviewed changes

chrisbeard assigned chrisbeard and hallfox Jun 2, 2025

hallfox reviewed Jun 2, 2025

View reviewed changes

Address review pt1

dbf6f5e

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

waldgange marked this pull request as ready for review June 11, 2025 13:29

waldgange requested a review from a team as a code owner June 11, 2025 13:29

waldgange added 2 commits June 11, 2025 16:30

Merge branch 'main' into fuzztests

db45f06

Formatting

db59087

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

bmq-oss-ci bot reviewed Jun 11, 2025

View reviewed changes

waldgange added 2 commits June 11, 2025 20:00

Move link flags to the toolchain

6a23fc2

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

Remove install rule

9fb98f6

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

waldgange requested a review from hallfox June 11, 2025 17:20

bmq-oss-ci bot reviewed Jun 11, 2025

View reviewed changes

Update CMakeLists.txt

d7eb932

Co-authored-by: Taylor Foxhall <lxmachina@gmail.com> Signed-off-by: Anton Pryakhin <antonpryakhin@gmail.com>

bmq-oss-ci bot reviewed Jun 13, 2025

View reviewed changes

Use preset

e11b880

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

bmq-oss-ci bot reviewed Jun 13, 2025

View reviewed changes

hallfox reviewed Jun 17, 2025

View reviewed changes

waldgange and others added 2 commits June 20, 2025 19:51

Address review pt2

6679231

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

Update docker/sanitizers/clang-libcxx-sanitizer.cmake

402061a

Co-authored-by: Taylor Foxhall <lxmachina@gmail.com> Signed-off-by: Anton Pryakhin <antonpryakhin@gmail.com>

waldgange requested a review from hallfox June 20, 2025 17:18

bmq-oss-ci bot reviewed Jun 20, 2025

View reviewed changes

Update readme

67d8867

Signed-off-by: Anton Pryakhin <apryakhin1@bloomberg.net>

bmq-oss-ci bot reviewed Jun 23, 2025

View reviewed changes

hallfox approved these changes Jun 25, 2025

View reviewed changes

waldgange merged commit 4285ece into bloomberg:main Jun 26, 2025
40 checks passed

	string(CONCAT TOOLCHAIN_CXX_FLAGS
	"${TOOLCHAIN_CXX_FLAGS} "
	"-stdlib=libc++ "
	"-I${LIBCXX_BUILD_PATH}/include/c++/v1 "
	)

Sanitizer	Fuzz?	std choice
ASAN	no	libc++ instrumented
MSAN	no	libc++ instrumented
TSAN	no	libc++ instrumented
UBSAN	no	libc++ instrumented
ASAN	yes	libstd++

Feat: fuzz tests #763

Feat: fuzz tests #763

Conversation

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!