* [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
@ 2020-04-20 8:47 Sergey Bronnikov
2020-04-24 13:56 ` Serge Petrenko
2020-04-28 11:19 ` Igor Munkin
0 siblings, 2 replies; 10+ messages in thread
From: Sergey Bronnikov @ 2020-04-20 8:47 UTC (permalink / raw)
To: tarantool-patches; +Cc: o.piskunov
- introduce CMake flag to enable building fuzzers
- add fuzzers based on LibFuzzer[1] to csv, http_parser and uri modules
[1] https://llvm.org/docs/LibFuzzer.html
Note: LibFuzzer requires Clang compiler
How-To Use:
$ mkdir build && cd build
$ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
$ make -j
$ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4
...
Follows: #1809
---
CMakeLists.txt | 2 +-
cmake/profile.cmake | 5 +++++
src/lib/csv/CMakeLists.txt | 11 +++++++++++
src/lib/csv/test_csv.c | 23 +++++++++++++++++++++++
src/lib/http_parser/CMakeLists.txt | 11 +++++++++++
src/lib/http_parser/test_http_parser.c | 22 ++++++++++++++++++++++
src/lib/uri/CMakeLists.txt | 12 ++++++++++++
src/lib/uri/test_uri.c | 22 ++++++++++++++++++++++
8 files changed, 107 insertions(+), 1 deletion(-)
create mode 100644 src/lib/csv/test_csv.c
create mode 100644 src/lib/http_parser/test_http_parser.c
create mode 100644 src/lib/uri/test_uri.c
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1d80b6806..99a502e3a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -571,7 +571,7 @@ set(PREFIX ${CMAKE_INSTALL_PREFIX})
set(options PACKAGE VERSION BUILD C_COMPILER CXX_COMPILER C_FLAGS CXX_FLAGS
PREFIX
ENABLE_SSE2 ENABLE_AVX
- ENABLE_GCOV ENABLE_GPROF ENABLE_VALGRIND ENABLE_ASAN
+ ENABLE_GCOV ENABLE_GPROF ENABLE_VALGRIND ENABLE_ASAN ENABLE_FUZZER
ENABLE_BACKTRACE
ENABLE_DOC
ENABLE_DIST
diff --git a/cmake/profile.cmake b/cmake/profile.cmake
index bc4bf67f5..b9fcd7655 100644
--- a/cmake/profile.cmake
+++ b/cmake/profile.cmake
@@ -42,6 +42,11 @@ else()
add_definitions(-DNVALGRIND=1)
endif()
+option(ENABLE_FUZZER "Enable fuzzing testing" OFF)
+if (ENABLE_FUZZER)
+ set(TESTING_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/Testing")
+endif ()
+
option(ENABLE_ASAN "Enable AddressSanitizer, a fast memory error detector based on compiler instrumentation" OFF)
if (ENABLE_ASAN)
if (CMAKE_COMPILER_IS_GNUCC)
diff --git a/src/lib/csv/CMakeLists.txt b/src/lib/csv/CMakeLists.txt
index 3580e4da2..d5a3ed1f6 100644
--- a/src/lib/csv/CMakeLists.txt
+++ b/src/lib/csv/CMakeLists.txt
@@ -4,3 +4,14 @@ set(lib_sources
set_source_files_compile_flags(${lib_sources})
add_library(csv STATIC ${lib_sources})
+
+if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+ set(TestName "test_csv")
+ add_executable(${TestName} ${TestName}.c)
+ set_target_properties(${TestName}
+ PROPERTIES
+ COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
+ LINK_FLAGS "-fsanitize=fuzzer,address")
+ target_link_libraries(${TestName} PRIVATE csv)
+ set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
+endif ()
diff --git a/src/lib/csv/test_csv.c b/src/lib/csv/test_csv.c
new file mode 100644
index 000000000..aa1703514
--- /dev/null
+++ b/src/lib/csv/test_csv.c
@@ -0,0 +1,23 @@
+#include <stdint.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <string.h>
+#include "csv.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+ struct csv csv;
+ csv_create(&csv);
+ char *buf = calloc(size, sizeof(char*));
+ if (buf == NULL) {
+ return -1;
+ }
+ memcpy(buf, data, size);
+ buf[size] = '\0';
+ char *end = buf + size;
+ csv_parse_chunk(&csv, buf, end);
+ csv_finish_parsing(&csv);
+ csv_destroy(&csv);
+ free(buf);
+
+ return 0;
+}
diff --git a/src/lib/http_parser/CMakeLists.txt b/src/lib/http_parser/CMakeLists.txt
index a48f83cb6..7af922fbb 100644
--- a/src/lib/http_parser/CMakeLists.txt
+++ b/src/lib/http_parser/CMakeLists.txt
@@ -1 +1,12 @@
add_library(http_parser STATIC http_parser.c)
+
+if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+ set(TestName "test_http_parser")
+ add_executable(${TestName} ${TestName}.c)
+ set_target_properties(${TestName}
+ PROPERTIES
+ COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
+ LINK_FLAGS "-fsanitize=fuzzer,address")
+ target_link_libraries(${TestName} PRIVATE http_parser)
+ set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
+endif ()
diff --git a/src/lib/http_parser/test_http_parser.c b/src/lib/http_parser/test_http_parser.c
new file mode 100644
index 000000000..a189e71e9
--- /dev/null
+++ b/src/lib/http_parser/test_http_parser.c
@@ -0,0 +1,22 @@
+#include <stdlib.h>
+#include <stdint.h>
+#include <stddef.h>
+#include "http_parser.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+ struct http_parser parser;
+ char *buf = (char*)data;
+ http_parser_create(&parser);
+ parser.hdr_name = (char *)calloc((int)size, sizeof(char));
+ if (parser.hdr_name == NULL) {
+ return -1;
+ }
+ char *end_buf = buf + size;
+ int rc = http_parse_header_line(&parser, &buf, end_buf, size);
+ free(parser.hdr_name);
+ if (rc != 0) {
+ return rc;
+ }
+
+ return 0;
+}
diff --git a/src/lib/uri/CMakeLists.txt b/src/lib/uri/CMakeLists.txt
index 96410e5bf..77f5c5d57 100644
--- a/src/lib/uri/CMakeLists.txt
+++ b/src/lib/uri/CMakeLists.txt
@@ -8,3 +8,15 @@ if (CC_HAS_WNO_IMPLICIT_FALLTHROUGH)
-Wno-implicit-fallthrough)
endif()
add_library(uri STATIC uri.c)
+
+if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+ set(TestName "test_uri")
+ add_executable(${TestName} ${TestName}.c)
+ add_compile_options(-fsanitize=fuzzer,address -g -O1)
+ set_target_properties(${TestName}
+ PROPERTIES
+ COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
+ LINK_FLAGS "-fsanitize=fuzzer,address")
+ target_link_libraries(${TestName} PRIVATE uri)
+ set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
+endif ()
diff --git a/src/lib/uri/test_uri.c b/src/lib/uri/test_uri.c
new file mode 100644
index 000000000..ad8db6ef2
--- /dev/null
+++ b/src/lib/uri/test_uri.c
@@ -0,0 +1,22 @@
+#include <stdlib.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <string.h>
+#include "uri.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+ char *buf = calloc(size, sizeof(char*));
+ if (!buf) {
+ return -1;
+ }
+ strncpy(buf, (char*)data, size);
+ buf[size] = '\0';
+ struct uri uri;
+ int rc = uri_parse(&uri, buf);
+ free(buf);
+ if (rc != 0) {
+ return rc;
+ }
+
+ return 0;
+}
--
2.23.0
--
sergeyb@
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
2020-04-20 8:47 [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers Sergey Bronnikov
@ 2020-04-24 13:56 ` Serge Petrenko
2020-04-29 9:28 ` Sergey Bronnikov
2020-04-28 11:19 ` Igor Munkin
1 sibling, 1 reply; 10+ messages in thread
From: Serge Petrenko @ 2020-04-24 13:56 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: o.piskunov, tml
Hi! Thanks for the patch!
I couldn’t build tarantool with ENABLE_FUZZER = ON on my machine, since I
don’t have LibFuzzer, even though I have clang and everything that Apple bundles
with it. I still have some comments though.
> 20 апр. 2020 г., в 11:47, Sergey Bronnikov <sergeyb@tarantool.org> написал(а):
>
> - introduce CMake flag to enable building fuzzers
> - add fuzzers based on LibFuzzer[1] to csv, http_parser and uri modules
>
> [1] https://llvm.org/docs/LibFuzzer.html
>
> Note: LibFuzzer requires Clang compiler
>
> How-To Use:
>
> $ mkdir build && cd build
> $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
> $ make -j
> $ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4
> ...
>
> Follows: #1809
> ---
Please attach the branch, like this:
https://github.com/tarantool/tarantool/tree/ligurio/gh-1809-libfuzzer
> CMakeLists.txt | 2 +-
> cmake/profile.cmake | 5 +++++
> src/lib/csv/CMakeLists.txt | 11 +++++++++++
> src/lib/csv/test_csv.c | 23 +++++++++++++++++++++++
> src/lib/http_parser/CMakeLists.txt | 11 +++++++++++
> src/lib/http_parser/test_http_parser.c | 22 ++++++++++++++++++++++
> src/lib/uri/CMakeLists.txt | 12 ++++++++++++
> src/lib/uri/test_uri.c | 22 ++++++++++++++++++++++
> 8 files changed, 107 insertions(+), 1 deletion(-)
> create mode 100644 src/lib/csv/test_csv.c
> create mode 100644 src/lib/http_parser/test_http_parser.c
> create mode 100644 src/lib/uri/test_uri.c
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 1d80b6806..99a502e3a 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -571,7 +571,7 @@ set(PREFIX ${CMAKE_INSTALL_PREFIX})
> set(options PACKAGE VERSION BUILD C_COMPILER CXX_COMPILER C_FLAGS CXX_FLAGS
> PREFIX
> ENABLE_SSE2 ENABLE_AVX
> - ENABLE_GCOV ENABLE_GPROF ENABLE_VALGRIND ENABLE_ASAN
> + ENABLE_GCOV ENABLE_GPROF ENABLE_VALGRIND ENABLE_ASAN ENABLE_FUZZER
> ENABLE_BACKTRACE
> ENABLE_DOC
> ENABLE_DIST
> diff --git a/cmake/profile.cmake b/cmake/profile.cmake
> index bc4bf67f5..b9fcd7655 100644
> --- a/cmake/profile.cmake
> +++ b/cmake/profile.cmake
> @@ -42,6 +42,11 @@ else()
> add_definitions(-DNVALGRIND=1)
> endif()
>
> +option(ENABLE_FUZZER "Enable fuzzing testing" OFF)
> +if (ENABLE_FUZZER)
> + set(TESTING_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/Testing")
> +endif ()
Maybe report some error on an attemt to set ENABLE_FUZZER=ON
when the compiler isn't clang? Then there’ll be no need to check
for CMAKE_CXX_COMPILER_ID in each test’s file.
> +
> option(ENABLE_ASAN "Enable AddressSanitizer, a fast memory error detector based on compiler instrumentation" OFF)
> if (ENABLE_ASAN)
> if (CMAKE_COMPILER_IS_GNUCC)
> diff --git a/src/lib/csv/CMakeLists.txt b/src/lib/csv/CMakeLists.txt
> index 3580e4da2..d5a3ed1f6 100644
> --- a/src/lib/csv/CMakeLists.txt
> +++ b/src/lib/csv/CMakeLists.txt
> @@ -4,3 +4,14 @@ set(lib_sources
>
> set_source_files_compile_flags(${lib_sources})
> add_library(csv STATIC ${lib_sources})
> +
> +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
Is there such a thing as CMAKE_C_COMPILER_ID ?
If yes, then maybe check it instead of _CXX_ one ?
> + set(TestName "test_csv")
> + add_executable(${TestName} ${TestName}.c)
> + set_target_properties(${TestName}
> + PROPERTIES
> + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> + LINK_FLAGS "-fsanitize=fuzzer,address")
> + target_link_libraries(${TestName} PRIVATE csv)
> + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
> +endif ()
> diff --git a/src/lib/csv/test_csv.c b/src/lib/csv/test_csv.c
> new file mode 100644
> index 000000000..aa1703514
> --- /dev/null
> +++ b/src/lib/csv/test_csv.c
Maybe we should put all the tests to test/fuzzing/smth ?
Since we have test/unit/smth.
> @@ -0,0 +1,23 @@
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include "csv.h"
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> + struct csv csv;
> + csv_create(&csv);
> + char *buf = calloc(size, sizeof(char*));
> + if (buf == NULL) {
> + return -1;
> + }
> + memcpy(buf, data, size);
> + buf[size] = '\0';
> + char *end = buf + size;
> + csv_parse_chunk(&csv, buf, end);
> + csv_finish_parsing(&csv);
> + csv_destroy(&csv);
> + free(buf);
> +
> + return 0;
> +}
> diff --git a/src/lib/http_parser/CMakeLists.txt b/src/lib/http_parser/CMakeLists.txt
> index a48f83cb6..7af922fbb 100644
> --- a/src/lib/http_parser/CMakeLists.txt
> +++ b/src/lib/http_parser/CMakeLists.txt
> @@ -1 +1,12 @@
> add_library(http_parser STATIC http_parser.c)
> +
> +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> + set(TestName "test_http_parser")
> + add_executable(${TestName} ${TestName}.c)
> + set_target_properties(${TestName}
> + PROPERTIES
> + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> + LINK_FLAGS "-fsanitize=fuzzer,address")
> + target_link_libraries(${TestName} PRIVATE http_parser)
> + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
> +endif ()
> diff --git a/src/lib/http_parser/test_http_parser.c b/src/lib/http_parser/test_http_parser.c
> new file mode 100644
> index 000000000..a189e71e9
> --- /dev/null
> +++ b/src/lib/http_parser/test_http_parser.c
> @@ -0,0 +1,22 @@
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stddef.h>
> +#include "http_parser.h"
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> + struct http_parser parser;
> + char *buf = (char*)data;
> + http_parser_create(&parser);
> + parser.hdr_name = (char *)calloc((int)size, sizeof(char));
> + if (parser.hdr_name == NULL) {
> + return -1;
> + }
> + char *end_buf = buf + size;
> + int rc = http_parse_header_line(&parser, &buf, end_buf, size);
> + free(parser.hdr_name);
> + if (rc != 0) {
> + return rc;
> + }
> +
> + return 0;
Maybe plain
return rc;
will do?
> +}
> diff --git a/src/lib/uri/CMakeLists.txt b/src/lib/uri/CMakeLists.txt
> index 96410e5bf..77f5c5d57 100644
> --- a/src/lib/uri/CMakeLists.txt
> +++ b/src/lib/uri/CMakeLists.txt
> @@ -8,3 +8,15 @@ if (CC_HAS_WNO_IMPLICIT_FALLTHROUGH)
> -Wno-implicit-fallthrough)
> endif()
> add_library(uri STATIC uri.c)
> +
> +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> + set(TestName "test_uri")
> + add_executable(${TestName} ${TestName}.c)
> + add_compile_options(-fsanitize=fuzzer,address -g -O1)
> + set_target_properties(${TestName}
> + PROPERTIES
> + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> + LINK_FLAGS "-fsanitize=fuzzer,address")
> + target_link_libraries(${TestName} PRIVATE uri)
> + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
> +endif ()
> diff --git a/src/lib/uri/test_uri.c b/src/lib/uri/test_uri.c
> new file mode 100644
> index 000000000..ad8db6ef2
> --- /dev/null
> +++ b/src/lib/uri/test_uri.c
> @@ -0,0 +1,22 @@
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include "uri.h"
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> + char *buf = calloc(size, sizeof(char*));
> + if (!buf) {
> + return -1;
> + }
> + strncpy(buf, (char*)data, size);
> + buf[size] = '\0';
> + struct uri uri;
> + int rc = uri_parse(&uri, buf);
> + free(buf);
> + if (rc != 0) {
> + return rc;
> + }
> +
> + return 0;
> +}
Same here.
> --
> 2.23.0
>
>
> --
> sergeyb@
--
Serge Petrenko
sergepetrenko@tarantool.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
2020-04-20 8:47 [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers Sergey Bronnikov
2020-04-24 13:56 ` Serge Petrenko
@ 2020-04-28 11:19 ` Igor Munkin
2020-04-29 12:09 ` Sergey Bronnikov
1 sibling, 1 reply; 10+ messages in thread
From: Igor Munkin @ 2020-04-28 11:19 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: o.piskunov, tarantool-patches
Sergey,
Thanks for the patch! I left several comments, please consider them.
On 20.04.20, Sergey Bronnikov wrote:
> - introduce CMake flag to enable building fuzzers
> - add fuzzers based on LibFuzzer[1] to csv, http_parser and uri modules
>
> [1] https://llvm.org/docs/LibFuzzer.html
>
> Note: LibFuzzer requires Clang compiler
>
> How-To Use:
>
> $ mkdir build && cd build
> $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
> $ make -j
> $ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4
I can't test your patch manually since this recipe doesn't work for me:
| $ cat /etc/os-release
| NAME=Gentoo
| ID=gentoo
| PRETTY_NAME="Gentoo/Linux"
| ANSI_COLOR="1;32"
| HOME_URL="https://www.gentoo.org/"
| SUPPORT_URL="https://www.gentoo.org/support/"
| BUG_REPORT_URL="https://bugs.gentoo.org/"
| $ clang --version
| clang version 8.0.1 (tags/RELEASE_801/final)
| Target: x86_64-pc-linux-gnu
| Thread model: posix
| InstalledDir: /usr/lib/llvm/8/bin
| $ clang++ --version
| clang version 8.0.1 (tags/RELEASE_801/final)
| Target: x86_64-pc-linux-gnu
| Thread model: posix
| InstalledDir: /usr/lib/llvm/8/bin
| $ git status | head -1
| On branch ligurio/gh-1809-libfuzzer
| $ mkdir -p libfuzzer
| $ cd !$
| cd libfuzzer
| $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
| -- Enabling LTO: FALSE
| -- OpenSSL 1.1.1d found
| --
| CMake Deprecation Warning at test/CMakeLists.txt:21 (cmake_policy):
| The OLD behavior for policy CMP0037 will be removed from a future version
| of CMake.
|
| The cmake-policies(7) manual explains that the OLD behaviors of all
| policies are deprecated and that a policy should be set to OLD only under
| specific short-term circumstances. Projects should be ported to the NEW
| behavior and not rely on setting a policy to OLD.
|
|
| --
| --
| -- Configuring done
| -- Generating done
| -- Build files have been written to: /home/imun/projects/tarantool
| $ echo $?
| 0
| $ make -j
| make: *** No targets specified and no makefile found. Stop.
> ...
>
> Follows: #1809
Please, don't use ':' in gh tags. It just doesn't respect our commit
message style.
> ---
> CMakeLists.txt | 2 +-
> cmake/profile.cmake | 5 +++++
> src/lib/csv/CMakeLists.txt | 11 +++++++++++
> src/lib/csv/test_csv.c | 23 +++++++++++++++++++++++
> src/lib/http_parser/CMakeLists.txt | 11 +++++++++++
> src/lib/http_parser/test_http_parser.c | 22 ++++++++++++++++++++++
> src/lib/uri/CMakeLists.txt | 12 ++++++++++++
> src/lib/uri/test_uri.c | 22 ++++++++++++++++++++++
> 8 files changed, 107 insertions(+), 1 deletion(-)
> create mode 100644 src/lib/csv/test_csv.c
> create mode 100644 src/lib/http_parser/test_http_parser.c
> create mode 100644 src/lib/uri/test_uri.c
>
<snipped>
> diff --git a/cmake/profile.cmake b/cmake/profile.cmake
> index bc4bf67f5..b9fcd7655 100644
> --- a/cmake/profile.cmake
> +++ b/cmake/profile.cmake
> @@ -42,6 +42,11 @@ else()
> add_definitions(-DNVALGRIND=1)
> endif()
>
> +option(ENABLE_FUZZER "Enable fuzzing testing" OFF)
> +if (ENABLE_FUZZER)
> + set(TESTING_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/Testing")
> +endif ()
There is no space before parentheses in other code, please adjust yours.
> +
> option(ENABLE_ASAN "Enable AddressSanitizer, a fast memory error detector based on compiler instrumentation" OFF)
> if (ENABLE_ASAN)
> if (CMAKE_COMPILER_IS_GNUCC)
> diff --git a/src/lib/csv/CMakeLists.txt b/src/lib/csv/CMakeLists.txt
> index 3580e4da2..d5a3ed1f6 100644
> --- a/src/lib/csv/CMakeLists.txt
> +++ b/src/lib/csv/CMakeLists.txt
> @@ -4,3 +4,14 @@ set(lib_sources
>
> set_source_files_compile_flags(${lib_sources})
> add_library(csv STATIC ${lib_sources})
> +
> +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> + set(TestName "test_csv")
> + add_executable(${TestName} ${TestName}.c)
> + set_target_properties(${TestName}
> + PROPERTIES
> + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> + LINK_FLAGS "-fsanitize=fuzzer,address")
> + target_link_libraries(${TestName} PRIVATE csv)
> + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
What if ${TESTING_OUTPUT_DIRECTORY} is not defined?
> +endif ()
There is no space before parentheses in other code, please adjust yours.
> diff --git a/src/lib/csv/test_csv.c b/src/lib/csv/test_csv.c
> new file mode 100644
> index 000000000..aa1703514
> --- /dev/null
> +++ b/src/lib/csv/test_csv.c
This C file doesn't respect our C style guide[1], please adjust it.
> @@ -0,0 +1,23 @@
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include "csv.h"
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> + struct csv csv;
> + csv_create(&csv);
> + char *buf = calloc(size, sizeof(char*));
> + if (buf == NULL) {
> + return -1;
> + }
> + memcpy(buf, data, size);
> + buf[size] = '\0';
> + char *end = buf + size;
> + csv_parse_chunk(&csv, buf, end);
> + csv_finish_parsing(&csv);
> + csv_destroy(&csv);
> + free(buf);
> +
> + return 0;
So, csv parsing succeeds regardless the input? Then what is the purpose
of the testing?
> +}
> diff --git a/src/lib/http_parser/CMakeLists.txt b/src/lib/http_parser/CMakeLists.txt
> index a48f83cb6..7af922fbb 100644
> --- a/src/lib/http_parser/CMakeLists.txt
> +++ b/src/lib/http_parser/CMakeLists.txt
> @@ -1 +1,12 @@
> add_library(http_parser STATIC http_parser.c)
> +
> +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> + set(TestName "test_http_parser")
> + add_executable(${TestName} ${TestName}.c)
> + set_target_properties(${TestName}
> + PROPERTIES
> + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> + LINK_FLAGS "-fsanitize=fuzzer,address")
> + target_link_libraries(${TestName} PRIVATE http_parser)
> + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
What if ${TESTING_OUTPUT_DIRECTORY} is not defined?
> +endif ()
There is no space before parentheses in other code, please adjust yours.
> diff --git a/src/lib/http_parser/test_http_parser.c b/src/lib/http_parser/test_http_parser.c
> new file mode 100644
> index 000000000..a189e71e9
> --- /dev/null
> +++ b/src/lib/http_parser/test_http_parser.c
This C file doesn't respect our C style guide[1], please adjust it.
> @@ -0,0 +1,22 @@
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stddef.h>
> +#include "http_parser.h"
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> + struct http_parser parser;
> + char *buf = (char*)data;
> + http_parser_create(&parser);
> + parser.hdr_name = (char *)calloc((int)size, sizeof(char));
> + if (parser.hdr_name == NULL) {
> + return -1;
> + }
> + char *end_buf = buf + size;
> + int rc = http_parse_header_line(&parser, &buf, end_buf, size);
> + free(parser.hdr_name);
> + if (rc != 0) {
> + return rc;
> + }
As Serge already mentioned, this branch is excess.
> +
> + return 0;
> +}
> diff --git a/src/lib/uri/CMakeLists.txt b/src/lib/uri/CMakeLists.txt
> index 96410e5bf..77f5c5d57 100644
> --- a/src/lib/uri/CMakeLists.txt
> +++ b/src/lib/uri/CMakeLists.txt
> @@ -8,3 +8,15 @@ if (CC_HAS_WNO_IMPLICIT_FALLTHROUGH)
> -Wno-implicit-fallthrough)
> endif()
> add_library(uri STATIC uri.c)
> +
> +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> + set(TestName "test_uri")
> + add_executable(${TestName} ${TestName}.c)
> + add_compile_options(-fsanitize=fuzzer,address -g -O1)
> + set_target_properties(${TestName}
> + PROPERTIES
> + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> + LINK_FLAGS "-fsanitize=fuzzer,address")
> + target_link_libraries(${TestName} PRIVATE uri)
> + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
What if ${TESTING_OUTPUT_DIRECTORY} is not defined?
> +endif ()
There is no space before parentheses in other code, please adjust yours.
> diff --git a/src/lib/uri/test_uri.c b/src/lib/uri/test_uri.c
> new file mode 100644
> index 000000000..ad8db6ef2
> --- /dev/null
> +++ b/src/lib/uri/test_uri.c
This C file doesn't respect our C style guide[1], please adjust it.
> @@ -0,0 +1,22 @@
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include "uri.h"
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> + char *buf = calloc(size, sizeof(char*));
> + if (!buf) {
> + return -1;
> + }
> + strncpy(buf, (char*)data, size);
> + buf[size] = '\0';
> + struct uri uri;
> + int rc = uri_parse(&uri, buf);
> + free(buf);
> + if (rc != 0) {
> + return rc;
> + }
As Serge already mentioned, this branch is excess.
> +
> + return 0;
> +}
> --
> 2.23.0
>
>
> --
> sergeyb@
[1]: https://www.tarantool.io/en/doc/2.2/dev_guide/c_style_guide/
--
Best regards,
IM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
2020-04-24 13:56 ` Serge Petrenko
@ 2020-04-29 9:28 ` Sergey Bronnikov
2020-04-30 11:10 ` Serge Petrenko
0 siblings, 1 reply; 10+ messages in thread
From: Sergey Bronnikov @ 2020-04-29 9:28 UTC (permalink / raw)
To: Serge Petrenko; +Cc: o.piskunov, tml
Hi, Serge
thanks for review. See my answers inline.
On 16:56 Fri 24 Apr , Serge Petrenko wrote:
<snipped>
> > +option(ENABLE_FUZZER "Enable fuzzing testing" OFF)
> > +if (ENABLE_FUZZER)
> > + set(TESTING_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/Testing")
> > +endif ()
>
> Maybe report some error on an attemt to set ENABLE_FUZZER=ON
> when the compiler isn't clang? Then there’ll be no need to check
> for CMAKE_CXX_COMPILER_ID in each test’s file.
Agree, added check for compiler here.
> > +
> > option(ENABLE_ASAN "Enable AddressSanitizer, a fast memory error detector based on compiler instrumentation" OFF)
> > if (ENABLE_ASAN)
> > if (CMAKE_COMPILER_IS_GNUCC)
> > diff --git a/src/lib/csv/CMakeLists.txt b/src/lib/csv/CMakeLists.txt
> > index 3580e4da2..d5a3ed1f6 100644
> > --- a/src/lib/csv/CMakeLists.txt
> > +++ b/src/lib/csv/CMakeLists.txt
> > @@ -4,3 +4,14 @@ set(lib_sources
> >
> > set_source_files_compile_flags(${lib_sources})
> > add_library(csv STATIC ${lib_sources})
> > +
> > +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
>
> Is there such a thing as CMAKE_C_COMPILER_ID ?
> If yes, then maybe check it instead of _CXX_ one ?
Agree, replaced.
> > + set(TestName "test_csv")
> > + add_executable(${TestName} ${TestName}.c)
> > + set_target_properties(${TestName}
> > + PROPERTIES
> > + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> > + LINK_FLAGS "-fsanitize=fuzzer,address")
> > + target_link_libraries(${TestName} PRIVATE csv)
> > + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
> > +endif ()
> > diff --git a/src/lib/csv/test_csv.c b/src/lib/csv/test_csv.c
> > new file mode 100644
> > index 000000000..aa1703514
> > --- /dev/null
> > +++ b/src/lib/csv/test_csv.c
>
> Maybe we should put all the tests to test/fuzzing/smth ?
> Since we have test/unit/smth.
I did it for convenience - it is more convenient when tests lie together
with code. I don't know why we place tarantool unit tests and
other tests in a single directory seperately with modules that they
tests. These three tests (csv, uri and http) are related to small libraries and
I don't see any reasons to place them outside of library dir.
<snipped>
--
sergeyb@
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
2020-04-28 11:19 ` Igor Munkin
@ 2020-04-29 12:09 ` Sergey Bronnikov
2020-05-26 15:54 ` Igor Munkin
0 siblings, 1 reply; 10+ messages in thread
From: Sergey Bronnikov @ 2020-04-29 12:09 UTC (permalink / raw)
To: Igor Munkin; +Cc: o.piskunov, tarantool-patches
Hi, Igor
thanks a lot for review! See my answers inline.
On 14:19 Tue 28 Apr , Igor Munkin wrote:
> Sergey,
>
> Thanks for the patch! I left several comments, please consider them.
>
> On 20.04.20, Sergey Bronnikov wrote:
> > - introduce CMake flag to enable building fuzzers
> > - add fuzzers based on LibFuzzer[1] to csv, http_parser and uri modules
> >
> > [1] https://llvm.org/docs/LibFuzzer.html
> >
> > Note: LibFuzzer requires Clang compiler
> >
> > How-To Use:
> >
> > $ mkdir build && cd build
> > $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
> > $ make -j
> > $ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4
>
> I can't test your patch manually since this recipe doesn't work for me:
perhaps you should install package sys-libs/compiler-rt-sanitizers [1]
1. https://packages.gentoo.org/packages/sys-libs/compiler-rt-sanitizers
> | $ cat /etc/os-release
> | NAME=Gentoo
> | ID=gentoo
> | PRETTY_NAME="Gentoo/Linux"
> | ANSI_COLOR="1;32"
> | HOME_URL="https://www.gentoo.org/"
> | SUPPORT_URL="https://www.gentoo.org/support/"
> | BUG_REPORT_URL="https://bugs.gentoo.org/"
> | $ clang --version
> | clang version 8.0.1 (tags/RELEASE_801/final)
> | Target: x86_64-pc-linux-gnu
> | Thread model: posix
> | InstalledDir: /usr/lib/llvm/8/bin
> | $ clang++ --version
> | clang version 8.0.1 (tags/RELEASE_801/final)
> | Target: x86_64-pc-linux-gnu
> | Thread model: posix
> | InstalledDir: /usr/lib/llvm/8/bin
> | $ git status | head -1
> | On branch ligurio/gh-1809-libfuzzer
> | $ mkdir -p libfuzzer
> | $ cd !$
> | cd libfuzzer
> | $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
> | -- Enabling LTO: FALSE
> | -- OpenSSL 1.1.1d found
> | --
> | CMake Deprecation Warning at test/CMakeLists.txt:21 (cmake_policy):
> | The OLD behavior for policy CMP0037 will be removed from a future version
> | of CMake.
> |
> | The cmake-policies(7) manual explains that the OLD behaviors of all
> | policies are deprecated and that a policy should be set to OLD only under
> | specific short-term circumstances. Projects should be ported to the NEW
> | behavior and not rely on setting a policy to OLD.
> |
> |
> | --
> | --
> | -- Configuring done
> | -- Generating done
> | -- Build files have been written to: /home/imun/projects/tarantool
> | $ echo $?
> | 0
> | $ make -j
> | make: *** No targets specified and no makefile found. Stop.
I suspect the problem was due to specifying "CXX=clang++".
I have changed check for compiler and it can probably fixes your problem.
> > ...
> >
> > Follows: #1809
>
> Please, don't use ':' in gh tags. It just doesn't respect our commit
> message style.
Fixed.
<snipped>
> > +if (ENABLE_FUZZER)
> > + set(TESTING_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/Testing")
> > +endif ()
>
> There is no space before parentheses in other code, please adjust yours.
Fixed.
<snipped>
> > +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> > + set(TestName "test_csv")
> > + add_executable(${TestName} ${TestName}.c)
> > + set_target_properties(${TestName}
> > + PROPERTIES
> > + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> > + LINK_FLAGS "-fsanitize=fuzzer,address")
> > + target_link_libraries(${TestName} PRIVATE csv)
> > + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
>
> What if ${TESTING_OUTPUT_DIRECTORY} is not defined?
Why it can be undefined if I set it in cmake/profile.cmake when ENABLE_FUZZER is enabled?
> > +endif ()
>
> There is no space before parentheses in other code, please adjust yours.
Fixed.
> > diff --git a/src/lib/csv/test_csv.c b/src/lib/csv/test_csv.c
> > new file mode 100644
> > index 000000000..aa1703514
> > --- /dev/null
> > +++ b/src/lib/csv/test_csv.c
>
> This C file doesn't respect our C style guide[1], please adjust it.
>
> > @@ -0,0 +1,23 @@
> > +#include <stdint.h>
> > +#include <stdlib.h>
> > +#include <stddef.h>
> > +#include <string.h>
> > +#include "csv.h"
> > +
> > +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> > + struct csv csv;
> > + csv_create(&csv);
> > + char *buf = calloc(size, sizeof(char*));
> > + if (buf == NULL) {
> > + return -1;
> > + }
> > + memcpy(buf, data, size);
> > + buf[size] = '\0';
> > + char *end = buf + size;
> > + csv_parse_chunk(&csv, buf, end);
> > + csv_finish_parsing(&csv);
> > + csv_destroy(&csv);
> > + free(buf);
> > +
> > + return 0;
>
> So, csv parsing succeeds regardless the input? Then what is the purpose
> of the testing?
Yes, function LLVMFuzzerTestOneInput succeeds when csv parsing
finishes without any issues like memory corruption, buffer overflow etc.
Unfortunately these csv_* functions returns void and I cannot additionaly
check was parsing successfull or not. Anyway fuzzing is about negative testing
and our goal to check function for reliability with junk input.
> > +}
> > diff --git a/src/lib/http_parser/CMakeLists.txt b/src/lib/http_parser/CMakeLists.txt
> > index a48f83cb6..7af922fbb 100644
> > --- a/src/lib/http_parser/CMakeLists.txt
> > +++ b/src/lib/http_parser/CMakeLists.txt
> > @@ -1 +1,12 @@
> > add_library(http_parser STATIC http_parser.c)
> > +
> > +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> > + set(TestName "test_http_parser")
> > + add_executable(${TestName} ${TestName}.c)
> > + set_target_properties(${TestName}
> > + PROPERTIES
> > + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> > + LINK_FLAGS "-fsanitize=fuzzer,address")
> > + target_link_libraries(${TestName} PRIVATE http_parser)
> > + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
>
> What if ${TESTING_OUTPUT_DIRECTORY} is not defined?
Why it can be undefined if I set it in cmake/profile.cmake when ENABLE_FUZZER is enabled?
> > +endif ()
>
> There is no space before parentheses in other code, please adjust yours.
Fixed.
> > diff --git a/src/lib/http_parser/test_http_parser.c b/src/lib/http_parser/test_http_parser.c
> > new file mode 100644
> > index 000000000..a189e71e9
> > --- /dev/null
> > +++ b/src/lib/http_parser/test_http_parser.c
>
> This C file doesn't respect our C style guide[1], please adjust it.
Fixed.
> > @@ -0,0 +1,22 @@
> > +#include <stdlib.h>
> > +#include <stdint.h>
> > +#include <stddef.h>
> > +#include "http_parser.h"
> > +
> > +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> > + struct http_parser parser;
> > + char *buf = (char*)data;
> > + http_parser_create(&parser);
> > + parser.hdr_name = (char *)calloc((int)size, sizeof(char));
> > + if (parser.hdr_name == NULL) {
> > + return -1;
> > + }
> > + char *end_buf = buf + size;
> > + int rc = http_parse_header_line(&parser, &buf, end_buf, size);
> > + free(parser.hdr_name);
> > + if (rc != 0) {
> > + return rc;
> > + }
>
> As Serge already mentioned, this branch is excess.
Agree, it's useless check.
> > +
> > + return 0;
> > +}
> > diff --git a/src/lib/uri/CMakeLists.txt b/src/lib/uri/CMakeLists.txt
> > index 96410e5bf..77f5c5d57 100644
> > --- a/src/lib/uri/CMakeLists.txt
> > +++ b/src/lib/uri/CMakeLists.txt
> > @@ -8,3 +8,15 @@ if (CC_HAS_WNO_IMPLICIT_FALLTHROUGH)
> > -Wno-implicit-fallthrough)
> > endif()
> > add_library(uri STATIC uri.c)
> > +
> > +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> > + set(TestName "test_uri")
> > + add_executable(${TestName} ${TestName}.c)
> > + add_compile_options(-fsanitize=fuzzer,address -g -O1)
> > + set_target_properties(${TestName}
> > + PROPERTIES
> > + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> > + LINK_FLAGS "-fsanitize=fuzzer,address")
> > + target_link_libraries(${TestName} PRIVATE uri)
> > + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
>
> What if ${TESTING_OUTPUT_DIRECTORY} is not defined?
Why it can be undefined if I set it in cmake/profile.cmake when ENABLE_FUZZER is enabled?
> > +endif ()
>
> There is no space before parentheses in other code, please adjust yours.
Fixed.
> > diff --git a/src/lib/uri/test_uri.c b/src/lib/uri/test_uri.c
> > new file mode 100644
> > index 000000000..ad8db6ef2
> > --- /dev/null
> > +++ b/src/lib/uri/test_uri.c
>
> This C file doesn't respect our C style guide[1], please adjust it.
Fixed.
> > @@ -0,0 +1,22 @@
> > +#include <stdlib.h>
> > +#include <stdint.h>
> > +#include <stddef.h>
> > +#include <string.h>
> > +#include "uri.h"
> > +
> > +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> > + char *buf = calloc(size, sizeof(char*));
> > + if (!buf) {
> > + return -1;
> > + }
> > + strncpy(buf, (char*)data, size);
> > + buf[size] = '\0';
> > + struct uri uri;
> > + int rc = uri_parse(&uri, buf);
> > + free(buf);
> > + if (rc != 0) {
> > + return rc;
> > + }
>
> As Serge already mentioned, this branch is excess.
Fixed.
<snipped>
--
sergeyb@
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
2020-04-29 9:28 ` Sergey Bronnikov
@ 2020-04-30 11:10 ` Serge Petrenko
2020-04-30 11:40 ` Sergey Bronnikov
0 siblings, 1 reply; 10+ messages in thread
From: Serge Petrenko @ 2020-04-30 11:10 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: o.piskunov, tml
29.04.2020 12:28, Sergey Bronnikov wrote:
> Hi, Serge
>
> thanks for review. See my answers inline.
>
> On 16:56 Fri 24 Apr , Serge Petrenko wrote:
>
> <snipped>
>
Hi! Thanks for the answers!
>>> +option(ENABLE_FUZZER "Enable fuzzing testing" OFF)
>>> +if (ENABLE_FUZZER)
>>> + set(TESTING_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/Testing")
>>> +endif ()
>> Maybe report some error on an attemt to set ENABLE_FUZZER=ON
>> when the compiler isn't clang? Then there’ll be no need to check
>> for CMAKE_CXX_COMPILER_ID in each test’s file.
> Agree, added check for compiler here.
Let me paste bits of the new patch below for commenting.
==================
diff --git a/cmake/profile.cmake b/cmake/profile.cmake
index bc4bf67f5..419f7b3cc 100644
--- a/cmake/profile.cmake
+++ b/cmake/profile.cmake
@@ -42,6 +42,15 @@ else()
add_definitions(-DNVALGRIND=1)
endif()
+option(ENABLE_FUZZER "Enable fuzzing testing" OFF)
+if(ENABLE_FUZZER)
+ if(NOT (CMAKE_C_COMPILER_ID STREQUAL "Clang"))
+ message(WARNING "Fuzzing supported with Clang compiler only.")
+ else()
+ set(TESTING_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/Testing")
+ endif()
+endif()
+
option(ENABLE_ASAN "Enable AddressSanitizer, a fast memory error
detector based on compiler instrumentation" OFF)
if (ENABLE_ASAN)
if (CMAKE_COMPILER_IS_GNUCC)
===============
shouldn't it be message(FATAL_ERROR "...") ? So that cmake config fails.
Otherwise it'll produce the warning, but proceed to build anyway.
>
>>> +
>>> option(ENABLE_ASAN "Enable AddressSanitizer, a fast memory error detector based on compiler instrumentation" OFF)
>>> if (ENABLE_ASAN)
>>> if (CMAKE_COMPILER_IS_GNUCC)
>>> diff --git a/src/lib/csv/CMakeLists.txt b/src/lib/csv/CMakeLists.txt
>>> index 3580e4da2..d5a3ed1f6 100644
>>> --- a/src/lib/csv/CMakeLists.txt
>>> +++ b/src/lib/csv/CMakeLists.txt
>>> @@ -4,3 +4,14 @@ set(lib_sources
>>>
>>> set_source_files_compile_flags(${lib_sources})
>>> add_library(csv STATIC ${lib_sources})
>>> +
>>> +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
>> Is there such a thing as CMAKE_C_COMPILER_ID ?
>> If yes, then maybe check it instead of _CXX_ one ?
> Agree, replaced.
==============
diff --git a/src/lib/csv/CMakeLists.txt b/src/lib/csv/CMakeLists.txt
index 3580e4da2..9be0a9e73 100644
--- a/src/lib/csv/CMakeLists.txt
+++ b/src/lib/csv/CMakeLists.txt
@@ -4,3 +4,14 @@ set(lib_sources
set_source_files_compile_flags(${lib_sources})
add_library(csv STATIC ${lib_sources})
+
+if(ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+ set(TestName "test_csv")
=================
If you error on compiler check, as mentioned above, there's no need to
check
for compiler here. It's the only place left, besides. You've omitted
the checks in
http_parser and uri tests.
>
>>> + set(TestName "test_csv")
>>> + add_executable(${TestName} ${TestName}.c)
>>> + set_target_properties(${TestName}
>>> + PROPERTIES
>>> + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
>>> + LINK_FLAGS "-fsanitize=fuzzer,address")
>>> + target_link_libraries(${TestName} PRIVATE csv)
>>> + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
>>> +endif ()
>>> diff --git a/src/lib/csv/test_csv.c b/src/lib/csv/test_csv.c
>>> new file mode 100644
>>> index 000000000..aa1703514
>>> --- /dev/null
>>> +++ b/src/lib/csv/test_csv.c
>> Maybe we should put all the tests to test/fuzzing/smth ?
>> Since we have test/unit/smth.
> I did it for convenience - it is more convenient when tests lie together
> with code. I don't know why we place tarantool unit tests and
> other tests in a single directory seperately with modules that they
> tests. These three tests (csv, uri and http) are related to small libraries and
> I don't see any reasons to place them outside of library dir.
Ok, it's up to you then.
> <snipped>
--
Serge Petrenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
2020-04-30 11:10 ` Serge Petrenko
@ 2020-04-30 11:40 ` Sergey Bronnikov
2020-04-30 13:01 ` Serge Petrenko
0 siblings, 1 reply; 10+ messages in thread
From: Sergey Bronnikov @ 2020-04-30 11:40 UTC (permalink / raw)
To: Serge Petrenko; +Cc: o.piskunov, tml
Hi, Serge
On 14:10 Thu 30 Apr , Serge Petrenko wrote:
> diff --git a/cmake/profile.cmake b/cmake/profile.cmake
> index bc4bf67f5..419f7b3cc 100644
> --- a/cmake/profile.cmake
> +++ b/cmake/profile.cmake
> @@ -42,6 +42,15 @@ else()
> add_definitions(-DNVALGRIND=1)
> endif()
>
> +option(ENABLE_FUZZER "Enable fuzzing testing" OFF)
> +if(ENABLE_FUZZER)
> + if(NOT (CMAKE_C_COMPILER_ID STREQUAL "Clang"))
> + message(WARNING "Fuzzing supported with Clang compiler only.")
> + else()
> + set(TESTING_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/Testing")
> + endif()
> +endif()
> +
> option(ENABLE_ASAN "Enable AddressSanitizer, a fast memory error detector
> based on compiler instrumentation" OFF)
> if (ENABLE_ASAN)
> if (CMAKE_COMPILER_IS_GNUCC)
>
> ===============
>
> shouldn't it be message(FATAL_ERROR "...") ? So that cmake config fails.
> Otherwise it'll produce the warning, but proceed to build anyway.
Agree, replaced it to FATAL_ERROR.
<snipped>
> +if(ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> + set(TestName "test_csv")
> =================
>
> If you error on compiler check, as mentioned above, there's no need to
> check for compiler here. It's the only place left, besides. You've omitted the
> checks in http_parser and uri tests.
Just forgot to remove it. Fixed in a branch.
<snipped>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
2020-04-30 11:40 ` Sergey Bronnikov
@ 2020-04-30 13:01 ` Serge Petrenko
0 siblings, 0 replies; 10+ messages in thread
From: Serge Petrenko @ 2020-04-30 13:01 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: o.piskunov, tml
30.04.2020 14:40, Sergey Bronnikov wrote:
> Hi, Serge
>
> On 14:10 Thu 30 Apr , Serge Petrenko wrote:
>> diff --git a/cmake/profile.cmake b/cmake/profile.cmake
>> index bc4bf67f5..419f7b3cc 100644
>> --- a/cmake/profile.cmake
>> +++ b/cmake/profile.cmake
>> @@ -42,6 +42,15 @@ else()
>> add_definitions(-DNVALGRIND=1)
>> endif()
>>
>> +option(ENABLE_FUZZER "Enable fuzzing testing" OFF)
>> +if(ENABLE_FUZZER)
>> + if(NOT (CMAKE_C_COMPILER_ID STREQUAL "Clang"))
>> + message(WARNING "Fuzzing supported with Clang compiler only.")
>> + else()
>> + set(TESTING_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/Testing")
>> + endif()
>> +endif()
>> +
>> option(ENABLE_ASAN "Enable AddressSanitizer, a fast memory error detector
>> based on compiler instrumentation" OFF)
>> if (ENABLE_ASAN)
>> if (CMAKE_COMPILER_IS_GNUCC)
>>
>> ===============
>>
>> shouldn't it be message(FATAL_ERROR "...") ? So that cmake config fails.
>> Otherwise it'll produce the warning, but proceed to build anyway.
> Agree, replaced it to FATAL_ERROR.
>
> <snipped>
>
>> +if(ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
>> + set(TestName "test_csv")
>> =================
>>
>> If you error on compiler check, as mentioned above, there's no need to
>> check for compiler here. It's the only place left, besides. You've omitted the
>> checks in http_parser and uri tests.
> Just forgot to remove it. Fixed in a branch.
Hi, Sergey. Thanks for the fixes! LGTM.
>
> <snipped>
--
--
Serge Petrenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
2020-04-29 12:09 ` Sergey Bronnikov
@ 2020-05-26 15:54 ` Igor Munkin
2020-12-01 16:22 ` Sergey Bronnikov
0 siblings, 1 reply; 10+ messages in thread
From: Igor Munkin @ 2020-05-26 15:54 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: o.piskunov, tarantool-patches, alexander.turenko
Sergey,
Thanks for the changes! Please consider my replies below.
On 29.04.20, Sergey Bronnikov wrote:
> Hi, Igor
>
> thanks a lot for review! See my answers inline.
>
> On 14:19 Tue 28 Apr , Igor Munkin wrote:
> > Sergey,
> >
> > Thanks for the patch! I left several comments, please consider them.
> >
> > On 20.04.20, Sergey Bronnikov wrote:
> > > - introduce CMake flag to enable building fuzzers
> > > - add fuzzers based on LibFuzzer[1] to csv, http_parser and uri modules
> > >
> > > [1] https://llvm.org/docs/LibFuzzer.html
> > >
> > > Note: LibFuzzer requires Clang compiler
> > >
> > > How-To Use:
> > >
> > > $ mkdir build && cd build
> > > $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
> > > $ make -j
> > > $ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4
I guess corpus directory need to be created prior to the <test_csv> run.
At least I faced the following error:
| $ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4
| INFO: Seed: 3782841313
| INFO: Loaded 1 modules (3 inline 8-bit counters): 3 [0x574eb0,
| 0x574eb3),
| INFO: Loaded 1 PC tables (3 PCs): 3 [0x54f540,0x54f570),
| No such file or directory: corpus/; exiting
After <mkdir> everything works fine.
> >
> > I can't test your patch manually since this recipe doesn't work for me:
>
> perhaps you should install package sys-libs/compiler-rt-sanitizers [1]
This package was installed.
| $ $ eix sys-libs/compiler-rt-sanitizers | head -1
| [I] sys-libs/compiler-rt-sanitizers
>
> 1. https://packages.gentoo.org/packages/sys-libs/compiler-rt-sanitizers
>
> > | $ cat /etc/os-release
> > | NAME=Gentoo
> > | ID=gentoo
> > | PRETTY_NAME="Gentoo/Linux"
> > | ANSI_COLOR="1;32"
> > | HOME_URL="https://www.gentoo.org/"
> > | SUPPORT_URL="https://www.gentoo.org/support/"
> > | BUG_REPORT_URL="https://bugs.gentoo.org/"
> > | $ clang --version
> > | clang version 8.0.1 (tags/RELEASE_801/final)
> > | Target: x86_64-pc-linux-gnu
> > | Thread model: posix
> > | InstalledDir: /usr/lib/llvm/8/bin
> > | $ clang++ --version
> > | clang version 8.0.1 (tags/RELEASE_801/final)
> > | Target: x86_64-pc-linux-gnu
> > | Thread model: posix
> > | InstalledDir: /usr/lib/llvm/8/bin
> > | $ git status | head -1
> > | On branch ligurio/gh-1809-libfuzzer
> > | $ mkdir -p libfuzzer
> > | $ cd !$
> > | cd libfuzzer
> > | $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
> > | -- Enabling LTO: FALSE
> > | -- OpenSSL 1.1.1d found
> > | --
> > | CMake Deprecation Warning at test/CMakeLists.txt:21 (cmake_policy):
> > | The OLD behavior for policy CMP0037 will be removed from a future version
> > | of CMake.
> > |
> > | The cmake-policies(7) manual explains that the OLD behaviors of all
> > | policies are deprecated and that a policy should be set to OLD only under
> > | specific short-term circumstances. Projects should be ported to the NEW
> > | behavior and not rely on setting a policy to OLD.
> > |
> > |
> > | --
> > | --
> > | -- Configuring done
> > | -- Generating done
> > | -- Build files have been written to: /home/imun/projects/tarantool
> > | $ echo $?
> > | 0
> > | $ make -j
> > | make: *** No targets specified and no makefile found. Stop.
>
> I suspect the problem was due to specifying "CXX=clang++".
> I have changed check for compiler and it can probably fixes your problem.
Yeah, this might be the root cause. As I mentioned above, now everything
is fine, thanks!
>
<snipped>
>
> > > diff --git a/src/lib/csv/test_csv.c b/src/lib/csv/test_csv.c
> > > new file mode 100644
> > > index 000000000..aa1703514
> > > --- /dev/null
> > > +++ b/src/lib/csv/test_csv.c
> >
> > This C file doesn't respect our C style guide[1], please adjust it.
> >
> > > @@ -0,0 +1,23 @@
> > > +#include <stdint.h>
> > > +#include <stdlib.h>
> > > +#include <stddef.h>
> > > +#include <string.h>
> > > +#include "csv.h"
> > > +
> > > +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> > > + struct csv csv;
> > > + csv_create(&csv);
> > > + char *buf = calloc(size, sizeof(char*));
> > > + if (buf == NULL) {
> > > + return -1;
> > > + }
> > > + memcpy(buf, data, size);
> > > + buf[size] = '\0';
> > > + char *end = buf + size;
> > > + csv_parse_chunk(&csv, buf, end);
> > > + csv_finish_parsing(&csv);
> > > + csv_destroy(&csv);
> > > + free(buf);
> > > +
> > > + return 0;
> >
> > So, csv parsing succeeds regardless the input? Then what is the purpose
> > of the testing?
>
> Yes, function LLVMFuzzerTestOneInput succeeds when csv parsing
> finishes without any issues like memory corruption, buffer overflow etc.
> Unfortunately these csv_* functions returns void and I cannot additionaly
> check was parsing successfull or not. Anyway fuzzing is about negative testing
> and our goal to check function for reliability with junk input.
No, you can. Despite the void function return type you can check the
error_status field in csv context structure which is set e.g. in
<csv_invalid> routine (called within <csv_finish_parsing> routine).
>
> > > +}
> > > diff --git a/src/lib/http_parser/CMakeLists.txt b/src/lib/http_parser/CMakeLists.txt
> > > index a48f83cb6..7af922fbb 100644
> > > --- a/src/lib/http_parser/CMakeLists.txt
> > > +++ b/src/lib/http_parser/CMakeLists.txt
> > > @@ -1 +1,12 @@
> > > add_library(http_parser STATIC http_parser.c)
> > > +
> > > +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
> > > + set(TestName "test_http_parser")
> > > + add_executable(${TestName} ${TestName}.c)
> > > + set_target_properties(${TestName}
> > > + PROPERTIES
> > > + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
> > > + LINK_FLAGS "-fsanitize=fuzzer,address")
> > > + target_link_libraries(${TestName} PRIVATE http_parser)
> > > + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
> >
> > What if ${TESTING_OUTPUT_DIRECTORY} is not defined?
>
> Why it can be undefined if I set it in cmake/profile.cmake when ENABLE_FUZZER is enabled?
Yes, but if one runs the following command, the build succeeds:
| $ cmake . && make -j
| -- The C compiler identification is GNU 8.3.0
| -- The CXX compiler identification is GNU 8.3.0
| -- Check for working C compiler: /usr/bin/cc
| -- Check for working C compiler: /usr/bin/cc -- works
| -- Detecting C compiler ABI info
| -- Detecting C compiler ABI info - done
| -- Detecting C compile features
| -- Detecting C compile features - done
| -- Check for working CXX compiler: /usr/bin/c++
| -- Check for working CXX compiler: /usr/bin/c++ -- works
| -- Detecting CXX compiler ABI info
| -- Detecting CXX compiler ABI info - done
| -- Detecting CXX compile features
| -- Detecting CXX compile features - done
| CMake Warning (dev) in CMakeLists.txt:
| No cmake_minimum_required command is present. A line of code such as
|
| cmake_minimum_required(VERSION 3.14)
|
| should be added at the top of the file. The version specified may be lower
| if you wish to support older CMake versions for this project. For more
| information run "cmake --help-policy CMP0000".
| This warning is for project developers. Use -Wno-dev to suppress it.
|
| -- Configuring done
| -- Generating done
| -- Build files have been written to: <tarantool-path>/src/lib/http_parser
| Scanning dependencies of target http_parser
| [ 50%] Building C object CMakeFiles/http_parser.dir/http_parser.o
| [100%] Linking C static library libhttp_parser.a
| [100%] Built target http_parser
At the same time the following command fails due to invalid
compiler/linker flags:
| $ cmake -DENABLE_FUZZER=ON . && make -j
| -- The C compiler identification is GNU 8.3.0
| -- The CXX compiler identification is GNU 8.3.0
| -- Check for working C compiler: /usr/bin/cc
| -- Check for working C compiler: /usr/bin/cc -- works
| -- Detecting C compiler ABI info
| -- Detecting C compiler ABI info - done
| -- Detecting C compile features
| -- Detecting C compile features - done
| -- Check for working CXX compiler: /usr/bin/c++
| -- Check for working CXX compiler: /usr/bin/c++ -- works
| -- Detecting CXX compiler ABI info
| -- Detecting CXX compiler ABI info - done
| -- Detecting CXX compile features
| -- Detecting CXX compile features - done
| CMake Warning (dev) in CMakeLists.txt:
| No cmake_minimum_required command is present. A line of code such as
|
| cmake_minimum_required(VERSION 3.14)
|
| should be added at the top of the file. The version specified may be lower
| if you wish to support older CMake versions for this project. For more
| information run "cmake --help-policy CMP0000".
| This warning is for project developers. Use -Wno-dev to suppress it.
|
| -- Configuring done
| -- Generating done
| -- Build files have been written to: <tarantool-path>/src/lib/http_parser
| Scanning dependencies of target http_parser
| [ 25%] Building C object CMakeFiles/http_parser.dir/http_parser.o
| [ 50%] Linking C static library libhttp_parser.a
| [ 50%] Built target http_parser
| Scanning dependencies of target test_http_parser
| [ 75%] Building C object CMakeFiles/test_http_parser.dir/test_http_parser.o
| cc: error: unrecognized argument to -fsanitize= option: 'fuzzer'
| make[2]: *** [CMakeFiles/test_http_parser.dir/build.make:63: CMakeFiles/test_http_parser.dir/test_http_parser.o] Error 1
| make[1]: *** [CMakeFiles/Makefile2:110: CMakeFiles/test_http_parser.dir/all] Error 2
| make: *** [Makefile:84: all] Error 2
So cmake respects the given options but has no check for toolchain. OK,
when the proper toolchain is set <make> command succeeds and
test_http_parser is build in the project root directory:
| $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON . && make -j
| -- The C compiler identification is Clang 8.0.1
| -- The CXX compiler identification is Clang 8.0.1
| -- Check for working C compiler: /usr/lib/llvm/8/bin/clang
| -- Check for working C compiler: /usr/lib/llvm/8/bin/clang -- works
| -- Detecting C compiler ABI info
| -- Detecting C compiler ABI info - done
| -- Detecting C compile features
| -- Detecting C compile features - done
| -- Check for working CXX compiler: /usr/lib/llvm/8/bin/clang++
| -- Check for working CXX compiler: /usr/lib/llvm/8/bin/clang++ -- works
| -- Detecting CXX compiler ABI info
| -- Detecting CXX compiler ABI info - done
| -- Detecting CXX compile features
| -- Detecting CXX compile features - done
| CMake Warning (dev) in CMakeLists.txt:
| No cmake_minimum_required command is present. A line of code such as
|
| cmake_minimum_required(VERSION 3.14)
|
| should be added at the top of the file. The version specified may be lower
| if you wish to support older CMake versions for this project. For more
| information run "cmake --help-policy CMP0000".
| This warning is for project developers. Use -Wno-dev to suppress it.
|
| -- Configuring done
| -- Generating done
| -- Build files have been written to: <tarantool-path>/src/lib/http_parser
| Scanning dependencies of target http_parser
| [ 25%] Building C object CMakeFiles/http_parser.dir/http_parser.o
| [ 50%] Linking C static library libhttp_parser.a
| [ 50%] Built target http_parser
| Scanning dependencies of target test_http_parser
| [ 75%] Building C object CMakeFiles/test_http_parser.dir/test_http_parser.o
| [100%] Linking C executable test_http_parser
| [100%] Built target test_http_parser
And this is the case I was writing about. Do you consider this as a bug?
Side note: this *might* be the main reason, why unit tests aren't placed
alongside with the corresponding libs but grouped within a separate
directory as Serge mentioned in his review.
>
<snipped>
>
> --
> sergeyb@
--
Best regards,
IM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
2020-05-26 15:54 ` Igor Munkin
@ 2020-12-01 16:22 ` Sergey Bronnikov
0 siblings, 0 replies; 10+ messages in thread
From: Sergey Bronnikov @ 2020-12-01 16:22 UTC (permalink / raw)
To: Igor Munkin; +Cc: o.piskunov, tarantool-patches, alexander.turenko
Hi, Igor!
Thanks for review!
I'm too late, but I finally updated a patch and send a new version [1].
Please see comments below.
1.
https://lists.tarantool.org/pipermail/tarantool-patches/2020-November/020959.html
On 26.05.2020 18:54, Igor Munkin wrote:
> Sergey,
>
> Thanks for the changes! Please consider my replies below.
>
> On 29.04.20, Sergey Bronnikov wrote:
>> Hi, Igor
>>
>> thanks a lot for review! See my answers inline.
>>
>> On 14:19 Tue 28 Apr , Igor Munkin wrote:
>>> Sergey,
>>>
>>> Thanks for the patch! I left several comments, please consider them.
>>>
>>> On 20.04.20, Sergey Bronnikov wrote:
>>>> - introduce CMake flag to enable building fuzzers
>>>> - add fuzzers based on LibFuzzer[1] to csv, http_parser and uri modules
>>>>
>>>> [1] https://llvm.org/docs/LibFuzzer.html
>>>>
>>>> Note: LibFuzzer requires Clang compiler
>>>>
>>>> How-To Use:
>>>>
>>>> $ mkdir build && cd build
>>>> $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
>>>> $ make -j
>>>> $ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4
> I guess corpus directory need to be created prior to the <test_csv> run.
> At least I faced the following error:
> | $ ./Testing/test_csv corpus/ -max_total_time=60*60*60 -workers=4
> | INFO: Seed: 3782841313
> | INFO: Loaded 1 modules (3 inline 8-bit counters): 3 [0x574eb0,
> | 0x574eb3),
> | INFO: Loaded 1 PC tables (3 PCs): 3 [0x54f540,0x54f570),
> | No such file or directory: corpus/; exiting
>
> After <mkdir> everything works fine.
>
>>> I can't test your patch manually since this recipe doesn't work for me:
>> perhaps you should install package sys-libs/compiler-rt-sanitizers [1]
> This package was installed.
>
> | $ $ eix sys-libs/compiler-rt-sanitizers | head -1
> | [I] sys-libs/compiler-rt-sanitizers
>
>> 1. https://packages.gentoo.org/packages/sys-libs/compiler-rt-sanitizers
>>
>>> | $ cat /etc/os-release
>>> | NAME=Gentoo
>>> | ID=gentoo
>>> | PRETTY_NAME="Gentoo/Linux"
>>> | ANSI_COLOR="1;32"
>>> | HOME_URL="https://www.gentoo.org/"
>>> | SUPPORT_URL="https://www.gentoo.org/support/"
>>> | BUG_REPORT_URL="https://bugs.gentoo.org/"
>>> | $ clang --version
>>> | clang version 8.0.1 (tags/RELEASE_801/final)
>>> | Target: x86_64-pc-linux-gnu
>>> | Thread model: posix
>>> | InstalledDir: /usr/lib/llvm/8/bin
>>> | $ clang++ --version
>>> | clang version 8.0.1 (tags/RELEASE_801/final)
>>> | Target: x86_64-pc-linux-gnu
>>> | Thread model: posix
>>> | InstalledDir: /usr/lib/llvm/8/bin
>>> | $ git status | head -1
>>> | On branch ligurio/gh-1809-libfuzzer
>>> | $ mkdir -p libfuzzer
>>> | $ cd !$
>>> | cd libfuzzer
>>> | $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON ..
>>> | -- Enabling LTO: FALSE
>>> | -- OpenSSL 1.1.1d found
>>> | --
>>> | CMake Deprecation Warning at test/CMakeLists.txt:21 (cmake_policy):
>>> | The OLD behavior for policy CMP0037 will be removed from a future version
>>> | of CMake.
>>> |
>>> | The cmake-policies(7) manual explains that the OLD behaviors of all
>>> | policies are deprecated and that a policy should be set to OLD only under
>>> | specific short-term circumstances. Projects should be ported to the NEW
>>> | behavior and not rely on setting a policy to OLD.
>>> |
>>> |
>>> | --
>>> | --
>>> | -- Configuring done
>>> | -- Generating done
>>> | -- Build files have been written to: /home/imun/projects/tarantool
>>> | $ echo $?
>>> | 0
>>> | $ make -j
>>> | make: *** No targets specified and no makefile found. Stop.
>> I suspect the problem was due to specifying "CXX=clang++".
>> I have changed check for compiler and it can probably fixes your problem.
> Yeah, this might be the root cause. As I mentioned above, now everything
> is fine, thanks!
>
> <snipped>
>
>>>> diff --git a/src/lib/csv/test_csv.c b/src/lib/csv/test_csv.c
>>>> new file mode 100644
>>>> index 000000000..aa1703514
>>>> --- /dev/null
>>>> +++ b/src/lib/csv/test_csv.c
>>> This C file doesn't respect our C style guide[1], please adjust it.
>>>
>>>> @@ -0,0 +1,23 @@
>>>> +#include <stdint.h>
>>>> +#include <stdlib.h>
>>>> +#include <stddef.h>
>>>> +#include <string.h>
>>>> +#include "csv.h"
>>>> +
>>>> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
>>>> + struct csv csv;
>>>> + csv_create(&csv);
>>>> + char *buf = calloc(size, sizeof(char*));
>>>> + if (buf == NULL) {
>>>> + return -1;
>>>> + }
>>>> + memcpy(buf, data, size);
>>>> + buf[size] = '\0';
>>>> + char *end = buf + size;
>>>> + csv_parse_chunk(&csv, buf, end);
>>>> + csv_finish_parsing(&csv);
>>>> + csv_destroy(&csv);
>>>> + free(buf);
>>>> +
>>>> + return 0;
>>> So, csv parsing succeeds regardless the input? Then what is the purpose
>>> of the testing?
>> Yes, function LLVMFuzzerTestOneInput succeeds when csv parsing
>> finishes without any issues like memory corruption, buffer overflow etc.
>> Unfortunately these csv_* functions returns void and I cannot additionaly
>> check was parsing successfull or not. Anyway fuzzing is about negative testing
>> and our goal to check function for reliability with junk input.
> No, you can. Despite the void function return type you can check the
> error_status field in csv context structure which is set e.g. in
> <csv_invalid> routine (called within <csv_finish_parsing> routine).
Updated test, now it takes return value from csv_get_error_status().
>>>> +}
>>>> diff --git a/src/lib/http_parser/CMakeLists.txt b/src/lib/http_parser/CMakeLists.txt
>>>> index a48f83cb6..7af922fbb 100644
>>>> --- a/src/lib/http_parser/CMakeLists.txt
>>>> +++ b/src/lib/http_parser/CMakeLists.txt
>>>> @@ -1 +1,12 @@
>>>> add_library(http_parser STATIC http_parser.c)
>>>> +
>>>> +if (ENABLE_FUZZER AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
>>>> + set(TestName "test_http_parser")
>>>> + add_executable(${TestName} ${TestName}.c)
>>>> + set_target_properties(${TestName}
>>>> + PROPERTIES
>>>> + COMPILE_FLAGS "-fsanitize=fuzzer,address -g -O1"
>>>> + LINK_FLAGS "-fsanitize=fuzzer,address")
>>>> + target_link_libraries(${TestName} PRIVATE http_parser)
>>>> + set_target_properties(${TestName} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
>>> What if ${TESTING_OUTPUT_DIRECTORY} is not defined?
>> Why it can be undefined if I set it in cmake/profile.cmake when ENABLE_FUZZER is enabled?
> Yes, but if one runs the following command, the build succeeds:
> | $ cmake . && make -j
> | -- The C compiler identification is GNU 8.3.0
> | -- The CXX compiler identification is GNU 8.3.0
> | -- Check for working C compiler: /usr/bin/cc
> | -- Check for working C compiler: /usr/bin/cc -- works
> | -- Detecting C compiler ABI info
> | -- Detecting C compiler ABI info - done
> | -- Detecting C compile features
> | -- Detecting C compile features - done
> | -- Check for working CXX compiler: /usr/bin/c++
> | -- Check for working CXX compiler: /usr/bin/c++ -- works
> | -- Detecting CXX compiler ABI info
> | -- Detecting CXX compiler ABI info - done
> | -- Detecting CXX compile features
> | -- Detecting CXX compile features - done
> | CMake Warning (dev) in CMakeLists.txt:
> | No cmake_minimum_required command is present. A line of code such as
> |
> | cmake_minimum_required(VERSION 3.14)
> |
> | should be added at the top of the file. The version specified may be lower
> | if you wish to support older CMake versions for this project. For more
> | information run "cmake --help-policy CMP0000".
> | This warning is for project developers. Use -Wno-dev to suppress it.
> |
> | -- Configuring done
> | -- Generating done
> | -- Build files have been written to: <tarantool-path>/src/lib/http_parser
> | Scanning dependencies of target http_parser
> | [ 50%] Building C object CMakeFiles/http_parser.dir/http_parser.o
> | [100%] Linking C static library libhttp_parser.a
> | [100%] Built target http_parser
>
> At the same time the following command fails due to invalid
> compiler/linker flags:
> | $ cmake -DENABLE_FUZZER=ON . && make -j
> | -- The C compiler identification is GNU 8.3.0
> | -- The CXX compiler identification is GNU 8.3.0
> | -- Check for working C compiler: /usr/bin/cc
> | -- Check for working C compiler: /usr/bin/cc -- works
> | -- Detecting C compiler ABI info
> | -- Detecting C compiler ABI info - done
> | -- Detecting C compile features
> | -- Detecting C compile features - done
> | -- Check for working CXX compiler: /usr/bin/c++
> | -- Check for working CXX compiler: /usr/bin/c++ -- works
> | -- Detecting CXX compiler ABI info
> | -- Detecting CXX compiler ABI info - done
> | -- Detecting CXX compile features
> | -- Detecting CXX compile features - done
> | CMake Warning (dev) in CMakeLists.txt:
> | No cmake_minimum_required command is present. A line of code such as
> |
> | cmake_minimum_required(VERSION 3.14)
> |
> | should be added at the top of the file. The version specified may be lower
> | if you wish to support older CMake versions for this project. For more
> | information run "cmake --help-policy CMP0000".
> | This warning is for project developers. Use -Wno-dev to suppress it.
> |
> | -- Configuring done
> | -- Generating done
> | -- Build files have been written to: <tarantool-path>/src/lib/http_parser
> | Scanning dependencies of target http_parser
> | [ 25%] Building C object CMakeFiles/http_parser.dir/http_parser.o
> | [ 50%] Linking C static library libhttp_parser.a
> | [ 50%] Built target http_parser
> | Scanning dependencies of target test_http_parser
> | [ 75%] Building C object CMakeFiles/test_http_parser.dir/test_http_parser.o
> | cc: error: unrecognized argument to -fsanitize= option: 'fuzzer'
> | make[2]: *** [CMakeFiles/test_http_parser.dir/build.make:63: CMakeFiles/test_http_parser.dir/test_http_parser.o] Error 1
> | make[1]: *** [CMakeFiles/Makefile2:110: CMakeFiles/test_http_parser.dir/all] Error 2
> | make: *** [Makefile:84: all] Error 2
>
> So cmake respects the given options but has no check for toolchain. OK,
> when the proper toolchain is set <make> command succeeds and
> test_http_parser is build in the project root directory:
> | $ CC=clang CXX=clang++ cmake -DENABLE_FUZZER=ON . && make -j
> | -- The C compiler identification is Clang 8.0.1
> | -- The CXX compiler identification is Clang 8.0.1
> | -- Check for working C compiler: /usr/lib/llvm/8/bin/clang
> | -- Check for working C compiler: /usr/lib/llvm/8/bin/clang -- works
> | -- Detecting C compiler ABI info
> | -- Detecting C compiler ABI info - done
> | -- Detecting C compile features
> | -- Detecting C compile features - done
> | -- Check for working CXX compiler: /usr/lib/llvm/8/bin/clang++
> | -- Check for working CXX compiler: /usr/lib/llvm/8/bin/clang++ -- works
> | -- Detecting CXX compiler ABI info
> | -- Detecting CXX compiler ABI info - done
> | -- Detecting CXX compile features
> | -- Detecting CXX compile features - done
> | CMake Warning (dev) in CMakeLists.txt:
> | No cmake_minimum_required command is present. A line of code such as
> |
> | cmake_minimum_required(VERSION 3.14)
> |
> | should be added at the top of the file. The version specified may be lower
> | if you wish to support older CMake versions for this project. For more
> | information run "cmake --help-policy CMP0000".
> | This warning is for project developers. Use -Wno-dev to suppress it.
> |
> | -- Configuring done
> | -- Generating done
> | -- Build files have been written to: <tarantool-path>/src/lib/http_parser
> | Scanning dependencies of target http_parser
> | [ 25%] Building C object CMakeFiles/http_parser.dir/http_parser.o
> | [ 50%] Linking C static library libhttp_parser.a
> | [ 50%] Built target http_parser
> | Scanning dependencies of target test_http_parser
> | [ 75%] Building C object CMakeFiles/test_http_parser.dir/test_http_parser.o
> | [100%] Linking C executable test_http_parser
> | [100%] Built target test_http_parser
>
> And this is the case I was writing about. Do you consider this as a bug?
>
> Side note: this *might* be the main reason, why unit tests aren't placed
> alongside with the corresponding libs but grouped within a separate
> directory as Serge mentioned in his review.
In a new version tests moved to a single directory test/fuzz like we did
with unit tests.
>
> <snipped>
>
>> --
>> sergeyb@
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-12-01 16:22 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-20 8:47 [Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers Sergey Bronnikov
2020-04-24 13:56 ` Serge Petrenko
2020-04-29 9:28 ` Sergey Bronnikov
2020-04-30 11:10 ` Serge Petrenko
2020-04-30 11:40 ` Sergey Bronnikov
2020-04-30 13:01 ` Serge Petrenko
2020-04-28 11:19 ` Igor Munkin
2020-04-29 12:09 ` Sergey Bronnikov
2020-05-26 15:54 ` Igor Munkin
2020-12-01 16:22 ` Sergey Bronnikov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox