[Tarantool-patches] [PATCH 1/4] test: add infrastructure for fuzzing testing and fuzzers
Sergey Bronnikov
sergeyb at tarantool.org
Sun Dec 13 21:56:32 MSK 2020
Igor, many thanks for review!
I've fixed patches and pushed to the branch (but left them as a separate
commits with prefix [TO SQUASH]).
On 07.12.2020 20:24, Igor Munkin wrote:
> Sergey,
>
> Thanks for the patch! Please consider the remaining comments below.
>
> On 30.11.20, sergeyb at tarantool.org wrote:
>> From: Sergey Bronnikov <sergeyb at tarantool.org>
>>
>> There is a number of bugs related to parsing and encoding/decoding data.
>> Examples:
>>
>> - csv: #2692, #4497, #2692
>> - uri: #585
>>
>> One of the effective method to find such issues is a fuzzing testing.
>> Patch introduce a CMake flag to enable building fuzzers (ENABLE_FUZZER)
> Typo: s/introduce/introduces/.
Fixed.
>
>> and add fuzzers based on LibFuzzer [1] to csv, http_parser and uri modules.
>> NOTE: LibFuzzer requires Clang compiler.
>>
>> [1] https://llvm.org/docs/LibFuzzer.html
>>
>> How-To Use:
>>
>> $ mkdir build && cd build
>> $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON -DENABLE_ASAN=ON -DCMAKE_BUILD_TYPE=Debug ..
>> $ make fuzzers
>> $ ./test/fuzz/csv_fuzzer -max_total_time=60*60*60 -workers=4 ../test/static/corpus/csv
> I tried your recipe for the current revision and got the following:
> | $ ./test/fuzz/csv_fuzzer -max_total_time=60*60*60 -workers=4 ../test/static/corpus/csv
> | INFO: Seed: 2899369680
> | INFO: Loaded 1 modules (3 inline 8-bit counters): 3 [0x57a130, 0x57a133),
> | INFO: Loaded 1 PC tables (3 PCs): 3 [0x553870,0x5538a0),
> | No such file or directory: ../test/static/corpus/csv; exiting
>
> AFAICS, the required directory is added in the following patch, so I
> checkout the branch HEAD and try once more:
> | $ ./test/fuzz/csv_fuzzer -max_total_time=60*60*60 -workers=4 ../test/static/corpus/csv
> | INFO: Seed: 1838565059
> | INFO: Loaded 1 modules (3 inline 8-bit counters): 3 [0x57a130, 0x57a133),
> | INFO: Loaded 1 PC tables (3 PCs): 3 [0x553870,0x5538a0),
> | INFO: 21 files found in ../test/static/corpus/csv
> | INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
> | INFO: seed corpus: files: 21 min: 1b max: 462b total: 1336b rss: 27Mb
> | csv_fuzzer: /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerLoop.cpp:537: void fuzzer::Fuzzer::ExecuteCallback(const uint8_t *, size_t): Assertion `Res == 0' failed.
> | ==15230== ERROR: libFuzzer: deadly signal
> | #0 0x507287 in __sanitizer_print_stack_trace /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/asan/asan_stack.cc:38:3
> | #1 0x44f978 in fuzzer::PrintStackTrace() /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerUtil.cpp:206:5
> | #2 0x4300f3 in fuzzer::Fuzzer::CrashCallback() /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerLoop.cpp:237:3
> | #3 0x4300b0 in fuzzer::Fuzzer::StaticCrashSignalCallback() /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerLoop.cpp:209:6
> | #4 0x7f179300c8bf (/lib64/libpthread.so.0+0x148bf)
> | #5 0x7f1792bfdf3a in gsignal (/lib64/libc.so.6+0x38f3a)
> | #6 0x7f1792be7534 in abort (/lib64/libc.so.6+0x22534)
> | #7 0x7f1792be740e in __tls_get_addr (/lib64/libc.so.6+0x2240e)
> | #8 0x7f1792bf5731 in __assert_fail (/lib64/libc.so.6+0x30731)
> | #9 0x431d06 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerLoop.cpp:537:5
> | #10 0x4310d5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerLoop.cpp:455:3
> | #11 0x433aad in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerLoop.cpp:745:7
> | #12 0x434240 in fuzzer::Fuzzer::Loop(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerLoop.cpp:768:3
> | #13 0x425e60 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerDriver.cpp:760:6
> | #14 0x450132 in main /var/tmp/portage/sys-libs/compiler-rt-sanitizers-8.0.1/work/compiler-rt-8.0.1.src/lib/fuzzer/FuzzerMain.cpp:20:10
> | #15 0x7f1792be8eda in __libc_start_main (/lib64/libc.so.6+0x23eda)
> | #16 0x41e919 in _start (/tarantool/build/test/fuzz/csv_fuzzer+0x41e919)
> |
> | NOTE: libFuzzer has rudimentary signal handlers.
> | Combine libFuzzer with AddressSanitizer or similar for better crash reports.
> | SUMMARY: libFuzzer: deadly signal
> | MS: 0 ; base unit: 0000000000000000000000000000000000000000
> | 0x22,0x61,0x62,0x63,0x22,0x2c,0x20,0x22,0x77,0x69,0x74,0x68,0x2c,0x63,0x6f,0x6d,0x6d,0x61,0x22,0x2c,0x20,0x22,0x5c,0x22,0x69,0x6e,0x20,0x71,0x75,0x6f,0x74,0x65,0x73,0x5c,0x22,0x22,0x2c,0x20,0x22,0x31,0x20,0x5c,0x22,0x20,0x71,0x75,0x6f,0x74,0x65,0x22,0xa, \"abc\", \"with,comma\", \"\\\"in quotes\\\"\", \"1 \\\" quote\"\x0a
> | artifact_prefix='./'; Test unit written to ./crash-6d131d28c6e20c3a0a0b46c3aa7308d3029ab636
> | Base64: ImFiYyIsICJ3aXRoLGNvbW1hIiwgIlwiaW4gcXVvdGVzXCIiLCAiMSBcIiBxdW90ZSIK
>
> I have no idea whether it is OK but this does look like it's not. Maybe
> there are some problems with my compiler/sanitizer? JFYI, the toolchain
> is the following:
> | $ clang -v
> | clang version 8.0.1 (tags/RELEASE_801/final)
> | Target: x86_64-pc-linux-gnu
> | Thread model: posix
> | InstalledDir: /usr/lib/llvm/8/bin
> | Selected GCC installation: /usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0
> | Candidate multilib: .;@m64
> | Candidate multilib: 32;@m32
> | Selected multilib: .;@m64
> | $ clang++ -v
> | clang version 8.0.1 (tags/RELEASE_801/final)
> | Target: x86_64-pc-linux-gnu
> | Thread model: posix
> | InstalledDir: /usr/lib/llvm/8/bin
> | Selected GCC installation: /usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0
> | Candidate multilib: .;@m64
> | Candidate multilib: 32;@m32
> | Selected multilib: .;@m64
Message with assert is definitely not ok. LibFuzzer documentation says
that all fuzzers must return 0 only [1].
--- a/test/fuzz/csv_fuzzer.c
+++ b/test/fuzz/csv_fuzzer.c
@@ -9,15 +9,14 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
size) {
csv_create(&csv);
char *buf = calloc(size, sizeof(char*));
if (buf == NULL)
- return -1;
+ return 0;
memcpy(buf, data, size);
buf[size] = '\0';
char *end = buf + size;
csv_parse_chunk(&csv, buf, end);
csv_finish_parsing(&csv);
- int rc = csv_get_error_status(&csv) == CSV_ER_INVALID ? 1 : 0;
csv_destroy(&csv);
free(buf);
- return rc;
+ return 0;
}
diff --git a/test/fuzz/http_parser_fuzzer.c b/test/fuzz/http_parser_fuzzer.c
index a0aaf6786..f2dd7d09a 100644
--- a/test/fuzz/http_parser_fuzzer.c
+++ b/test/fuzz/http_parser_fuzzer.c
@@ -9,10 +9,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
size) {
http_parser_create(&parser);
parser.hdr_name = (char *)calloc((int)size, sizeof(char));
if (parser.hdr_name == NULL)
- return -1;
+ return 0;
char *end_buf = buf + size;
- int rc = http_parse_header_line(&parser, &buf, end_buf, size);
+ http_parse_header_line(&parser, &buf, end_buf, size);
free(parser.hdr_name);
- return rc;
+ return 0;
}
--- a/test/fuzz/uri_fuzzer.c
+++ b/test/fuzz/uri_fuzzer.c
@@ -8,12 +8,12 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
size)
{
char *buf = calloc(size, sizeof(char*));
if (!buf)
- return -1;
+ return 0;
strncpy(buf, (char*)data, size);
buf[size] = '\0';
struct uri uri;
- int rc = uri_parse(&uri, buf);
+ uri_parse(&uri, buf);
free(buf);
- return rc;
+ return 0;
}
For the rest I believe the reason is
| NOTE: libFuzzer has rudimentary signal handlers.
| Combine libFuzzer with AddressSanitizer or similar for better crash reports.
>
>> Part of #1809
>> ---
>> CMakeLists.txt | 2 +-
>> cmake/profile.cmake | 13 ++++++++++
>> test/CMakeLists.txt | 3 +++
>> test/fuzz/CMakeLists.txt | 45 ++++++++++++++++++++++++++++++++++
>> test/fuzz/csv_fuzzer.c | 23 +++++++++++++++++
>> test/fuzz/http_parser_fuzzer.c | 18 ++++++++++++++
>> test/fuzz/uri_fuzzer.c | 19 ++++++++++++++
>> 7 files changed, 122 insertions(+), 1 deletion(-)
>> create mode 100644 test/fuzz/CMakeLists.txt
>> create mode 100644 test/fuzz/csv_fuzzer.c
>> create mode 100644 test/fuzz/http_parser_fuzzer.c
>> create mode 100644 test/fuzz/uri_fuzzer.c
>>
> <snipped>
>
>> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
>> index 10882c6a1..d20a4eb5d 100644
>> --- a/test/CMakeLists.txt
>> +++ b/test/CMakeLists.txt
>> @@ -75,6 +75,9 @@ add_subdirectory(app-tap)
>> add_subdirectory(box)
>> add_subdirectory(box-tap)
>> add_subdirectory(unit)
>> +if(ENABLE_FUZZER)
>> + add_subdirectory(fuzz)
>> +endif()
> Minor: Well, I don't get the idea *why* this change is added here: it
> neither takes the place in alphabetical order, nor is added to the end
> of the list.
Sure, sorted alphabetically now.
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -74,10 +74,10 @@ add_subdirectory(app)
add_subdirectory(app-tap)
add_subdirectory(box)
add_subdirectory(box-tap)
-add_subdirectory(unit)
if(ENABLE_FUZZER)
add_subdirectory(fuzz)
endif()
+add_subdirectory(unit)
add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test
${PROJECT_BINARY_DIR}/third_party/luajit/test)
>> add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test
>> ${PROJECT_BINARY_DIR}/third_party/luajit/test)
>>
>> diff --git a/test/fuzz/CMakeLists.txt b/test/fuzz/CMakeLists.txt
>> new file mode 100644
>> index 000000000..142d38f67
>> --- /dev/null
>> +++ b/test/fuzz/CMakeLists.txt
>> @@ -0,0 +1,45 @@
>> +include_directories(${PROJECT_SOURCE_DIR}/src)
>> +include_directories(${PROJECT_BINARY_DIR}/src)
> Minor: It would be nice to explicitly mention the line above is added
> for autogenerated headers and LuaJIT ones. Feel free to ignore.
Fixed.
--- a/test/fuzz/CMakeLists.txt
+++ b/test/fuzz/CMakeLists.txt
@@ -1,3 +1,4 @@
+# Added for autogenerated headers and LuaJIT ones.
include_directories(${PROJECT_SOURCE_DIR}/src)
include_directories(${PROJECT_BINARY_DIR}/src)
include_directories(${PROJECT_SOURCE_DIR}/src/box)
@@ -50,8 +51,8 @@ add_executable(http_parser_fuzzer http_parser_fuzzer.c)
target_link_libraries(http_parser_fuzzer PUBLIC http_parser fuzzer_config)
>
>> +include_directories(${PROJECT_SOURCE_DIR}/src/box)
>> +
>> +# A special target with fuzzer and sanitizer flags.
>> +add_library(fuzzer_config INTERFACE)
>> +
>> +target_compile_options(
>> + fuzzer_config
>> + INTERFACE
>> + $<$<BOOL:${ENABLE_ASAN}>:
>> + -fsanitize=fuzzer,address
>> + >
>> + $<$<BOOL:${ENABLE_UB_SANITIZER}>:
>> + -fsanitize=fuzzer,undefined
>> + >
>> +)
>> +target_link_libraries(
>> + fuzzer_config
>> + INTERFACE
>> + $<$<BOOL:${ENABLE_ASAN}>:
>> + -fsanitize=fuzzer,address
>> + >
>> + $<$<BOOL:${ENABLE_UB_SANITIZER}>:
>> + -fsanitize=fuzzer,undefined
>> + >
>> +)
> OK, I ran <make fuzzers> with more verbose output and have two notes
> regarding it.
> | Scanning dependencies of target csv
> | make[3]: Leaving directory '/tarantool/build'
> | make -f src/lib/csv/CMakeFiles/csv.dir/build.make src/lib/csv/CMakeFiles/csv.dir/build
> | make[3]: Entering directory '/tarantool/build'
> | [ 0%] Building C object src/lib/csv/CMakeFiles/csv.dir/csv.c.o
> | cd /tarantool/build/src/lib/csv && /usr/lib/llvm/9/bin/clang-9 -DCORO_ASM
> | -DLUAJIT_SMART_STRINGS=1 -DLUAJIT_USE_ASAN=1 -DLUA_USE_APICHECK=1
> | -DLUA_USE_ASSERT=1 -DNVALGRIND=1 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
> | -D__STDC_CONSTANT_MACROS=1 -D__STDC_FORMAT_MACROS=1 -D__STDC_LIMIT_MACROS=1
> | -I/tarantool/src -I/tarantool/build/src -I/tarantool/src/lib
> | -I/tarantool/src/lib/small -I/tarantool/src/lib/small/third_party
> | -I/tarantool/src/lib/core -I/tarantool -I/tarantool/third_party/zstd/lib
> | -I/tarantool/third_party/zstd/lib/common -I/tarantool/build/third_party
> | -I/tarantool/third_party -I/tarantool/third_party/coro
> | -I/tarantool/third_party/luajit/src -I/tarantool/third_party/libyaml/include
> | -I/tarantool/src/lib/msgpuck -I/tarantool/build/build/curl/dest/include
> | -I/tarantool/build/third_party/decNumber
> | -I/tarantool/third_party/libutil_freebsd -fexceptions -funwind-tables
> | -fno-omit-frame-pointer -fno-stack-protector -fno-common -fopenmp -msse2
> | -fsanitize=address -fsanitize-blacklist=/tarantool/asan/asan.supp
> | -std=c11 -Wall -Wextra -Wno-strict-aliasing -Wno-char-subscripts
> | -Wno-gnu-alignof-expression -Werror -g -ggdb -O0 -UASAN_INTERFACE_OLD
> | -o CMakeFiles/csv.dir/csv.c.o -c /tarantool/src/lib/csv/csv.c
> | [ 0%] Linking C static library libcsv.a
> | cd /tarantool/build/src/lib/csv && /usr/bin/cmake -P CMakeFiles/csv.dir/cmake_clean_target.cmake
> | cd /tarantool/build/src/lib/csv && /usr/bin/cmake -E cmake_link_script CMakeFiles/csv.dir/link.txt --verbose=1
> | /usr/bin/ar qc libcsv.a CMakeFiles/csv.dir/csv.c.o
> | /usr/bin/ranlib libcsv.a
> | make[3]: Leaving directory '/tarantool/build'
> | [ 0%] Built target csv
> | make -f test/fuzz/CMakeFiles/csv_fuzzer.dir/build.make test/fuzz/CMakeFiles/csv_fuzzer.dir/depend
> | make[3]: Entering directory '/tarantool/build'
> | cd /tarantool/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles"
> | /tarantool /tarantool/test/fuzz /tarantool/build /tarantool/build/test/fuzz
> | /tarantool/build/test/fuzz/CMakeFiles/csv_fuzzer.dir/DependInfo.cmake --color=
> | Dependee "/tarantool/build/test/fuzz/CMakeFiles/csv_fuzzer.dir/DependInfo.cmake"
> | is newer than depender "/tarantool/build/test/fuzz/CMakeFiles/csv_fuzzer.dir/depend.internal".
> | Dependee "/tarantool/build/test/fuzz/CMakeFiles/CMakeDirectoryInformation.cmake"
> | is newer than depender "/tarantool/build/test/fuzz/CMakeFiles/csv_fuzzer.dir/depend.internal".
> | Scanning dependencies of target csv_fuzzer
> | make[3]: Leaving directory '/tarantool/build'
> | make -f test/fuzz/CMakeFiles/csv_fuzzer.dir/build.make test/fuzz/CMakeFiles/csv_fuzzer.dir/build
> | make[3]: Entering directory '/tarantool/build'
> | [ 0%] Building C object test/fuzz/CMakeFiles/csv_fuzzer.dir/csv_fuzzer.c.o
> | cd /tarantool/build/test/fuzz && /usr/lib/llvm/9/bin/clang-9 -DCORO_ASM
> | -DLUAJIT_SMART_STRINGS=1 -DLUAJIT_USE_ASAN=1 -DLUA_USE_APICHECK=1
> | -DLUA_USE_ASSERT=1 -DNVALGRIND=1 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
> | -D__STDC_CONSTANT_MACROS=1 -D__STDC_FORMAT_MACROS=1 -D__STDC_LIMIT_MACROS=1
> | -I/tarantool/src -I/tarantool/build/src -I/tarantool/src/lib
> | -I/tarantool/src/lib/small -I/tarantool/src/lib/small/third_party
> | -I/tarantool/src/lib/core -I/tarantool -I/tarantool/third_party/zstd/lib
> | -I/tarantool/third_party/zstd/lib/common -I/tarantool/third_party/luajit/src
> | -I/tarantool/src/lib/msgpuck -I/tarantool/src/box -fexceptions
> | -funwind-tables -fno-omit-frame-pointer -fno-stack-protector -fno-common -fopenmp -msse2
> | -fsanitize=address -fsanitize-blacklist=/tarantool/asan/asan.supp
> | -std=c11 -Wall -Wextra -Wno-strict-aliasing -Wno-char-subscripts
> | -Wno-gnu-alignof-expression -Werror -Wno-unused-parameter -g -ggdb -O0
> | -UASAN_INTERFACE_OLD -fsanitize=fuzzer,address
> | -o CMakeFiles/csv_fuzzer.dir/csv_fuzzer.c.o -c /tarantool/test/fuzz/csv_fuzzer.c
> | [ 0%] Linking C executable csv_fuzzer
> | cd /tarantool/build/test/fuzz && /usr/bin/cmake -E cmake_link_script CMakeFiles/csv_fuzzer.dir/link.txt --verbose=1
> | /usr/lib/llvm/9/bin/clang-9 -fexceptions -funwind-tables
> | -fno-omit-frame-pointer -fno-stack-protector -fno-common -fopenmp -msse2
> | -fsanitize=address -fsanitize-blacklist=/tarantool/asan/asan.supp
> | -std=c11 -Wall -Wextra -Wno-strict-aliasing -Wno-char-subscripts
> | -Wno-gnu-alignof-expression -Werror -Wno-unused-parameter -g -ggdb -O0
> | -rdynamic CMakeFiles/csv_fuzzer.dir/csv_fuzzer.c.o -o csv_fuzzer
> | ../../src/lib/csv/libcsv.a -fsanitize=fuzzer,address
> | make[3]: Leaving directory '/tarantool/build'
> | [ 0%] Built target csv_fuzzer
>
> 1. I'm totally not an expert, but quite confused with the fact the
> libcsv is build w/o <fuzzer> flag, but csv_fuzzer is build with it.
You are right. Project source code should be instrumented too and I
enable it:
diff --git a/cmake/profile.cmake b/cmake/profile.cmake
index 45e3d112c..308d1b0fb 100644
--- a/cmake/profile.cmake
+++ b/cmake/profile.cmake
@@ -53,6 +53,9 @@ if(ENABLE_FUZZER)
" $ CC=clang CXX=clang++ cmake . <...> -DENABLE_FUZZER=ON
&& make -j\n"
"\n")
endif()
+ if (NOT OSS_FUZZ)
+ add_compile_flags("C;CXX" -fsanitize=fuzzer-no-link)
+ endif()
endif()
option(ENABLE_ASAN "Enable AddressSanitizer, a fast memory error
detector based on compiler instrumentation" OFF)
You can easily check that option is passed to every source file
(although for us interested csv, uri and http_parser libraries)
when CMAKE_EXPORT_COMPILE_COMMANDS is enabled and passed to CMake.
Entry for src/lib/csv.c contains -fsanitize=fuzzer-no-link in a
compile_commands.json:
{
"directory": "/home/sergeyb/sources/MRG/tarantool/build/src/lib/csv",
"command": "/usr/bin/clang -DCORO_ASM -DLUAJIT_SMART_STRINGS=1
-DLUA_USE_APICHECK=1 -DLUA_USE_ASSERT=1 -DNVALGRIND=1
-D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS=1
-D__STDC_FORMAT_MACROS=1 -D__STDC_LIMIT_MACROS=1
-I/home/sergeyb/sources/MRG/tarantool/src
-I/home/sergeyb/sources/MRG/tarantool/build/src
-I/home/sergeyb/sources/MRG/tarantool/src/lib
-I/home/sergeyb/sources/MRG/tarantool/src/lib/small
-I/home/sergeyb/sources/MRG/tarantool/src/lib/small/third_party
-I/home/sergeyb/sources/MRG/tarantool/src/lib/core
-I/home/sergeyb/sources/MRG/tarantool
-I/home/sergeyb/sources/MRG/tarantool/third_party/zstd/lib
-I/home/sergeyb/sources/MRG/tarantool/third_party/zstd/lib/common
-I/home/sergeyb/sources/MRG/tarantool/build/third_party
-I/home/sergeyb/sources/MRG/tarantool/third_party
-I/home/sergeyb/sources/MRG/tarantool/third_party/coro
-I/home/sergeyb/sources/MRG/tarantool/third_party/luajit/src
-I/home/sergeyb/sources/MRG/tarantool/third_party/libyaml/include
-I/home/sergeyb/sources/MRG/tarantool/src/lib/msgpuck
-I/home/sergeyb/sources/MRG/tarantool/build/build/curl/dest/include
-I/home/sergeyb/sources/MRG/tarantool/build/third_party/decNumber
-I/home/sergeyb/sources/MRG/tarantool/third_party/libutil_freebsd
-fexceptions -funwind-tables -fno-common -fopenmp -msse2
*-fsanitize=fuzzer-no-link* -std=c11 -Wall -Wextra -Wno-strict-aliasing
-Wno-char-subscripts -Wno-gnu-alignof-expression -Werror -g -ggdb -O0
-o CMakeFiles/csv.dir/csv.c.o -c
/home/sergeyb/sources/MRG/tarantool/src/lib/csv/csv.c",
"file": "/home/sergeyb/sources/MRG/tarantool/src/lib/csv/csv.c"
},
You may not understand why options with "-fsanitize=fuzzer" two times
(in cmake/profile.cmake and test/fuzz/CMakeLists.txt). I'll clarify it
in advance:
- cmake/profile.cmake is for project source files,
-fsanitize=fuzzer-no-link option allows to instrument project source
files for fuzzing, but LibFuzzer will not replace main() in these files.
- test/fuzz/CMakeLists.txt uses -fsanitize=fuzzer and not
-fsanitize=fuzzer-no-link because we want to add automatically generated
main() for each fuzzer.
> 2. Do you need to specify <address> flag once more, when ASAN is
> enabled? If not the hunk above looks excess, doesn't it?
Agree, it was a bad idea to manage UBSan and ASAN flags in yet another
place.
Moreover we don't use all flags provided by UBSan. There is an
explanation in [2].
I have updated compilation and link flags in test/fuzz/CMakeLists.txt:
--- a/test/fuzz/CMakeLists.txt
+++ b/test/fuzz/CMakeLists.txt
@@ -9,14 +9,8 @@ add_library(fuzzer_config INTERFACE)
target_compile_options(
fuzzer_config
INTERFACE
- $<$<BOOL:${ENABLE_ASAN}>:
- -fsanitize=fuzzer,address
- >
- $<$<BOOL:${ENABLE_UB_SANITIZER}>:
- -fsanitize=fuzzer,undefined
- >
$<$<NOT:$<BOOL:${OSS_FUZZ}>>:
- -fsanitize=fuzzer
+ -fsanitize=fuzzer
>
$<$<BOOL:${OSS_FUZZ}>:
${CXX}
@@ -26,14 +20,8 @@ target_compile_options(
target_link_libraries(
fuzzer_config
INTERFACE
- $<$<BOOL:${ENABLE_ASAN}>:
- -fsanitize=fuzzer,address
- >
- $<$<BOOL:${ENABLE_UB_SANITIZER}>:
- -fsanitize=fuzzer,undefined
- >
$<$<NOT:$<BOOL:${OSS_FUZZ}>>:
- -fsanitize=fuzzer
+ -fsanitize=fuzzer
>
$<$<BOOL:${OSS_FUZZ}>:
$ENV{LIB_FUZZING_ENGINE}
>> +
> <snipped>
>
>> +
>> +set(fuzzing_binaries csv_fuzzer
>> + http_parser_fuzzer
>> + uri_fuzzer)
> Spaces are used for indentation in CMake-related sources, not tabs.
> Surprisingly, you made it the right way below.
Fixed.
--- a/test/fuzz/CMakeLists.txt
+++ b/test/fuzz/CMakeLists.txt
@@ -1,3 +1,4 @@
+# Added for autogenerated headers and LuaJIT ones.
include_directories(${PROJECT_SOURCE_DIR}/src)
include_directories(${PROJECT_BINARY_DIR}/src)
include_directories(${PROJECT_SOURCE_DIR}/src/box)
@@ -50,8 +51,8 @@ add_executable(http_parser_fuzzer http_parser_fuzzer.c)
target_link_libraries(http_parser_fuzzer PUBLIC http_parser fuzzer_config)
set(fuzzing_binaries csv_fuzzer
- http_parser_fuzzer
- uri_fuzzer)
+ http_parser_fuzzer
+ uri_fuzzer)
add_custom_target(fuzzers
DEPENDS ${fuzzing_binaries}
>
>> +
>> +add_custom_target(fuzzers
>> + DEPENDS ${fuzzing_binaries}
>> + COMMENT "Build fuzzers")
>> diff --git a/test/fuzz/csv_fuzzer.c b/test/fuzz/csv_fuzzer.c
>> new file mode 100644
>> index 000000000..8853d6308
>> --- /dev/null
>> +++ b/test/fuzz/csv_fuzzer.c
> */me feeling myself like a parrot*
>
> Why do you violate our style guides[1] using spaces instead of tabs for
> indentation? IIRC I've already mentioned it here[2]...
>
> Otherwise this hunk looks fine.
Fixed indentation and placed function type on the same line with
function name.
Code style also recommend to use goto(), but I believe that LibFuzzer
someday will start to accept different exit codes
and probably it is better to keep code as is without using goto().
>
>> @@ -0,0 +1,23 @@
> <snipped>
>
>> diff --git a/test/fuzz/http_parser_fuzzer.c b/test/fuzz/http_parser_fuzzer.c
>> new file mode 100644
>> index 000000000..a0aaf6786
>> --- /dev/null
>> +++ b/test/fuzz/http_parser_fuzzer.c
> Ditto.
same as for csv_fuzzer.c
>
>> @@ -0,0 +1,18 @@
> <snipped>
>
>> diff --git a/test/fuzz/uri_fuzzer.c b/test/fuzz/uri_fuzzer.c
>> new file mode 100644
>> index 000000000..8397505bd
>> --- /dev/null
>> +++ b/test/fuzz/uri_fuzzer.c
> Ditto.
same as for csv_fuzzer.c
>
>> @@ -0,0 +1,19 @@
> <snipped>
>
>> 2.25.1
>>
> [1]: https://www.tarantool.io/en/doc/latest/dev_guide/c_style_guide/#chapter-1-indentation
> [2]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/016409.html
>
Also added a warning that triggered when someone use ENABLE_FUZZER and
OSS_FUZZ without
environment variable LIB_FUZZING_ENGINE:
--- a/cmake/profile.cmake
+++ b/cmake/profile.cmake
@@ -53,6 +53,13 @@ if(ENABLE_FUZZER)
" $ CC=clang CXX=clang++ cmake . <...> -DENABLE_FUZZER=ON
&& make -j\n"
"\n")
endif()
+ if(OSS_FUZZ AND NOT DEFINED ENV{LIB_FUZZING_ENGINE})
+ message(SEND_ERROR
+ "OSS-Fuzz builds require the environment variable "
+ "LIB_FUZZING_ENGINE to be set. If you are seeing this "
+ "warning, it points to a deeper problem in the ossfuzz "
+ "build setup.")
+ endif()
if (NOT OSS_FUZZ)
add_compile_flags("C;CXX" -fsanitize=fuzzer-no-link)
endif()
1. http://llvm.org/docs/LibFuzzer.html#id22
2.
https://github.com/tarantool/tarantool/blob/master/cmake/compiler.cmake#L290-L320
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20201213/2a4777d5/attachment.html>
More information about the Tarantool-patches
mailing list