Merge pull request #60 from whimxiqal/bug/overlap-overflow

Guard against overflow for dynamic interval overlap

Author: 5cript

.github/workflows/build_and_test.yml

@@ -42,7 +42,7 @@ jobs:
         run: sudo apt-get update
 
       - name: Do install
-        run: sudo apt-get install -y ${{ matrix.compiler.pkg }} cmake ninja-build libgtest-dev libgmock-dev
+        run: sudo apt-get install -y ${{ matrix.compiler.pkg }} cmake ninja-build
 
       - name: Checkout
         uses: actions/checkout@v2
@@ -64,24 +64,15 @@ jobs:
         uses: actions/checkout@v2
 
       - name: Setup MSVC
-        uses: microsoft/setup-msbuild@v1.0.2
-
-      - name: Install vcpkg and gtest
-        run: |
-          git clone https://github.com/microsoft/vcpkg.git
-          .\vcpkg\bootstrap-vcpkg.bat
-          .\vcpkg\vcpkg.exe install gtest
-        shell: pwsh
-        env:
-          VCPKG_DEFAULT_TRIPLET: x64-windows
+        uses: ilammy/msvc-dev-cmd@v1
 
       - name: CMake Configure
-        run: cmake -B ${{github.workspace}}/build -G"Visual Studio 17 2022" -A x64 -DCMAKE_TOOLCHAIN_FILE=${{github.workspace}}/vcpkg/scripts/buildsystems/vcpkg.cmake -DCMAKE_CXX_STANDARD=20 -DINT_TREE_USE_OPTIONAL_POLYFILL=on -DINT_TREE_BUILD_EXAMPLES=on -DINT_TREE_ENABLE_TESTS=on -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}}
+        run: cmake -B ${{github.workspace}}/build -G"Ninja" -DCMAKE_CXX_STANDARD=20 -DINT_TREE_USE_OPTIONAL_POLYFILL=on -DINT_TREE_BUILD_EXAMPLES=on -DINT_TREE_ENABLE_TESTS=on -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}}
 
       - name: Build
         run: cmake --build ${{github.workspace}}/build --config ${{env.BUILD_TYPE}}
 
       - name: Test
         working-directory: ${{github.workspace}}/build
-        run: .\tests\Release\tree-tests.exe
-        shell: cmd
+        run: .\tests\tree-tests.exe
+        shell: cmd

README.md

