Tarantool development patches archive
 help / color / mirror / Atom feed
* [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