[Tarantool-patches] [PATCH v2] Add infrastructure for fuzzing testing and fuzzers
Igor Munkin
imun at tarantool.org
Tue May 26 18:54:04 MSK 2020
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
More information about the Tarantool-patches
mailing list