@@ -56,9 +56,16 @@ int main()
 ```
 
 ## Compile & Run Testing
-Having googletest (find here on github) installed / built is a requirement to run the tests.
 Create a build folder, navigate there, run cmake and build the tree-tests target.
-You might have to adapt the linker line for gtest, if you built it yourself and didn't install it into your system.
+
+```bash
+mkdir build
+cd build
+cmake .. -DINT_TREE_ENABLE_TESTS=on
+cmake --build .
+./tests/tree-tests
+```
+
 If you want to generate the pretty drawings, install cairo, pull the submodule and pass INT_TREE_DRAW_EXAMPLES=on to the cmake command line to generate a drawings/make_drawings executeable.
 
 Some features of this library require the presence of an optional type.

include/interval-tree/interval_types.hpp

@@ -6,6 +6,7 @@
 #include <algorithm>
 #include <utility>
 #include <type_traits>
+#include <limits>
 
 namespace lib_interval_tree
 {
@@ -220,7 +221,8 @@ namespace lib_interval_tree
 #endif
         overlaps(numerical_type l1, numerical_type h1, numerical_type l2, numerical_type h2)
         {
-            return (l1 <= (h2 + 1)) && (l2 <= (h1 + 1));
+            return (h2 == std::numeric_limits<numerical_type>::max() || l1 <= (h2 + 1)) &&
+                (h1 == std::numeric_limits<numerical_type>::max() || l2 <= (h1 + 1));
         }
 
         template <typename numerical_type>
@@ -412,23 +414,25 @@ namespace lib_interval_tree
 
             auto closedOverlap =
                 closed::overlaps(closedEquiv1.low(), closedEquiv1.high(), closedEquiv2.low(), closedEquiv2.high());
-            if (!closedOverlap)
+            if (closedOverlap)
             {
-                if (closedEquiv1.high() + 1 == closedEquiv2.low() &&
-                    (ival1.right_border() == interval_border::closed_adjacent ||
-                     ival2.left_border() == interval_border::closed_adjacent))
-                {
-                    return true;
-                }
-                if (closedEquiv2.high() + 1 == closedEquiv1.low() &&
-                    (ival2.right_border() == interval_border::closed_adjacent ||
-                     ival1.left_border() == interval_border::closed_adjacent))
-                {
-                    return true;
-                }
-                return false;
+                return true;
             }
-            return true;
+            if (closedEquiv1.high() != std::numeric_limits<typename interval_type::value_type>::max() &&
+                closedEquiv1.high() + 1 == closedEquiv2.low() &&
+                (ival1.right_border() == interval_border::closed_adjacent ||
+                 ival2.left_border() == interval_border::closed_adjacent))
+            {
+                return true;
+            }
+            if (closedEquiv2.high() != std::numeric_limits<typename interval_type::value_type>::max() &&
+                closedEquiv2.high() + 1 == closedEquiv1.low() &&
+                (ival2.right_border() == interval_border::closed_adjacent ||
+                 ival1.left_border() == interval_border::closed_adjacent))
+            {
+                return true;
+            }
+            return false;
         }
 
         template <typename interval_type>

tests/CMakeLists.txt

@@ -19,7 +19,14 @@ file(GLOB sources "*.cpp")
 # Add Executable
 add_executable(tree-tests ${sources})
 
-find_package(GTest REQUIRED)
+include(FetchContent)
+FetchContent_Declare(
+    googletest
+    GIT_REPOSITORY https://github.com/google/googletest.git
+    GIT_TAG        v1.15.2
+)
+set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+FetchContent_MakeAvailable(googletest)
 
 target_link_libraries(tree-tests PRIVATE interval-tree GTest::gtest GTest::gmock GTest::gmock_main)
 
@@ -47,4 +54,9 @@ if (DEFINED ENV{MSYSTEM})
         COMMAND bash -c "ldd $<TARGET_FILE:tree-tests>" | "grep" "clang" | awk "NF == 4 { system(\"${CMAKE_COMMAND} -E copy \" \$3 \" $<TARGET_FILE_DIR:tree-tests>\") }"
         VERBATIM
     )
-endif()
+endif()
+
+if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
+    target_compile_options(tree-tests PRIVATE -fsanitize=signed-integer-overflow -fno-sanitize-recover=all)
+    target_link_options(tree-tests PRIVATE -fsanitize=signed-integer-overflow -fno-sanitize-recover=all)
+endif()

tests/interval_tests.hpp

@@ -640,6 +640,94 @@ TEST_F(OverlapTests, DynamicBorderTests)
     // one side open or closed follows from other tests
 }
 
+TEST_F(OverlapTests, DynamicBorderTestsLimits)
+{
+    const auto MIN = std::numeric_limits<int>::min();
+    const auto MAX = std::numeric_limits<int>::max();
+
+    constexpr auto c = interval_border::closed;
+    constexpr auto o = interval_border::open;
+    constexpr auto ca = interval_border::closed_adjacent;
+
+    // one min
+    EXPECT_TRUE(i<dynamic>(MIN, 5, c, c).overlaps({3, 16, c, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, o, c).overlaps({3, 16, c, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, ca, c).overlaps({3, 16, c, c}));
+
+    // one max
+    EXPECT_TRUE(i<dynamic>(0, 5, c, c).overlaps({3, MAX, c, c}));
+    EXPECT_TRUE(i<dynamic>(0, 5, c, c).overlaps({3, MAX, c, o}));
+    EXPECT_TRUE(i<dynamic>(0, 5, c, c).overlaps({3, MAX, c, ca}));
+
+    // both min
+    EXPECT_TRUE(i<dynamic>(MIN, 5, c, c).overlaps({MIN, 16, c, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, o, c).overlaps({MIN, 16, c, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, ca, c).overlaps({MIN, 16, c, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, c, c).overlaps({MIN, 16, o, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, o, c).overlaps({MIN, 16, o, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, ca, c).overlaps({MIN, 16, o, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, c, c).overlaps({MIN, 16, ca, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, o, c).overlaps({MIN, 16, ca, c}));
+    EXPECT_TRUE(i<dynamic>(MIN, 5, ca, c).overlaps({MIN, 16, ca, c}));
+
+    // both max
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, c).overlaps({3, MAX, c, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, o).overlaps({3, MAX, c, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, ca).overlaps({3, MAX, c, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, c).overlaps({3, MAX, c, o}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, c).overlaps({3, MAX, c, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, c).overlaps({3, MAX, c, ca}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, ca).overlaps({3, MAX, c, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, ca).overlaps({3, MAX, c, o}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, ca).overlaps({3, MAX, c, ca}));
+
+    // min-max overlap
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, c).overlaps({MIN, 3, c, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, o).overlaps({MIN, 3, c, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, ca).overlaps({MIN, 3, c, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, c).overlaps({MIN, 3, o, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, o).overlaps({MIN, 3, o, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, ca).overlaps({MIN, 3, o, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, c).overlaps({MIN, 3, ca, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, o).overlaps({MIN, 3, ca, c}));
+    EXPECT_TRUE(i<dynamic>(0, MAX, c, ca).overlaps({MIN, 3, ca, c}));
+
+    // min-max not overlap
+    EXPECT_FALSE(i<dynamic>(1, MAX, c, c).overlaps({MIN, 0, c, c}));
+    EXPECT_FALSE(i<dynamic>(1, MAX, c, o).overlaps({MIN, 0, c, c}));
+    EXPECT_FALSE(i<dynamic>(1, MAX, c, ca).overlaps({MIN, 0, c, c}));
+    EXPECT_FALSE(i<dynamic>(1, MAX, c, c).overlaps({MIN, 0, o, c}));
+    EXPECT_FALSE(i<dynamic>(1, MAX, c, o).overlaps({MIN, 0, o, c}));
+    EXPECT_FALSE(i<dynamic>(1, MAX, c, ca).overlaps({MIN, 0, o, c}));
+    EXPECT_FALSE(i<dynamic>(1, MAX, c, c).overlaps({MIN, 0, ca, c}));
+    EXPECT_FALSE(i<dynamic>(1, MAX, c, o).overlaps({MIN, 0, ca, c}));
+    EXPECT_FALSE(i<dynamic>(1, MAX, c, ca).overlaps({MIN, 0, ca, c}));
+
+    EXPECT_FALSE(i<dynamic>(MIN, -1, c, c).overlaps({MAX, MAX, c, c}));
+    EXPECT_FALSE(i<dynamic>(MIN, -1, c, ca).overlaps({MAX, MAX, c, c}));
+    EXPECT_FALSE(i<dynamic>(MIN, -1, c, c).overlaps({MAX, MAX, ca, c}));
+    EXPECT_FALSE(i<dynamic>(MAX, MAX, c, c).overlaps({MIN, -1, c, c}));
+    EXPECT_FALSE(i<dynamic>(MAX, MAX, ca, c).overlaps({MIN, -1, c, c}));
+    EXPECT_FALSE(i<dynamic>(MAX, MAX, c, c).overlaps({MIN, -1, c, ca}));
+}
+
+TEST_F(OverlapTests, ClosedAdjacentOverlapLimits)
+{
+    const auto MIN = std::numeric_limits<int>::min();
+    const auto MAX = std::numeric_limits<int>::max();
+
+    EXPECT_TRUE(i<closed_adjacent>(0, 5).overlaps({3, MAX}));
+    EXPECT_TRUE(i<closed_adjacent>(0, MAX).overlaps({3, 16}));
+    EXPECT_TRUE(i<closed_adjacent>(0, MAX).overlaps({3, MAX}));
+
+    EXPECT_TRUE(i<closed_adjacent>(0, MAX - 1).overlaps({MAX, MAX}));
+    EXPECT_TRUE(i<closed_adjacent>(MAX, MAX).overlaps({0, MAX - 1}));
+
+    EXPECT_FALSE(i<closed_adjacent>(0, 5).overlaps({MAX, MAX}));
+    EXPECT_FALSE(i<closed_adjacent>(MIN, -1).overlaps({MAX, MAX}));
+    EXPECT_FALSE(i<closed_adjacent>(MAX, MAX).overlaps({MIN, -1}));
+}
+
 TEST_F(IntervalTests, DynamicWithinTest)
 {
     auto base = i<dynamic>(-100, 100, interval_border::closed, interval_border::closed);