Merge pull request #168 from tcbrindle/pr/cartesian_size_fix

Avoid overflow in cartesian_product size() impl
tcbrindle · Feb 3, 2024 · 5900bb8 · 5900bb8
2 parents 85b016e + b4c244d
commit 5900bb8
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 26 deletions.
diff --git a/include/flux/op/cartesian_base.hpp b/include/flux/op/cartesian_base.hpp
@@ -221,8 +221,10 @@ struct cartesian_traits_base_impl {
         requires (CartesianKind == cartesian_kind::product
                   && (sized_sequence<Bases> && ...))
     {
-        return std::apply([](auto&... base) {
-            return (flux::size(base) * ...);
+        return std::apply([](auto& base0, auto&... bases) {
+            distance_t sz = flux::size(base0);
+            ((sz = num::checked_mul(sz, flux::size(bases))), ...);
+            return sz;
         }, self.bases_);
     }
 

diff --git a/test/test_cartesian_product.cpp b/test/test_cartesian_product.cpp
@@ -498,9 +498,22 @@ constexpr bool test_cartesian_product()
 }
 static_assert(test_cartesian_product());
 
+// https://github.com/tcbrindle/flux/issues/167
+void issue_167()
+{
+    // Check that overflowing size() is correctly caught
+    auto ints = flux::ints(0, std::numeric_limits<flux::distance_t>::max());
+
+    auto prod = flux::cartesian_product(ints, ints, ints);
+
+    REQUIRE_THROWS_AS(flux::size(prod), flux::unrecoverable_error);
+}
+
 }
 
 TEST_CASE("cartesian_product")
 {
     REQUIRE(test_cartesian_product());
+
+    issue_167();
 }
diff --git a/test/test_cartesian_product_map.cpp b/test/test_cartesian_product_map.cpp
@@ -15,7 +15,8 @@ constexpr bool test_cartesian_product_map()
         std::array arr1{100, 200};
         std::array arr2{1, 2, 3, 4, 5};
 
-        auto cart = flux::cartesian_product_map(sum, flux::ref(arr1), flux::ref(arr2));
+        auto cart =
+            flux::cartesian_product_map(sum, flux::ref(arr1), flux::ref(arr2));
         using C = decltype(cart);
 
         static_assert(flux::sequence<C>);
@@ -27,14 +28,12 @@ constexpr bool test_cartesian_product_map()
 
         STATIC_CHECK(flux::size(cart) == 2 * 5);
 
-        STATIC_CHECK(check_equal(cart, { 101, 102, 103, 104, 105,
-                                         201, 202, 203, 204, 205
-                                       }));
+        STATIC_CHECK(check_equal(
+            cart, {101, 102, 103, 104, 105, 201, 202, 203, 204, 205}));
 
-        STATIC_CHECK(check_equal(flux::reverse(flux::ref(cart)),
-                                 { 205, 204, 203, 202, 201,
-                                   105, 104, 103, 102, 101
-                                 }));
+        STATIC_CHECK(
+            check_equal(flux::reverse(flux::ref(cart)),
+                        {205, 204, 203, 202, 201, 105, 104, 103, 102, 101}));
 
         // Random access checks
         STATIC_CHECK(flux::distance(cart, cart.first(), cart.last()) == 2 * 5);
@@ -52,7 +51,8 @@ constexpr bool test_cartesian_product_map()
         std::array arr2{10, 20, 30};
         std::array arr3{1, 2, 3, 4};
 
-        auto cart = flux::cartesian_product_map(sum, flux::ref(arr1), flux::ref(arr2), flux::ref(arr3));
+        auto cart = flux::cartesian_product_map(
+            sum, flux::ref(arr1), flux::ref(arr2), flux::ref(arr3));
         using C = decltype(cart);
 
         static_assert(flux::sequence<C>);
@@ -64,13 +64,10 @@ constexpr bool test_cartesian_product_map()
 
         STATIC_CHECK(flux::size(cart) == 2 * 3 * 4);
 
-        STATIC_CHECK(check_equal(cart, { 111, 112, 113, 114,
-                                         121, 122, 123, 124,
-                                         131, 132, 133, 134,
-                                         211, 212, 213, 214,
-                                         221, 222, 223, 224,
-                                         231, 232, 233, 234 }
-        ));
+        STATIC_CHECK(
+            check_equal(cart, {111, 112, 113, 114, 121, 122, 123, 124,
+                               131, 132, 133, 134, 211, 212, 213, 214,
+                               221, 222, 223, 224, 231, 232, 233, 234}));
 
         {
             auto cur = flux::next(cart, cart.first(), 7);
@@ -84,7 +81,8 @@ constexpr bool test_cartesian_product_map()
 
     {
         auto seq0 = single_pass_only(flux::from(std::array{100, 200}));
-        auto cart = flux::cartesian_product_map(sum, std::move(seq0), std::array{1, 2, 3});
+        auto cart = flux::cartesian_product_map(sum, std::move(seq0),
+                                                std::array{1, 2, 3});
         using C = decltype(cart);
 
         static_assert(flux::sequence<C>);
@@ -100,7 +98,8 @@ constexpr bool test_cartesian_product_map()
         auto arr = std::array{1, 2, 3, 4, 5};
         auto emp = flux::empty<int>;
 
-        auto cart = flux::cartesian_product_map(sum, flux::ref(arr), std::move(emp));
+        auto cart =
+            flux::cartesian_product_map(sum, flux::ref(arr), std::move(emp));
 
         static_assert(flux::bidirectional_sequence<decltype(cart)>);
 
@@ -116,26 +115,38 @@ constexpr bool test_cartesian_product_map()
         double vals[3][3] = {};
         auto get = [&vals](auto i, auto j) -> double& { return vals[i][j]; };
 
-        auto seq = flux::cartesian_product_map(get, flux::iota(0, 3), flux::iota(0, 3));
+        auto seq = flux::cartesian_product_map(get, flux::iota(0, 3),
+                                               flux::iota(0, 3));
 
         static_assert(std::same_as<flux::element_t<decltype(seq)>, double&>);
 
         seq.fill(100.0);
 
         for (int i = 0; i < 3; i++) {
-            for (int j = 0; j < 3; j++) {
-                STATIC_CHECK(vals[i][j] == 100.0);
-            }
+            for (int j = 0; j < 3; j++) { STATIC_CHECK(vals[i][j] == 100.0); }
         }
     }
 
     return true;
 }
 static_assert(test_cartesian_product_map());
 
+// https://github.com/tcbrindle/flux/issues/167
+void issue_167()
+{
+    // Check that overflowing size() is correctly caught
+    auto ints = flux::ints(0, std::numeric_limits<flux::distance_t>::max());
+
+    auto prod = flux::cartesian_product_map([](auto...) { return 0; }, ints, ints, ints);
+
+    REQUIRE_THROWS_AS(flux::size(prod), flux::unrecoverable_error);
+}
+
+}
+
 TEST_CASE("cartesian_product_map")
 {
     REQUIRE(test_cartesian_product_map());
-}
 
-}
+    issue_167();
+}