From 6ce2f35ba8898985ad62e265a9b448ae678dfc4e Mon Sep 17 00:00:00 2001 From: Thomas Braun Date: Fri, 23 Aug 2019 23:52:48 +0200 Subject: [PATCH 1/3] Fix outputting extreme integer values in edge cases For some gcc version (Ubuntu 5.5.0-12ubuntu1~16.04) the existing code crashes when the minimum value of int64_t is outputted. Resurrect the code from before 546e2cbf (:rotating_light: fixed some warnings, 2019-03-13) but delegate the sign removal so that the compilers don't complain about taking the negative value of an unsigned value. In addition we also rewrite the expression so that we first increment and then negate. The definition of remove_sign(number_unsigned_t) is never called as unsigned values are never negative. --- include/nlohmann/detail/output/serializer.hpp | 28 ++++++++++++++++++- single_include/nlohmann/json.hpp | 28 ++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/include/nlohmann/detail/output/serializer.hpp b/include/nlohmann/detail/output/serializer.hpp index e6811ceb..0a4ddbda 100644 --- a/include/nlohmann/detail/output/serializer.hpp +++ b/include/nlohmann/detail/output/serializer.hpp @@ -630,7 +630,7 @@ class serializer if (is_negative) { *buffer_ptr = '-'; - abs_value = static_cast(std::abs(static_cast(x))); + abs_value = remove_sign(x); // account one more byte for the minus sign n_chars = 1 + count_digits(abs_value); @@ -811,6 +811,32 @@ class serializer return state; } + /* + * Overload to make the compiler happy while it is instantiating + * dump_integer for number_unsigned_t. + * Must never be called. + */ + number_unsigned_t remove_sign(number_unsigned_t x) + { + assert(false); // LCOV_EXCL_LINE + return x; // LCOV_EXCL_LINE + } + + /* + * Helper function for dump_integer + * + * This function takes a negative signed integer and returns its absolute + * value as unsigned integer. The plus/minus shuffling is necessary as we can + * not directly remove the sign of an arbitrary signed integer as the + * absolute values of INT_MIN and INT_MAX are usually not the same. See + * #1708 for details. + */ + inline number_unsigned_t remove_sign(number_integer_t x) noexcept + { + assert(x < 0 and x < (std::numeric_limits::max)()); + return static_cast(-(x + 1)) + 1; + } + private: /// the output of the serializer output_adapter_t o = nullptr; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 2a32a829..a9f00a94 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -14264,7 +14264,7 @@ class serializer if (is_negative) { *buffer_ptr = '-'; - abs_value = static_cast(std::abs(static_cast(x))); + abs_value = remove_sign(x); // account one more byte for the minus sign n_chars = 1 + count_digits(abs_value); @@ -14445,6 +14445,32 @@ class serializer return state; } + /* + * Overload to make the compiler happy while it is instantiating + * dump_integer for number_unsigned_t. + * Must never be called. + */ + number_unsigned_t remove_sign(number_unsigned_t x) + { + assert(false); // LCOV_EXCL_LINE + return x; // LCOV_EXCL_LINE + } + + /* + * Helper function for dump_integer + * + * This function takes a negative signed integer and returns its absolute + * value as unsigned integer. The plus/minus shuffling is necessary as we can + * not directly remove the sign of an arbitrary signed integer as the + * absolute values of INT_MIN and INT_MAX are usually not the same. See + * #1708 for details. + */ + inline number_unsigned_t remove_sign(number_integer_t x) noexcept + { + assert(x < 0 and x < (std::numeric_limits::max)()); + return static_cast(-(x + 1)) + 1; + } + private: /// the output of the serializer output_adapter_t o = nullptr; From 70aa8a31a26d8605be14d43b6c40c53982c7db70 Mon Sep 17 00:00:00 2001 From: Thomas Braun Date: Fri, 23 Aug 2019 23:50:13 +0200 Subject: [PATCH 2/3] Add regression test for dumping the minimum value of int64_t --- test/src/unit-regression.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/src/unit-regression.cpp b/test/src/unit-regression.cpp index 866a985b..c9e8b0d7 100644 --- a/test/src/unit-regression.cpp +++ b/test/src/unit-regression.cpp @@ -1802,6 +1802,13 @@ TEST_CASE("regression tests") json j = json::parse("[-9223372036854775808]"); CHECK(j.dump() == "[-9223372036854775808]"); } + + SECTION("issue #1708 - minimum value of int64_t can be outputted") + { + constexpr auto smallest = (std::numeric_limits::min)(); + json j = smallest; + CHECK(j.dump() == std::to_string(smallest)); + } } #if not defined(JSON_NOEXCEPTION) From 8067c3ca5b840f734571660ac9e377331f19f90a Mon Sep 17 00:00:00 2001 From: Thomas Braun Date: Sun, 25 Aug 2019 22:02:49 +0200 Subject: [PATCH 3/3] Add serialization unit tests for extreme integer values --- test/src/unit-serialization.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/src/unit-serialization.cpp b/test/src/unit-serialization.cpp index 3a0b28e4..dadec343 100644 --- a/test/src/unit-serialization.cpp +++ b/test/src/unit-serialization.cpp @@ -189,3 +189,20 @@ TEST_CASE("serialization") test("[3,\"false\",false]", "[3,\\\"false\\\",false]"); } } + +TEST_CASE_TEMPLATE("serialization for extreme integer values", T, int32_t, uint32_t, int64_t, uint64_t) +{ + SECTION("minimum") + { + constexpr auto minimum = (std::numeric_limits::min)(); + json j = minimum; + CHECK(j.dump() == std::to_string(minimum)); + } + + SECTION("maximum") + { + constexpr auto maximum = (std::numeric_limits::max)(); + json j = maximum; + CHECK(j.dump() == std::to_string(maximum)); + } +}