Allow infinity and NaN to be parsed. (#26)

* Allow infinity and NaN to be parsed. * Use libc++ (instead of libstdc++) when using clang * Exclude an ASAN option due to a bug.
DataDog · Mar 31, 2023 · 7686f1b · 7686f1b
1 parent 66be1fb
commit 7686f1b
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 26 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -42,6 +42,8 @@ jobs:
     resource_class: xlarge
     environment:
       MAKE_JOB_COUNT: 8
+      # See <https://github.com/llvm/llvm-project/issues/59432>.
+      ASAN_OPTIONS: alloc_dealloc_mismatch=0
     steps:
     - checkout
     - run: mkdir .build

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -17,6 +17,12 @@ set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 
+# If we're building with clang, then use the libc++ version of the standard
+# library instead of libstdc++. Better coverage of build configurations.
+if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libc++")
+endif()
+
 include(ProcessorCount)
 ProcessorCount(NUM_PROCESSORS)
 set(MAKE_JOB_COUNT ${NUM_PROCESSORS} CACHE STRING "Number of jobs to use when building libcurl")

diff --git a/Dockerfile b/Dockerfile
@@ -10,11 +10,13 @@ env DEBIAN_FRONTEND=noninteractive
 # - Make available more recent versions of git.
 # - Update the package lists and upgrade already-installed software.
 # - Install build tooling:
-#   - GCC, clang, make, git, debugger, formatter, and miscellanea
+#   - GCC, clang, libc++, make, git, debugger, formatter, and miscellanea
 run apt-get update && apt-get install --yes software-properties-common && \
     add-apt-repository ppa:git-core/ppa --yes && \
     apt-get update && apt-get upgrade --yes && \
-    apt-get install --yes wget build-essential clang sed gdb clang-format git ssh shellcheck
+    apt-get install --yes \
+        wget build-essential clang sed gdb clang-format git ssh shellcheck \
+        libc++-dev libc++abi-dev
 
 # bazelisk, a launcher for bazel. `bazelisk --help` will cause the latest
 # version to be downloaded.

diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp
@@ -585,28 +585,35 @@ TEST_CASE("TracerConfig::trace_sampler") {
       struct TestCase {
         std::string name;
         std::string env_value;
-        Error::Code expected_error;
+        std::vector<Error::Code> allowed_errors;
       };
 
       auto test_case = GENERATE(values<TestCase>({
-          {"empty", "", Error::INVALID_DOUBLE},
-          {"nonsense", "nonsense", Error::INVALID_DOUBLE},
-          {"trailing space", "0.23   ", Error::INVALID_DOUBLE},
-          {"out of range of double", "123e9999999999", Error::INVALID_DOUBLE},
-          {"NaN", "NaN", Error::INVALID_DOUBLE},
-          {"nan", "nan", Error::INVALID_DOUBLE},
-          {"inf", "inf", Error::INVALID_DOUBLE},
-          {"Inf", "Inf", Error::INVALID_DOUBLE},
-          {"below range", "-0.1", Error::RATE_OUT_OF_RANGE},
-          {"above range", "1.1", Error::RATE_OUT_OF_RANGE},
+          {"empty", "", {Error::INVALID_DOUBLE}},
+          {"nonsense", "nonsense", {Error::INVALID_DOUBLE}},
+          {"trailing space", "0.23   ", {Error::INVALID_DOUBLE}},
+          {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}},
+          // Some C++ standard libraries parse "nan" and "inf" as the
+          // corresponding special floating point values. Other standard
+          // libraries consider "nan" and "inf" invalid.
+          // So, either the double will fail to parse, or parsing will succeed
+          // but the resulting value will be outside of the inclusive range
+          // [0.0, 1.0] of the `Rate` type.
+          {"NaN", "NaN", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}},
+          {"nan", "nan", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}},
+          {"inf", "inf", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}},
+          {"Inf", "Inf", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}},
+          {"below range", "-0.1", {Error::RATE_OUT_OF_RANGE}},
+          {"above range", "1.1", {Error::RATE_OUT_OF_RANGE}},
       }));
 
       CAPTURE(test_case.name);
 
       const EnvGuard guard{"DD_TRACE_SAMPLE_RATE", test_case.env_value};
       auto finalized = finalize_config(config);
       REQUIRE(!finalized);
-      REQUIRE(finalized.error().code == test_case.expected_error);
+      REQUIRE_THAT(test_case.allowed_errors,
+                   Catch::Matchers::VectorContains(finalized.error().code));
     }
   }
 
@@ -644,28 +651,45 @@ TEST_CASE("TracerConfig::trace_sampler") {
       struct TestCase {
         std::string name;
         std::string env_value;
-        Error::Code expected_error;
+        std::vector<Error::Code> allowed_errors;
       };
 
       auto test_case = GENERATE(values<TestCase>({
-          {"empty", "", Error::INVALID_DOUBLE},
-          {"nonsense", "nonsense", Error::INVALID_DOUBLE},
-          {"trailing space", "23   ", Error::INVALID_DOUBLE},
-          {"out of range of double", "123e9999999999", Error::INVALID_DOUBLE},
-          {"NaN", "NaN", Error::INVALID_DOUBLE},
-          {"nan", "nan", Error::INVALID_DOUBLE},
-          {"inf", "inf", Error::INVALID_DOUBLE},
-          {"Inf", "Inf", Error::INVALID_DOUBLE},
-          {"below range", "-0.1", Error::MAX_PER_SECOND_OUT_OF_RANGE},
-          {"zero (also below range)", "0", Error::MAX_PER_SECOND_OUT_OF_RANGE},
+          {"empty", "", {Error::INVALID_DOUBLE}},
+          {"nonsense", "nonsense", {Error::INVALID_DOUBLE}},
+          {"trailing space", "23   ", {Error::INVALID_DOUBLE}},
+          {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}},
+          // Some C++ standard libraries parse "nan" and "inf" as the
+          // corresponding special floating point values. Other standard
+          // libraries consider "nan" and "inf" invalid.
+          // So, either the double will fail to parse, or parsing will succeed
+          // but the resulting value will be outside of the exclusive range
+          // (0.0, Inf) allowed.
+          {"NaN",
+           "NaN",
+           {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
+          {"nan",
+           "nan",
+           {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
+          {"inf",
+           "inf",
+           {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
+          {"Inf",
+           "Inf",
+           {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
+          {"below range", "-0.1", {Error::MAX_PER_SECOND_OUT_OF_RANGE}},
+          {"zero (also below range)",
+           "0",
+           {Error::MAX_PER_SECOND_OUT_OF_RANGE}},
       }));
 
       CAPTURE(test_case.name);
 
       const EnvGuard guard{"DD_TRACE_RATE_LIMIT", test_case.env_value};
       auto finalized = finalize_config(config);
       REQUIRE(!finalized);
-      REQUIRE(finalized.error().code == test_case.expected_error);
+      REQUIRE_THAT(test_case.allowed_errors,
+                   Catch::Matchers::VectorContains(finalized.error().code));
     }
   }