From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id F179F45C304 for ; Sun, 13 Dec 2020 21:56:33 +0300 (MSK) References: <20201207172444.GE5396@tarantool.org> From: Sergey Bronnikov Message-ID: <51ad8f7a-c804-7e06-ceb9-0c8adcc56d16@tarantool.org> Date: Sun, 13 Dec 2020 21:56:32 +0300 MIME-Version: 1.0 In-Reply-To: <20201207172444.GE5396@tarantool.org> Content-Type: multipart/alternative; boundary="------------A76F018F309FFC852B3D6E1F" Content-Language: en-US Subject: Re: [Tarantool-patches] [PATCH 1/4] test: add infrastructure for fuzzing testing and fuzzers List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org This is a multi-part message in MIME format. --------------A76F018F309FFC852B3D6E1F Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit 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@tarantool.org wrote: >> From: Sergey Bronnikov >> >> 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::allocator >, fuzzer::fuzzer_allocator, std::allocator > > > 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::allocator >, fuzzer::fuzzer_allocator, std::allocator > > > 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 >> > > >> 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 >> + $<$: >> + -fsanitize=fuzzer,address >> + > >> + $<$: >> + -fsanitize=fuzzer,undefined >> + > >> +) >> +target_link_libraries( >> + fuzzer_config >> + INTERFACE >> + $<$: >> + -fsanitize=fuzzer,address >> + > >> + $<$: >> + -fsanitize=fuzzer,undefined >> + > >> +) > OK, I ran 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 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
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 -        $<$: -        -fsanitize=fuzzer,address -        > -        $<$: -        -fsanitize=fuzzer,undefined -        >          $<$>: -          -fsanitize=fuzzer +        -fsanitize=fuzzer          >          $<$:          ${CXX} @@ -26,14 +20,8 @@ target_compile_options(  target_link_libraries(      fuzzer_config      INTERFACE -        $<$: -        -fsanitize=fuzzer,address -        > -        $<$: -        -fsanitize=fuzzer,undefined -        >          $<$>: -          -fsanitize=fuzzer +        -fsanitize=fuzzer          >          $<$:          $ENV{LIB_FUZZING_ENGINE} >> + > > >> + >> +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 @@ > > >> 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 @@ > > >> 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 @@ > > >> 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 --------------A76F018F309FFC852B3D6E1F Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit

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@tarantool.org wrote:

From: Sergey Bronnikov <sergeyb@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

  


--------------A76F018F309FFC852B3D6E1F--