From f0bff49ffd27452f32ec456af1ac39b8c3a5fc13 Mon Sep 17 00:00:00 2001 From: Thomas Braun Date: Tue, 27 Aug 2019 14:22:01 +0200 Subject: [PATCH 1/5] test/CMakeLists.txt: Remove trailing whitespace --- test/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index b73dfc9a..c34f38f0 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -71,7 +71,7 @@ set_target_properties(doctest_main PROPERTIES COMPILE_DEFINITIONS "$<$:_SCL_SECURE_NO_WARNINGS>" COMPILE_OPTIONS "$<$:/EHsc;$<$:/Od>>" ) -if (${CMAKE_VERSION} VERSION_LESS "3.8.0") +if (${CMAKE_VERSION} VERSION_LESS "3.8.0") target_compile_features(doctest_main PUBLIC cxx_range_for) else() target_compile_features(doctest_main PUBLIC cxx_std_11) @@ -92,7 +92,7 @@ if(MSVC) # Disable warning C4566: character represented by universal-character-name '\uFF01' cannot be represented in the current code page (1252) # Disable warning C4996: 'nlohmann::basic_json::operator <<': was declared deprecated set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4389 /wd4309 /wd4566 /wd4996") - + # https://github.com/nlohmann/json/issues/1114 set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /bigobj") endif() From 9ea3e191213f8c282218b5a3251f1a9f56db01b3 Mon Sep 17 00:00:00 2001 From: Thomas Braun Date: Tue, 27 Aug 2019 14:22:19 +0200 Subject: [PATCH 2/5] .travis/cmake: Rework clang sanitizer invocation - Switch to clang-7 - Adapt PATH so that llvm-symbolizer can be found for useful stacktraces - Adapt compile flags "-O0" ensures much faster compile times "-fno-sanitize-recover=all -fsanitize-recover=unsigned-integer-overflow" this fails the build on all issues except unsigned integer overflows. Not failing in this case is required in combination with the sanitizer suppression file as only recoverable errors can be suppressed. The UBSAN suppression file ignores errors from stl_bvector.h (which holds std::vector). Clang reports that error as SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/bits/stl_bvector.h:158:20 in Start 34: test-deserialization_all 28/88 Test #71: test-testsuites_default .............***Failed 0.32 sec /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/bits/stl_bvector.h:158:20: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'unsigned int' #0 0x628f72 in std::_Bit_iterator_base::_M_bump_down() /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/bits/stl_bvector.h:158:20 #1 0x628d16 in std::_Bit_iterator::operator--() /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/bits/stl_bvector.h:251:7 #2 0x634aac in std::vector >::pop_back() /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/bits/stl_bvector.h:1010:7 #3 0x61eff0 in bool nlohmann::detail::parser, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer> >::sax_parse_internal, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer> > >(nlohmann::detail::json_sax_dom_parser, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer> >*) /home/firma/devel/json/include/nlohmann/detail/input/parser.hpp:439:28 #4 0x604864 in nlohmann::detail::parser, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer> >::parse(bool, nlohmann::basic_json, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer>&) /home/firma/devel/json/include/nlohmann/detail/input/parser.hpp:116:13 #5 0x5f8079 in nlohmann::operator>>(std::istream&, nlohmann::basic_json, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer>&) /home/firma/devel/json/include/nlohmann/json.hpp:6356:42 #6 0x5e1d92 in _DOCTEST_ANON_FUNC_21() /home/firma/devel/json/test/src/unit-testsuites.cpp:343:9 #7 0x7207fe in doctest::Context::run() /home/firma/devel/json/test/thirdparty/doctest/doctest.h:5938:21 #8 0x72681a in main /home/firma/devel/json/test/thirdparty/doctest/doctest.h:6016:71 #9 0x7f75d22362e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0) #10 0x4c28b9 in _start (/home/firma/devel/json/build/test/test-testsuites+0x4c28b9) The pop_back() in parser.hpp assert(not states.empty()); -> states.pop_back(); triggers the UBSAN report. But the assertion above ensure that we only call pop_back() on an non-empty vector, therefore this is a STL library bug and thus must be ignored for us. --- .travis.yml | 9 ++++++--- test/CMakeLists.txt | 2 +- test/src/UBSAN.supp | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 test/src/UBSAN.supp diff --git a/.travis.yml b/.travis.yml index 7078fd09..a8f5e96f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -43,12 +43,15 @@ matrix: - os: linux compiler: clang env: - - COMPILER=clang++-5.0 + - COMPILER=clang++-7 - CMAKE_OPTIONS=-DJSON_Sanitizer=ON + - UBSAN_OPTIONS=print_stacktrace=1,suppressions=$(pwd)/test/src/UBSAN.supp addons: apt: - sources: ['ubuntu-toolchain-r-test', 'llvm-toolchain-trusty-5.0'] - packages: ['g++-6', 'clang-5.0', 'ninja-build'] + sources: ['ubuntu-toolchain-r-test', 'llvm-toolchain-trusty-7'] + packages: ['g++-6', 'clang-7', 'ninja-build'] + before_script: + - export PATH=$PATH:/usr/lib/llvm-7/bin # cppcheck - os: linux diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c34f38f0..e13d31bb 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -6,7 +6,7 @@ option(JSON_Coverage "Build test suite with coverage information" OFF) if(JSON_Sanitizer) message(STATUS "Building test suite with Clang sanitizer") if(NOT MSVC) - set(CMAKE_CXX_FLAGS "-g -O2 -fsanitize=address -fsanitize=undefined -fsanitize=integer -fsanitize=nullability -fno-omit-frame-pointer") + set(CMAKE_CXX_FLAGS "-g -O0 -fsanitize=address -fsanitize=undefined -fsanitize=integer -fsanitize=nullability -fno-omit-frame-pointer -fno-sanitize-recover=all -fsanitize-recover=unsigned-integer-overflow") endif() endif() diff --git a/test/src/UBSAN.supp b/test/src/UBSAN.supp new file mode 100644 index 00000000..b19f0436 --- /dev/null +++ b/test/src/UBSAN.supp @@ -0,0 +1 @@ +unsigned-integer-overflow:stl_bvector.h From 61fe5f1eee4cb95ca2625580c009d6c9aa7040a7 Mon Sep 17 00:00:00 2001 From: Thomas Braun Date: Fri, 30 Aug 2019 12:39:46 +0200 Subject: [PATCH 3/5] input_buffer_adapter: Fix handling of nullptr input Clang UBSAN currently complains that the char * to input_buffer_adapter is a nullptr. Turns out it is actually required to accept nullptr, see for example line 415 in input_adapters.hpp ... // the address of first cannot be used: use nullptr ia = std::make_shared(nullptr, len); .... Therefore we have to handle it gracefully here. We now also ignore the length parameter l if b is a nullptr. --- include/nlohmann/detail/input/input_adapters.hpp | 4 ++-- single_include/nlohmann/json.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index b7b11de0..9512a771 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -131,9 +131,8 @@ class input_stream_adapter : public input_adapter_protocol class input_buffer_adapter : public input_adapter_protocol { public: - JSON_HEDLEY_NON_NULL(2) input_buffer_adapter(const char* b, const std::size_t l) noexcept - : cursor(b), limit(b + l) + : cursor(b), limit(b == nullptr ? nullptr : (b + l)) {} // delete because of pointer members @@ -147,6 +146,7 @@ class input_buffer_adapter : public input_adapter_protocol { if (JSON_HEDLEY_LIKELY(cursor < limit)) { + assert(cursor != nullptr and limit != nullptr); return std::char_traits::to_int_type(*(cursor++)); } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 2a32a829..4bbe2707 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -3882,9 +3882,8 @@ class input_stream_adapter : public input_adapter_protocol class input_buffer_adapter : public input_adapter_protocol { public: - JSON_HEDLEY_NON_NULL(2) input_buffer_adapter(const char* b, const std::size_t l) noexcept - : cursor(b), limit(b + l) + : cursor(b), limit(b == nullptr ? nullptr : (b + l)) {} // delete because of pointer members @@ -3898,6 +3897,7 @@ class input_buffer_adapter : public input_adapter_protocol { if (JSON_HEDLEY_LIKELY(cursor < limit)) { + assert(cursor != nullptr and limit != nullptr); return std::char_traits::to_int_type(*(cursor++)); } From d5c0d52f3798811f9673ee952af3181721bed3d0 Mon Sep 17 00:00:00 2001 From: Thomas Braun Date: Fri, 30 Aug 2019 13:07:37 +0200 Subject: [PATCH 4/5] external_constructor: Handle empty array properly Clang UBSAN complains with the following message when an empty std::valarray is passed in: SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/valarray:571:14 in 2/2 Test #68: test-regression_all ..............***Failed 4.68 sec /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/valarray:571:14: runtime error: reference binding to null pointer of type 'const do uble' #0 0x6fbe57 in std::valarray::operator[](unsigned long) const /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/valarray: 571:7 #1 0x6fbe57 in double const* std::begin(std::valarray const&) /usr/lib/gcc/x86_64-linux-gnu/6.3.0/../../../../include/c++/6.3.0/v alarray:1207 #2 0x6fbe57 in void nlohmann::detail::external_constructor<(nlohmann::detail::value_t)2>::construct, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_seri alizer>, double, 0>(nlohmann::basic_json, std::allocator >, bool , long, unsigned long, double, std::allocator, nlohmann::adl_serializer>&, std::valarray const&) /home/firma/devel/json/include/nlohmann/deta il/conversions/to_json.hpp:157 #3 0x5e3fe3 in void nlohmann::detail::to_json , std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer>, double, 0>(nlohmann::basic_json, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohman n::adl_serializer>&, std::valarray const&) /home/firma/devel/json/include/nlohmann/detail/conversions/to_json.hpp:270:5 #4 0x5e3fe3 in decltype((to_json(fp, std::forward&>(fp0))) , ((void)())) nlohmann::detail::to_json_fn::operator(), std::allocator >, bool, long, unsigned long, double , std::allocator, nlohmann::adl_serializer>, std::valarray&>(nlohmann::basic_json, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer>&, std::valarray&) c onst /home/firma/devel/json/include/nlohmann/detail/conversions/to_json.hpp:334 #5 0x5e3fe3 in decltype((nlohmann::(anonymous namespace)::to_json(fp, std::forward&>(fp0))) , ((void)())) nlohmann::adl_ser ializer, void>::to_json, st d::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer>, std::valarray&>(nlohmann::basic_json, std::allocator >, bool, long, unsigned long, double, std::allocator , nlohmann::adl_serializer>&, std::valarray&) /home/firma/devel/json/include/nlohmann/adl_serializer.hpp:45 #6 0x5e3fe3 in nlohmann::basic_json, std::allocator >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer>::basic_json&, std::valarray, 0>(std::valarray&) /home/firma/devel/json/include/nlohmann/json.hpp:1257 #7 0x5e3fe3 in _DOCTEST_ANON_FUNC_2() /home/firma/devel/json/test/src/unit-regression.cpp:1377 #8 0x77313e in doctest::Context::run() /home/firma/devel/json/test/thirdparty/doctest/doctest.h:5938:21 #9 0x777ae0 in main /home/firma/devel/json/test/thirdparty/doctest/doctest.h:6016:71 #10 0x7fae220532e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0) #11 0x4a6479 in _start (/home/firma/devel/json/build/test/test-regression+0x4a6479) The important thing to note here is that a std::valarray is *not* a STL container, so the usual containter and iterator semantics don't apply. Therefore we have to check if the container is non-empty before. --- include/nlohmann/detail/conversions/to_json.hpp | 5 ++++- single_include/nlohmann/json.hpp | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/nlohmann/detail/conversions/to_json.hpp b/include/nlohmann/detail/conversions/to_json.hpp index db9eaf2b..c3ac5aa8 100644 --- a/include/nlohmann/detail/conversions/to_json.hpp +++ b/include/nlohmann/detail/conversions/to_json.hpp @@ -154,7 +154,10 @@ struct external_constructor j.m_type = value_t::array; j.m_value = value_t::array; j.m_value.array->resize(arr.size()); - std::copy(std::begin(arr), std::end(arr), j.m_value.array->begin()); + if (arr.size() > 0) + { + std::copy(std::begin(arr), std::end(arr), j.m_value.array->begin()); + } j.assert_invariant(); } }; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 4bbe2707..e54c3e41 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -3489,7 +3489,10 @@ struct external_constructor j.m_type = value_t::array; j.m_value = value_t::array; j.m_value.array->resize(arr.size()); - std::copy(std::begin(arr), std::end(arr), j.m_value.array->begin()); + if (arr.size() > 0) + { + std::copy(std::begin(arr), std::end(arr), j.m_value.array->begin()); + } j.assert_invariant(); } }; From 6d701b29dfa5824b852c8714e75ca6546e8f4de5 Mon Sep 17 00:00:00 2001 From: Thomas Braun Date: Tue, 3 Sep 2019 13:32:25 +0200 Subject: [PATCH 5/5] .travis.yml: Increase the timeout to 45 minutes The clang sanitizer tests, and there especially the unicode tests, can hit the default timeout of 25 minutes (1500 seconds) quite easily, so let's raise the timeout to 45 minutes (2700 seconds). --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a8f5e96f..8f871c09 100644 --- a/.travis.yml +++ b/.travis.yml @@ -333,7 +333,7 @@ script: # compile and execute unit tests - mkdir -p build && cd build - cmake .. ${CMAKE_OPTIONS} -DJSON_MultipleHeaders=${MULTIPLE_HEADERS} -GNinja && cmake --build . --config Release - - ctest -C Release -V -j + - ctest -C Release --timeout 2700 -V -j - cd .. # check if homebrew works (only checks develop branch)