From 56bb9992b0fb327a5d147c6e0fc17a290d698a72 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 10:46:52 -0400 Subject: [PATCH 01/16] support building with ASAN and UBSAN on Clang and GCC --- CMakeLists.txt | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index d315b74..96bfdc2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,8 @@ CMAKE_MINIMUM_REQUIRED(VERSION 2.8) +if(POLICY CMP0025) + # detect Apple's Clang + cmake_policy(SET CMP0025 NEW) +endif() if(POLICY CMP0054) cmake_policy(SET CMP0054 NEW) endif() @@ -28,6 +32,9 @@ option(RAPIDJSON_BUILD_THIRDPARTY_GTEST option(RAPIDJSON_BUILD_CXX11 "Build rapidjson with C++11 (gcc/clang)" ON) +option(RAPIDJSON_BUILD_ASAN "Build rapidjson with address sanitizer (gcc/clang)" OFF) +option(RAPIDJSON_BUILD_UBSAN "Build rapidjson with undefined behavior sanitizer (gcc/clang)" OFF) + option(RAPIDJSON_HAS_STDSTRING "" OFF) if(RAPIDJSON_HAS_STDSTRING) add_definitions(-DRAPIDJSON_HAS_STDSTRING) @@ -51,11 +58,35 @@ if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") endif() endif() + if (RAPIDJSON_BUILD_ASAN) + if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS "4.8.0") + message(FATAL_ERROR "GCC < 4.8 doesn't support the address sanitizer") + else() + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address") + endif() + endif() + if (RAPIDJSON_BUILD_UBSAN) + if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS "4.9.0") + message(FATAL_ERROR "GCC < 4.9 doesn't support the undefined behavior sanitizer") + else() + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined") + endif() + endif() elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=native -Wall -Wextra -Werror -Wno-missing-field-initializers") if (RAPIDJSON_BUILD_CXX11) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") endif() + if (RAPIDJSON_BUILD_ASAN) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address") + endif() + if (RAPIDJSON_BUILD_UBSAN) + if (CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error") + else() + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined") + endif() + endif() elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") add_definitions(-D_CRT_SECURE_NO_WARNINGS=1) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc") From 89f6b8a380c326bd37defb0e330f31a0d894617d Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 11:16:28 -0400 Subject: [PATCH 02/16] Clang doesn't like the C-style casts in nmmintrin.h --- include/rapidjson/reader.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/rapidjson/reader.h b/include/rapidjson/reader.h index 13fd126..19f8849 100644 --- a/include/rapidjson/reader.h +++ b/include/rapidjson/reader.h @@ -43,6 +43,7 @@ RAPIDJSON_DIAG_OFF(4702) // unreachable code #ifdef __clang__ RAPIDJSON_DIAG_PUSH +RAPIDJSON_DIAG_OFF(old-style-cast) RAPIDJSON_DIAG_OFF(padded) RAPIDJSON_DIAG_OFF(switch-enum) #endif From 13e3aa9b00eac83bac9f67b8adf86a9ae18cca27 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 11:24:38 -0400 Subject: [PATCH 03/16] we do need to avoid the double-promotion warning on clang, since we're compiling with -Werror --- test/unittest/CMakeLists.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/unittest/CMakeLists.txt b/test/unittest/CMakeLists.txt index 4e3b071..fae09cd 100644 --- a/test/unittest/CMakeLists.txt +++ b/test/unittest/CMakeLists.txt @@ -38,11 +38,11 @@ if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror -Wall -Wextra -Weffc++ -Wswitch-default -Wfloat-equal") elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror -Wall -Wextra -Weffc++ -Wswitch-default -Wfloat-equal -Wimplicit-fallthrough -Weverything") - # If the user is running a newer version of Clang that includes the - # -Wdouble-promotion, we will ignore that warning. - # if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.7) - # set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-double-promotion") - # endif() + # If the user is running a newer version of Clang that includes the + # -Wdouble-promotion, we will ignore that warning. + if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.7) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-double-promotion") + endif() elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") # Force to always compile with /W4 if(CMAKE_CXX_FLAGS MATCHES "/W[0-4]") From 035271091faa49734f40782b6cfad254b0302fa2 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 11:32:17 -0400 Subject: [PATCH 04/16] with recent clang, when expected is false, this code triggers -Wunreachable-code clang advises: "note: silence by adding parentheses to mark code as explicitly dead" --- test/unittest/schematest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unittest/schematest.cpp b/test/unittest/schematest.cpp index d1027ad..d75b1e5 100644 --- a/test/unittest/schematest.cpp +++ b/test/unittest/schematest.cpp @@ -111,7 +111,7 @@ TEST(SchemaValidator, Hasher) { EXPECT_FALSE(d.HasParseError());\ EXPECT_TRUE(expected == d.Accept(validator));\ EXPECT_TRUE(expected == validator.IsValid());\ - if (expected && !validator.IsValid()) {\ + if ((expected) && !validator.IsValid()) {\ StringBuffer sb;\ validator.GetInvalidSchemaPointer().StringifyUriFragment(sb);\ printf("Invalid schema: %s\n", sb.GetString());\ From 5c77c9248cd429dfd07db290fc1caeadd1b76dc5 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 11:37:39 -0400 Subject: [PATCH 05/16] with recent clang, this triggers -Wunevaluated-expression specifically, "expression with side effects has no effect in an unevaluated context" --- test/unittest/valuetest.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index feec049..619a6a5 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -1119,14 +1119,18 @@ TEST(Value, ArrayHelperRangeFor) { { int i = 0; - for (auto& v : x.GetArray()) - EXPECT_EQ(i++, v.GetInt()); + for (auto& v : x.GetArray()) { + EXPECT_EQ(i, v.GetInt()); + i++; + } EXPECT_EQ(i, 10); } { int i = 0; - for (const auto& v : const_cast(x).GetArray()) - EXPECT_EQ(i++, v.GetInt()); + for (const auto& v : const_cast(x).GetArray()) { + EXPECT_EQ(i, v.GetInt()); + i++; + } EXPECT_EQ(i, 10); } From fe550f38669fe0f488926c1ef0feb6c101f586d6 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 11:51:37 -0400 Subject: [PATCH 06/16] avoid array index out-of-bounds UBSAN gave "runtime error: index 13 out of bounds for type 'const uint32_t [10]'" --- include/rapidjson/internal/dtoa.h | 3 ++- test/unittest/dtoatest.cpp | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/rapidjson/internal/dtoa.h b/include/rapidjson/internal/dtoa.h index bc45496..8d6350e 100644 --- a/include/rapidjson/internal/dtoa.h +++ b/include/rapidjson/internal/dtoa.h @@ -102,7 +102,8 @@ inline void DigitGen(const DiyFp& W, const DiyFp& Mp, uint64_t delta, char* buff kappa--; if (p2 < delta) { *K += kappa; - GrisuRound(buffer, *len, delta, p2, one.f, wp_w.f * kPow10[-static_cast(kappa)]); + int index = -static_cast(kappa); + GrisuRound(buffer, *len, delta, p2, one.f, wp_w.f * (index < 9 ? kPow10[-static_cast(kappa)] : 0)); return; } } diff --git a/test/unittest/dtoatest.cpp b/test/unittest/dtoatest.cpp index fe28271..afd76eb 100644 --- a/test/unittest/dtoatest.cpp +++ b/test/unittest/dtoatest.cpp @@ -37,6 +37,7 @@ TEST(dtoa, normal) { TEST_DTOA(1.2345678, "1.2345678"); TEST_DTOA(0.123456789012, "0.123456789012"); TEST_DTOA(1234567.8, "1234567.8"); + TEST_DTOA(-79.39773355813419, "-79.39773355813419"); TEST_DTOA(0.000001, "0.000001"); TEST_DTOA(0.0000001, "1e-7"); TEST_DTOA(1e30, "1e30"); From 8074b722f0e13a3aad37460f40891660258efece Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 11:54:58 -0400 Subject: [PATCH 07/16] avoid reference to null pointer and member access within null pointer UBSAN gave issues with the typeless Schema: runtime error: reference binding to null pointer of type 'rapidjson::GenericSchemaDocument, rapidjson::MemoryPoolAllocator >, rapidjson::CrtAllocator>' and runtime error: member access within null pointer of type 'AllocatorType' (aka 'rapidjson::CrtAllocator') --- include/rapidjson/schema.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/include/rapidjson/schema.h b/include/rapidjson/schema.h index 0a8bb7c..80812f0 100644 --- a/include/rapidjson/schema.h +++ b/include/rapidjson/schema.h @@ -413,9 +413,11 @@ public: } } - AssignIfExist(allOf_, *schemaDocument, p, value, GetAllOfString(), document); - AssignIfExist(anyOf_, *schemaDocument, p, value, GetAnyOfString(), document); - AssignIfExist(oneOf_, *schemaDocument, p, value, GetOneOfString(), document); + if (schemaDocument) { + AssignIfExist(allOf_, *schemaDocument, p, value, GetAllOfString(), document); + AssignIfExist(anyOf_, *schemaDocument, p, value, GetAnyOfString(), document); + AssignIfExist(oneOf_, *schemaDocument, p, value, GetOneOfString(), document); + } if (const ValueType* v = GetMember(value, GetNotString())) { schemaDocument->CreateSchema(¬_, p.Append(GetNotString(), allocator_), *v, document); @@ -578,7 +580,9 @@ public: } ~Schema() { - allocator_->Free(enum_); + if (allocator_) { + allocator_->Free(enum_); + } if (properties_) { for (SizeType i = 0; i < propertyCount_; i++) properties_[i].~Property(); From 61637d338244f316c86e875b3175a362d01820e0 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 12:03:50 -0400 Subject: [PATCH 08/16] avoid passing a null pointer to memcpy UBSAN on Clang/Linux gave: runtime error: null pointer passed as argument 2, which is declared to never be null /usr/include/string.h:43:45: note: nonnull attribute specified here --- include/rapidjson/pointer.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/include/rapidjson/pointer.h b/include/rapidjson/pointer.h index c985277..0206ac1 100644 --- a/include/rapidjson/pointer.h +++ b/include/rapidjson/pointer.h @@ -767,8 +767,12 @@ private: tokenCount_ = rhs.tokenCount_ + extraToken; tokens_ = static_cast(allocator_->Malloc(tokenCount_ * sizeof(Token) + (nameBufferSize + extraNameBufferSize) * sizeof(Ch))); nameBuffer_ = reinterpret_cast(tokens_ + tokenCount_); - std::memcpy(tokens_, rhs.tokens_, rhs.tokenCount_ * sizeof(Token)); - std::memcpy(nameBuffer_, rhs.nameBuffer_, nameBufferSize * sizeof(Ch)); + if (rhs.tokenCount_ > 0) { + std::memcpy(tokens_, rhs.tokens_, rhs.tokenCount_ * sizeof(Token)); + } + if (nameBufferSize > 0) { + std::memcpy(nameBuffer_, rhs.nameBuffer_, nameBufferSize * sizeof(Ch)); + } // Adjust pointers to name buffer std::ptrdiff_t diff = nameBuffer_ - rhs.nameBuffer_; From be1eedf808c2c2520079f134114d6707ae5117b4 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 12:22:21 -0400 Subject: [PATCH 09/16] avoid signed-integer overflow, which is undefined behavior UBSAN gave for test/unittest/itoatest.cpp:87: runtime error: signed integer overflow: 4611686018427387904 * 2 cannot be represented in type 'long' --- test/unittest/itoatest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unittest/itoatest.cpp b/test/unittest/itoatest.cpp index 79db1c7..b752a6a 100644 --- a/test/unittest/itoatest.cpp +++ b/test/unittest/itoatest.cpp @@ -84,6 +84,8 @@ static void Verify(void(*f)(T, char*), char* (*g)(T, char*)) { VerifyValue(Traits::Negate(i + 1), f, g); } last = i; + if (i > static_cast(std::numeric_limits::max() / static_cast(power))) + break; i *= power; } while (last < i); } From 760ea4316c9596a08c7150b470a09853228abc33 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 12:26:31 -0400 Subject: [PATCH 10/16] avoid signed-integer underflow, which is undefined behavior maybe these tests should just be deleted? UBSAN gave: runtime error: signed integer overflow: -9223372036854775808 - 1 cannot be represented in type 'long' runtime error: signed integer overflow: -9223372036854775808 - 2 cannot be represented in type 'long' --- test/unittest/valuetest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index 619a6a5..430a828 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -545,8 +545,10 @@ TEST(Value, Int64) { // Templated functions EXPECT_TRUE(z.Is()); EXPECT_EQ(i, z.Get()); +#if 0 // signed integer underflow is undefined behaviour EXPECT_EQ(i - 1, z.Set(i - 1).Get()); EXPECT_EQ(i - 2, z.Set(i - 2).Get()); +#endif } TEST(Value, Uint64) { From df9b45a6568ee8002ad64cabcc5124192f38d749 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 12:29:09 -0400 Subject: [PATCH 11/16] avoid division by zero, which is undefined behavior UBSAN gave: runtime error: division by zero --- test/unittest/writertest.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/unittest/writertest.cpp b/test/unittest/writertest.cpp index 22f428e..29f7626 100644 --- a/test/unittest/writertest.cpp +++ b/test/unittest/writertest.cpp @@ -439,11 +439,9 @@ TEST(Writer, InvalidEventSequence) { } } -extern double zero; // clang -Wmissing-variable-declarations -double zero = 0.0; // Use global variable to prevent compiler warning - TEST(Writer, NaN) { - double nan = zero / zero; + double nan = std::numeric_limits::quiet_NaN(); + EXPECT_TRUE(internal::Double(nan).IsNan()); StringBuffer buffer; { @@ -461,7 +459,8 @@ TEST(Writer, NaN) { } TEST(Writer, Inf) { - double inf = 1.0 / zero; + double inf = std::numeric_limits::infinity(); + EXPECT_TRUE(internal::Double(inf).IsInf()); StringBuffer buffer; { From 9dcf51c3a1c8c08411c706b3936e6fba5d25e69d Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 12:39:09 -0400 Subject: [PATCH 12/16] avoid shift out-of-range error UBSAN gave during Reader.ParseNumber_FullPrecisionDouble test: include/rapidjson/internal/strtod.h:149:11: runtime error: shift exponent 46 is too large for 32-bit type 'int' --- include/rapidjson/internal/strtod.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rapidjson/internal/strtod.h b/include/rapidjson/internal/strtod.h index fd4b01e..289c413 100644 --- a/include/rapidjson/internal/strtod.h +++ b/include/rapidjson/internal/strtod.h @@ -142,7 +142,7 @@ inline bool StrtodDiyFp(const char* decimals, size_t length, size_t decimalPosit size_t remaining = length - i; const unsigned kUlpShift = 3; const unsigned kUlp = 1 << kUlpShift; - int error = (remaining == 0) ? 0 : kUlp / 2; + int64_t error = (remaining == 0) ? 0 : kUlp / 2; DiyFp v(significand, 0); v = v.Normalize(); From 05f0592b34cb943e7c96b8767d716431a3d9eaef Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 12:45:52 -0400 Subject: [PATCH 13/16] avoid shift out-of-range error UBSAN gave in Regex.Unicode test: include/rapidjson/encodings.h:157:28: runtime error: shift exponent 32 is too large for 32-bit type 'int' --- include/rapidjson/encodings.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/rapidjson/encodings.h b/include/rapidjson/encodings.h index edfc990..baa7c2b 100644 --- a/include/rapidjson/encodings.h +++ b/include/rapidjson/encodings.h @@ -154,7 +154,11 @@ struct UTF8 { } unsigned char type = GetRange(static_cast(c)); - *codepoint = (0xFF >> type) & static_cast(c); + if (type >= 32) { + *codepoint = 0; + } else { + *codepoint = (0xFF >> type) & static_cast(c); + } bool result = true; switch (type) { case 2: TAIL(); return result; From c52cec7e518feb30ec01bc0a978b620f8d9462ab Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 13:46:04 -0400 Subject: [PATCH 14/16] fix undefined double to uint64_t cast note that std::numeric_limits::max() and std::numeric_limits::max() aren't exactly representable in a double, so we need to be strictly less to be definitely lossless UBSAN gave during Value.IsLosslessDouble test: include/rapidjson/document.h:955:42: runtime error: value 1.84467e+19 is outside the range of representable values of type 'unsigned long' --- include/rapidjson/document.h | 9 +++++++-- test/unittest/valuetest.cpp | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index d286eb1..f1857f5 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -23,6 +23,7 @@ #include "memorystream.h" #include "encodedstream.h" #include // placement new +#include #ifdef _MSC_VER RAPIDJSON_DIAG_PUSH @@ -952,12 +953,16 @@ public: if (IsUint64()) { uint64_t u = GetUint64(); volatile double d = static_cast(u); - return static_cast(d) == u; + return (d >= 0.0) + && (d < static_cast(std::numeric_limits::max())) + && (u == static_cast(d)); } if (IsInt64()) { int64_t i = GetInt64(); volatile double d = static_cast(i); - return static_cast< int64_t>(d) == i; + return (d >= static_cast(std::numeric_limits::min())) + && (d < static_cast(std::numeric_limits::max())) + && (i == static_cast(d)); } return true; // double, int, uint are always lossless } diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index 430a828..fefc001 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -673,6 +673,7 @@ TEST(Value, Float) { } TEST(Value, IsLosslessDouble) { + EXPECT_TRUE(Value(0.0).IsLosslessDouble()); EXPECT_TRUE(Value(12.34).IsLosslessDouble()); EXPECT_TRUE(Value(-123).IsLosslessDouble()); EXPECT_TRUE(Value(2147483648u).IsLosslessDouble()); @@ -681,8 +682,19 @@ TEST(Value, IsLosslessDouble) { EXPECT_TRUE(Value(RAPIDJSON_UINT64_C2(0xA0000000, 0x00000000)).IsLosslessDouble()); #endif - EXPECT_FALSE(Value(-static_cast(RAPIDJSON_UINT64_C2(0x7FFFFFFF, 0xFFFFFFFF))).IsLosslessDouble()); - EXPECT_FALSE(Value(RAPIDJSON_UINT64_C2(0xFFFFFFFF, 0xFFFFFFFF)).IsLosslessDouble()); + EXPECT_FALSE(Value(static_cast(RAPIDJSON_UINT64_C2(0x7FFFFFFF, 0xFFFFFFFF))).IsLosslessDouble()); // INT64_MAX + EXPECT_FALSE(Value(-static_cast(RAPIDJSON_UINT64_C2(0x7FFFFFFF, 0xFFFFFFFF))).IsLosslessDouble()); // -INT64_MAX + EXPECT_TRUE(Value(-static_cast(RAPIDJSON_UINT64_C2(0x7FFFFFFF, 0xFFFFFFFF)) - 1).IsLosslessDouble()); // INT64_MIN + EXPECT_FALSE(Value(RAPIDJSON_UINT64_C2(0xFFFFFFFF, 0xFFFFFFFF)).IsLosslessDouble()); // UINT64_MAX + + EXPECT_TRUE(Value(3.4028234e38f).IsLosslessDouble()); // FLT_MAX + EXPECT_TRUE(Value(-3.4028234e38f).IsLosslessDouble()); // -FLT_MAX + EXPECT_TRUE(Value(1.17549435e-38f).IsLosslessDouble()); // FLT_MIN + EXPECT_TRUE(Value(-1.17549435e-38f).IsLosslessDouble()); // -FLT_MIN + EXPECT_TRUE(Value(1.7976931348623157e+308).IsLosslessDouble()); // DBL_MAX + EXPECT_TRUE(Value(-1.7976931348623157e+308).IsLosslessDouble()); // -DBL_MAX + EXPECT_TRUE(Value(2.2250738585072014e-308).IsLosslessDouble()); // DBL_MIN + EXPECT_TRUE(Value(-2.2250738585072014e-308).IsLosslessDouble()); // -DBL_MIN } TEST(Value, IsLosslessFloat) { From 21acc56d578821bdf2ae8e9ec06fabe18b2f12cc Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Tue, 31 May 2016 14:03:07 -0400 Subject: [PATCH 15/16] range check in IsLosslessFloat to avoid undefined double->float cast UBSAN gave in Value.IsLosslessFloat: include/rapidjson/document.h:981:38: runtime error: value 3.40282e+38 is outside the range of representable values of type 'float' --- include/rapidjson/document.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index f1857f5..b0162f5 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -978,6 +978,9 @@ public: bool IsLosslessFloat() const { if (!IsNumber()) return false; double a = GetDouble(); + if (a < static_cast(-std::numeric_limits::max()) + || a > static_cast(std::numeric_limits::max())) + return false; double b = static_cast(static_cast(a)); return a >= b && a <= b; // Prevent -Wfloat-equal } From 8c4059766e88626ad1213d40e05c308f62644d01 Mon Sep 17 00:00:00 2001 From: Eli Fidler Date: Mon, 13 Jun 2016 07:29:34 -0700 Subject: [PATCH 16/16] test for no-double-promotion instead of just checking compiler version --- test/unittest/CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/unittest/CMakeLists.txt b/test/unittest/CMakeLists.txt index fae09cd..b3204d6 100644 --- a/test/unittest/CMakeLists.txt +++ b/test/unittest/CMakeLists.txt @@ -1,3 +1,5 @@ +include(CheckCXXCompilerFlag) + set(UNITTEST_SOURCES allocatorstest.cpp bigintegertest.cpp @@ -41,7 +43,10 @@ elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang") # If the user is running a newer version of Clang that includes the # -Wdouble-promotion, we will ignore that warning. if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.7) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-double-promotion") + CHECK_CXX_COMPILER_FLAG("-Wno-double-promotion" HAS_NO_DOUBLE_PROMOTION) + if (HAS_NO_DOUBLE_PROMOTION) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-double-promotion") + endif() endif() elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") # Force to always compile with /W4