diff --git a/include/nlohmann/detail/conversions/to_json.hpp b/include/nlohmann/detail/conversions/to_json.hpp index f3b4994f1..d75f34bb8 100644 --- a/include/nlohmann/detail/conversions/to_json.hpp +++ b/include/nlohmann/detail/conversions/to_json.hpp @@ -43,6 +43,10 @@ namespace detail * j.m_data.m_value.destroy(j.m_data.m_type) to avoid a memory leak in case j contains an * allocated value (e.g., a string). See bug issue * https://github.com/nlohmann/json/issues/2865 for more information. + * + * A value that has to be allocated is created before the old one is destroyed: + * were it the other way around, an exception while creating the new value would + * leave j with the type of the new value, but the pointer to the destroyed old one. */ template struct external_constructor; @@ -66,18 +70,20 @@ struct external_constructor template static void construct(BasicJsonType& j, const typename BasicJsonType::string_t& s) { + const typename BasicJsonType::json_value value(s); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::string; - j.m_data.m_value = s; + j.m_data.m_value = value; j.assert_invariant(); } template static void construct(BasicJsonType& j, typename BasicJsonType::string_t&& s) { + const typename BasicJsonType::json_value value(std::move(s)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::string; - j.m_data.m_value = std::move(s); + j.m_data.m_value = value; j.assert_invariant(); } @@ -86,9 +92,10 @@ struct external_constructor int > = 0 > static void construct(BasicJsonType& j, const CompatibleStringType& str) { + auto* created = j.template create(str); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::string; - j.m_data.m_value.string = j.template create(str); + j.m_data.m_value.string = created; j.assert_invariant(); } }; @@ -99,18 +106,20 @@ struct external_constructor template static void construct(BasicJsonType& j, const typename BasicJsonType::binary_t& b) { + const typename BasicJsonType::json_value value(b); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::binary; - j.m_data.m_value = typename BasicJsonType::binary_t(b); + j.m_data.m_value = value; j.assert_invariant(); } template static void construct(BasicJsonType& j, typename BasicJsonType::binary_t&& b) { + const typename BasicJsonType::json_value value(std::move(b)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::binary; - j.m_data.m_value = typename BasicJsonType::binary_t(std::move(b)); + j.m_data.m_value = value; j.assert_invariant(); } }; @@ -160,9 +169,10 @@ struct external_constructor template static void construct(BasicJsonType& j, const typename BasicJsonType::array_t& arr) { + const typename BasicJsonType::json_value value(arr); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::array; - j.m_data.m_value = arr; + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -170,9 +180,10 @@ struct external_constructor template static void construct(BasicJsonType& j, typename BasicJsonType::array_t&& arr) { + const typename BasicJsonType::json_value value(std::move(arr)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::array; - j.m_data.m_value = std::move(arr); + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -188,9 +199,10 @@ struct external_constructor using std::begin; using std::end; + auto* created = j.template create(begin(arr), end(arr)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::array; - j.m_data.m_value.array = j.template create(begin(arr), end(arr)); + j.m_data.m_value.array = created; j.set_parents(); j.assert_invariant(); } @@ -198,15 +210,17 @@ struct external_constructor template static void construct(BasicJsonType& j, const std::vector& arr) { - j.m_data.m_value.destroy(j.m_data.m_type); - j.m_data.m_type = value_t::array; - j.m_data.m_value = value_t::array; - j.m_data.m_value.array->reserve(arr.size()); + typename BasicJsonType::array_t elements; + elements.reserve(arr.size()); for (const bool x : arr) { - j.m_data.m_value.array->push_back(x); - j.set_parent(j.m_data.m_value.array->back()); + elements.push_back(x); } + const typename BasicJsonType::json_value value(std::move(elements)); + j.m_data.m_value.destroy(j.m_data.m_type); + j.m_data.m_type = value_t::array; + j.m_data.m_value = value; + j.set_parents(); j.assert_invariant(); } @@ -214,11 +228,12 @@ struct external_constructor enable_if_t::value, int> = 0> static void construct(BasicJsonType& j, const std::valarray& arr) { + typename BasicJsonType::array_t elements(arr.size()); + std::copy(std::begin(arr), std::end(arr), elements.begin()); + const typename BasicJsonType::json_value value(std::move(elements)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::array; - j.m_data.m_value = value_t::array; - j.m_data.m_value.array->resize(arr.size()); - std::copy(std::begin(arr), std::end(arr), j.m_data.m_value.array->begin()); + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -228,13 +243,15 @@ struct external_constructor enable_if_t>::value, int> = 0> static void construct(BasicJsonType& j, CompatibleArrayType && arr) { - j.m_data.m_value.destroy(j.m_data.m_type); - j.m_data.m_type = value_t::array; - j.m_data.m_value = value_t::array; + typename BasicJsonType::array_t elements; for (auto&& x : std::forward(arr)) { - j.m_data.m_value.array->push_back(x); + elements.push_back(x); } + const typename BasicJsonType::json_value value(std::move(elements)); + j.m_data.m_value.destroy(j.m_data.m_type); + j.m_data.m_type = value_t::array; + j.m_data.m_value = value; // set the parents only once all elements are in place: a push_back // that reallocates moves the earlier elements, which does not keep // their parent pointers @@ -250,9 +267,10 @@ struct external_constructor template static void construct(BasicJsonType& j, const typename BasicJsonType::object_t& obj) { + const typename BasicJsonType::json_value value(obj); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::object; - j.m_data.m_value = obj; + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -260,9 +278,10 @@ struct external_constructor template static void construct(BasicJsonType& j, typename BasicJsonType::object_t&& obj) { + const typename BasicJsonType::json_value value(std::move(obj)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::object; - j.m_data.m_value = std::move(obj); + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -274,9 +293,10 @@ struct external_constructor using std::begin; using std::end; + auto* created = j.template create(begin(obj), end(obj)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::object; - j.m_data.m_value.object = j.template create(begin(obj), end(obj)); + j.m_data.m_value.object = created; j.set_parents(); j.assert_invariant(); } diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 6f6bb8d04..10ceb6100 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -2056,8 +2056,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec if (is_an_object) { // the initializer list is a list of pairs -> create an object - m_data.m_type = value_t::object; m_data.m_value = value_t::object; + m_data.m_type = value_t::object; for (auto& element_ref : init) { @@ -2079,8 +2079,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } #endif // the initializer list describes an array -> create an array - m_data.m_type = value_t::array; m_data.m_value.array = create(init.begin(), init.end()); + m_data.m_type = value_t::array; } set_parents(); @@ -2093,8 +2093,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec static basic_json binary(const typename binary_t::container_type& init) { auto res = basic_json(); - res.m_data.m_type = value_t::binary; res.m_data.m_value = init; + res.m_data.m_type = value_t::binary; return res; } @@ -2104,8 +2104,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec static basic_json binary(const typename binary_t::container_type& init, typename binary_t::subtype_type subtype) { auto res = basic_json(); - res.m_data.m_type = value_t::binary; res.m_data.m_value = binary_t(init, subtype); + res.m_data.m_type = value_t::binary; return res; } @@ -2115,8 +2115,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec static basic_json binary(typename binary_t::container_type&& init) { auto res = basic_json(); - res.m_data.m_type = value_t::binary; res.m_data.m_value = std::move(init); + res.m_data.m_type = value_t::binary; return res; } @@ -2126,8 +2126,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec static basic_json binary(typename binary_t::container_type&& init, typename binary_t::subtype_type subtype) { auto res = basic_json(); - res.m_data.m_type = value_t::binary; res.m_data.m_value = binary_t(std::move(init), subtype); + res.m_data.m_type = value_t::binary; return res; } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index d0a651093..5d6f6e360 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -6837,6 +6837,10 @@ namespace detail * j.m_data.m_value.destroy(j.m_data.m_type) to avoid a memory leak in case j contains an * allocated value (e.g., a string). See bug issue * https://github.com/nlohmann/json/issues/2865 for more information. + * + * A value that has to be allocated is created before the old one is destroyed: + * were it the other way around, an exception while creating the new value would + * leave j with the type of the new value, but the pointer to the destroyed old one. */ template struct external_constructor; @@ -6860,18 +6864,20 @@ struct external_constructor template static void construct(BasicJsonType& j, const typename BasicJsonType::string_t& s) { + const typename BasicJsonType::json_value value(s); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::string; - j.m_data.m_value = s; + j.m_data.m_value = value; j.assert_invariant(); } template static void construct(BasicJsonType& j, typename BasicJsonType::string_t&& s) { + const typename BasicJsonType::json_value value(std::move(s)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::string; - j.m_data.m_value = std::move(s); + j.m_data.m_value = value; j.assert_invariant(); } @@ -6880,9 +6886,10 @@ struct external_constructor int > = 0 > static void construct(BasicJsonType& j, const CompatibleStringType& str) { + auto* created = j.template create(str); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::string; - j.m_data.m_value.string = j.template create(str); + j.m_data.m_value.string = created; j.assert_invariant(); } }; @@ -6893,18 +6900,20 @@ struct external_constructor template static void construct(BasicJsonType& j, const typename BasicJsonType::binary_t& b) { + const typename BasicJsonType::json_value value(b); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::binary; - j.m_data.m_value = typename BasicJsonType::binary_t(b); + j.m_data.m_value = value; j.assert_invariant(); } template static void construct(BasicJsonType& j, typename BasicJsonType::binary_t&& b) { + const typename BasicJsonType::json_value value(std::move(b)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::binary; - j.m_data.m_value = typename BasicJsonType::binary_t(std::move(b)); + j.m_data.m_value = value; j.assert_invariant(); } }; @@ -6954,9 +6963,10 @@ struct external_constructor template static void construct(BasicJsonType& j, const typename BasicJsonType::array_t& arr) { + const typename BasicJsonType::json_value value(arr); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::array; - j.m_data.m_value = arr; + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -6964,9 +6974,10 @@ struct external_constructor template static void construct(BasicJsonType& j, typename BasicJsonType::array_t&& arr) { + const typename BasicJsonType::json_value value(std::move(arr)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::array; - j.m_data.m_value = std::move(arr); + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -6982,9 +6993,10 @@ struct external_constructor using std::begin; using std::end; + auto* created = j.template create(begin(arr), end(arr)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::array; - j.m_data.m_value.array = j.template create(begin(arr), end(arr)); + j.m_data.m_value.array = created; j.set_parents(); j.assert_invariant(); } @@ -6992,15 +7004,17 @@ struct external_constructor template static void construct(BasicJsonType& j, const std::vector& arr) { - j.m_data.m_value.destroy(j.m_data.m_type); - j.m_data.m_type = value_t::array; - j.m_data.m_value = value_t::array; - j.m_data.m_value.array->reserve(arr.size()); + typename BasicJsonType::array_t elements; + elements.reserve(arr.size()); for (const bool x : arr) { - j.m_data.m_value.array->push_back(x); - j.set_parent(j.m_data.m_value.array->back()); + elements.push_back(x); } + const typename BasicJsonType::json_value value(std::move(elements)); + j.m_data.m_value.destroy(j.m_data.m_type); + j.m_data.m_type = value_t::array; + j.m_data.m_value = value; + j.set_parents(); j.assert_invariant(); } @@ -7008,11 +7022,12 @@ struct external_constructor enable_if_t::value, int> = 0> static void construct(BasicJsonType& j, const std::valarray& arr) { + typename BasicJsonType::array_t elements(arr.size()); + std::copy(std::begin(arr), std::end(arr), elements.begin()); + const typename BasicJsonType::json_value value(std::move(elements)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::array; - j.m_data.m_value = value_t::array; - j.m_data.m_value.array->resize(arr.size()); - std::copy(std::begin(arr), std::end(arr), j.m_data.m_value.array->begin()); + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -7022,13 +7037,15 @@ struct external_constructor enable_if_t>::value, int> = 0> static void construct(BasicJsonType& j, CompatibleArrayType && arr) { - j.m_data.m_value.destroy(j.m_data.m_type); - j.m_data.m_type = value_t::array; - j.m_data.m_value = value_t::array; + typename BasicJsonType::array_t elements; for (auto&& x : std::forward(arr)) { - j.m_data.m_value.array->push_back(x); + elements.push_back(x); } + const typename BasicJsonType::json_value value(std::move(elements)); + j.m_data.m_value.destroy(j.m_data.m_type); + j.m_data.m_type = value_t::array; + j.m_data.m_value = value; // set the parents only once all elements are in place: a push_back // that reallocates moves the earlier elements, which does not keep // their parent pointers @@ -7044,9 +7061,10 @@ struct external_constructor template static void construct(BasicJsonType& j, const typename BasicJsonType::object_t& obj) { + const typename BasicJsonType::json_value value(obj); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::object; - j.m_data.m_value = obj; + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -7054,9 +7072,10 @@ struct external_constructor template static void construct(BasicJsonType& j, typename BasicJsonType::object_t&& obj) { + const typename BasicJsonType::json_value value(std::move(obj)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::object; - j.m_data.m_value = std::move(obj); + j.m_data.m_value = value; j.set_parents(); j.assert_invariant(); } @@ -7068,9 +7087,10 @@ struct external_constructor using std::begin; using std::end; + auto* created = j.template create(begin(obj), end(obj)); j.m_data.m_value.destroy(j.m_data.m_type); j.m_data.m_type = value_t::object; - j.m_data.m_value.object = j.template create(begin(obj), end(obj)); + j.m_data.m_value.object = created; j.set_parents(); j.assert_invariant(); } @@ -29236,8 +29256,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec if (is_an_object) { // the initializer list is a list of pairs -> create an object - m_data.m_type = value_t::object; m_data.m_value = value_t::object; + m_data.m_type = value_t::object; for (auto& element_ref : init) { @@ -29259,8 +29279,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } #endif // the initializer list describes an array -> create an array - m_data.m_type = value_t::array; m_data.m_value.array = create(init.begin(), init.end()); + m_data.m_type = value_t::array; } set_parents(); @@ -29273,8 +29293,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec static basic_json binary(const typename binary_t::container_type& init) { auto res = basic_json(); - res.m_data.m_type = value_t::binary; res.m_data.m_value = init; + res.m_data.m_type = value_t::binary; return res; } @@ -29284,8 +29304,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec static basic_json binary(const typename binary_t::container_type& init, typename binary_t::subtype_type subtype) { auto res = basic_json(); - res.m_data.m_type = value_t::binary; res.m_data.m_value = binary_t(init, subtype); + res.m_data.m_type = value_t::binary; return res; } @@ -29295,8 +29315,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec static basic_json binary(typename binary_t::container_type&& init) { auto res = basic_json(); - res.m_data.m_type = value_t::binary; res.m_data.m_value = std::move(init); + res.m_data.m_type = value_t::binary; return res; } @@ -29306,8 +29326,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec static basic_json binary(typename binary_t::container_type&& init, typename binary_t::subtype_type subtype) { auto res = basic_json(); - res.m_data.m_type = value_t::binary; res.m_data.m_value = binary_t(std::move(init), subtype); + res.m_data.m_type = value_t::binary; return res; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8735934de..ef806892e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -141,6 +141,11 @@ json_test_set_test_options(test-unicode4 TEST_PROPERTIES TIMEOUT 3000) # only the #972 regression test needs thirdparty/fifo_map on its include path json_test_set_test_options(test-regression1 LINK_LIBRARIES fifo_map_include) +# GCC's false -Warray-bounds error with JSON_DIAGNOSTICS only shows up when optimizing (#5742) +json_test_set_test_options(test-diagnostics-optimized + COMPILE_OPTIONS $<$:-O3 -Werror=array-bounds> +) + ############################################################################# # add unit tests ############################################################################# diff --git a/tests/src/unit-allocator.cpp b/tests/src/unit-allocator.cpp index a358e75f9..647748678 100644 --- a/tests/src/unit-allocator.cpp +++ b/tests/src/unit-allocator.cpp @@ -12,6 +12,11 @@ #include using nlohmann::json; +#include +#if JSON_HAS_RANGES + #include +#endif + namespace { // special test case to check if memory is leaked if constructor throws @@ -607,3 +612,173 @@ TEST_CASE("bad my_allocator::construct") j["test"].push_back("should not leak"); } } + +// the no-exceptions CI job skips every CHECK_THROWS_AS, which would leave +// next_construct_fails set for the next allocation outside a check +#if !defined(JSON_NOEXCEPTION) +TEST_CASE("a failed allocation leaves the value unchanged") +{ + // create JSON type using the throwing allocator + using my_json = nlohmann::basic_json; + + // Each of these creates a string, array, object, or binary value. The + // value must be created before the type is changed: otherwise, a failed + // creation left a value of the new type without anything behind it (an + // assertion in its destructor, a null pointer everywhere else) or, when + // an old value was destroyed first, with a pointer to that destroyed one. + + SECTION("creating a binary value") + { + const std::vector bytes = {1, 2, 3}; + my_json _; + + next_construct_fails = true; + CHECK_THROWS_AS(_ = my_json::binary(bytes), std::bad_alloc&); + next_construct_fails = true; + CHECK_THROWS_AS(_ = my_json::binary(bytes, 42), std::bad_alloc&); + next_construct_fails = true; + CHECK_THROWS_AS(_ = my_json::binary(std::vector(bytes)), std::bad_alloc&); + next_construct_fails = true; + CHECK_THROWS_AS(_ = my_json::binary(std::vector(bytes), 42), std::bad_alloc&); + next_construct_fails = false; + } + + SECTION("turning a null value into an array or object") + { + my_json j; + + next_construct_fails = true; + CHECK_THROWS_AS(j[0], std::bad_alloc&); + CHECK(j.is_null()); + + next_construct_fails = true; + CHECK_THROWS_AS(j["key"], std::bad_alloc&); + CHECK(j.is_null()); + +#ifdef JSON_HAS_CPP_17 + next_construct_fails = true; + CHECK_THROWS_AS(j[std::string_view("key")], std::bad_alloc&); + CHECK(j.is_null()); +#endif + + next_construct_fails = true; + CHECK_THROWS_AS(j.push_back(my_json(1)), std::bad_alloc&); + CHECK(j.is_null()); + + const my_json one = 1; + next_construct_fails = true; + CHECK_THROWS_AS(j.push_back(one), std::bad_alloc&); + CHECK(j.is_null()); + + next_construct_fails = true; + CHECK_THROWS_AS(j.push_back(my_json::object_t::value_type("key", 1)), std::bad_alloc&); + CHECK(j.is_null()); + + next_construct_fails = true; + CHECK_THROWS_AS(j.emplace_back(1), std::bad_alloc&); + CHECK(j.is_null()); + + next_construct_fails = true; + CHECK_THROWS_AS(j.emplace("key", 1), std::bad_alloc&); + CHECK(j.is_null()); + + const my_json object = {{"key", 1}}; + next_construct_fails = true; + CHECK_THROWS_AS(j.update(object), std::bad_alloc&); + CHECK(j.is_null()); + + next_construct_fails = false; + } + + // With iterator debugging, VS 2015's containers construct a proxy with the + // allocator in constructors that cannot report its failure, so a failing + // allocator crashes this section there (SIGSEGV with VS 2015 Debug x86). +#if !(defined(_MSC_VER) && _MSC_VER < 1910 && defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL > 0) + SECTION("converting into an existing value") + { + // to_json replaces the value it is given; the old one must survive a + // failed creation of the new one + my_json j = "old"; + + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, std::string("new")), std::bad_alloc&); + CHECK(j == "old"); + + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, std::vector {1, 2}), std::bad_alloc&); + CHECK(j == "old"); + + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, std::vector {true, false}), std::bad_alloc&); + CHECK(j == "old"); + + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, std::map {{"a", 1}}), std::bad_alloc&); + CHECK(j == "old"); + + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, my_json::binary_t({1, 2})), std::bad_alloc&); + CHECK(j == "old"); + + // the overloads for lvalues of the value types, for the value types + // themselves, and for the remaining compatible types + const std::string string = "new"; + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, string), std::bad_alloc&); + CHECK(j == "old"); + + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, "new"), std::bad_alloc&); + CHECK(j == "old"); + + // to_json only moves a binary value that it converted from another + // container type, which my_json's std::vector is not + using binary_constructor = nlohmann::detail::external_constructor; + next_construct_fails = true; + CHECK_THROWS_AS(binary_constructor::construct(j, my_json::binary_t({1, 2})), std::bad_alloc&); + CHECK(j == "old"); + + my_json::array_t array = {1, 2}; + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, array), std::bad_alloc&); + CHECK(j == "old"); + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, std::move(array)), std::bad_alloc&); + CHECK(j == "old"); + + my_json::object_t object = {{"a", 1}}; + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, object), std::bad_alloc&); + CHECK(j == "old"); + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, std::move(object)), std::bad_alloc&); + CHECK(j == "old"); + + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, std::valarray {1, 2}), std::bad_alloc&); + CHECK(j == "old"); + +#if JSON_HAS_RANGES && !defined(__MINGW32__) + const std::vector numbers = {1, 2}; + next_construct_fails = true; + CHECK_THROWS_AS(nlohmann::to_json(j, numbers | std::views::filter([](int /*unused*/) + { + return true; + })), std::bad_alloc&); + CHECK(j == "old"); +#endif + + next_construct_fails = false; + nlohmann::to_json(j, std::vector {1, 2}); + CHECK(j == my_json({1, 2})); + } +#endif +} +#endif diff --git a/tests/src/unit-diagnostics-optimized.cpp b/tests/src/unit-diagnostics-optimized.cpp new file mode 100644 index 000000000..a020c2fe9 --- /dev/null +++ b/tests/src/unit-diagnostics-optimized.cpp @@ -0,0 +1,80 @@ +// __ _____ _____ _____ +// __| | __| | | | JSON for Modern C++ (supporting code) +// | | |__ | | | | | | version 3.12.0 +// |_____|_____|_____|_|___| https://github.com/nlohmann/json +// +// SPDX-FileCopyrightText: 2013-2026 Niels Lohmann +// SPDX-License-Identifier: MIT + +// Regression test for https://github.com/nlohmann/json/issues/5742: with +// JSON_DIAGNOSTICS, GCC (12 to at least 16) reported a false -Warray-bounds +// error in the inlined set_parents() at -O3. The type of a new string was set +// before the string was allocated, so GCC had to assume that operator new +// could change it again and checked the object branch of set_parents() +// against the string's allocation. Setting the type after creating the value +// avoids this. The warning depends on GCC's inlining decisions, so the +// sections cover two patterns that trigger it on different GCC versions +// (#4819 and #5742). +// On GCC, this file is compiled with -O3 -Werror=array-bounds (see +// tests/CMakeLists.txt), so the test fails to build if the warning returns. + +#include "doctest_compatibility.h" + +#ifdef JSON_DIAGNOSTICS + #undef JSON_DIAGNOSTICS +#endif + +#define JSON_DIAGNOSTICS 1 + +#include +using nlohmann::json; + +#include +#include +#include +#include + +namespace +{ +enum class diag_color +{ + red, + green, + blue +}; + +void to_json(json& j, const diag_color& c) +{ + static const std::pair m[] = // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) + { + {diag_color::red, "r"}, + {diag_color::green, "g"}, + {diag_color::blue, "b"}, + }; + const auto* it = std::find_if(std::begin(m), std::end(m), [c](const std::pair& p) + { + return p.first == c; + }); + j = it->second; +} +} // namespace + +TEST_CASE("diagnostics with optimization") +{ + SECTION("issue #4819 - object in vector") + { + std::vector jsons{}; + jsons.emplace_back(json({{"key", "value"}})); + CHECK(jsons.back()["key"] == "value"); + } + + SECTION("issue #5742 - string values from a static table") + { + json j = json::array(); + j.push_back(diag_color::red); + j.push_back(diag_color::green); + j.push_back(diag_color::blue); + CHECK(j.dump() == R"(["r","g","b"])"); + CHECK_THROWS_WITH_AS(j[1].get(), "[json.exception.type_error.302] (/1) type must be number, but is string", json::type_error); + } +}