test_eigen.py test_nonunit_stride_to_python bug fix (ASAN failure) (#…

…4217)

* Disable test triggering ASAN failure (to pin-point where the problem is).

* Fix unsafe "block" implementation in test_eigen.cpp

* Undo changes (i.e. revert back to master).

* Detect "type_caster for Eigen::Ref made a copy."

This is achieved without
* reaching into internals,
* making test_eigen.cpp depend on pybind11/numpy.h.

* Add comment pointing to PR, for easy reference.

tests/test_eigen.cpp

@@ -197,11 +197,40 @@ TEST_SUBMODULE(eigen, m) {
 
     // Return a block of a matrix (gives non-standard strides)
     m.def("block",
-          [](const Eigen::Ref<const Eigen::MatrixXd> &x,
-             int start_row,
-             int start_col,
-             int block_rows,
-             int block_cols) { return x.block(start_row, start_col, block_rows, block_cols); });
+          [m](const py::object &x_obj,
+              int start_row,
+              int start_col,
+              int block_rows,
+              int block_cols) {
+              return m.attr("_block")(x_obj, x_obj, start_row, start_col, block_rows, block_cols);
+          });
+
+    m.def(
+        "_block",
+        [](const py::object &x_obj,
+           const Eigen::Ref<const Eigen::MatrixXd> &x,
+           int start_row,
+           int start_col,
+           int block_rows,
+           int block_cols) {
+            // See PR #4217 for background. This test is a bit over the top, but might be useful
+            // as a concrete example to point to when explaining the dangling reference trap.
+            auto i0 = py::make_tuple(0, 0);
+            auto x0_orig = x_obj[*i0].cast<double>();
+            if (x(0, 0) != x0_orig) {
+                throw std::runtime_error(
+                    "Something in the type_caster for Eigen::Ref is terribly wrong.");
+            }
+            double x0_mod = x0_orig + 1;
+            x_obj[*i0] = x0_mod;
+            auto copy_detected = (x(0, 0) != x0_mod);
+            x_obj[*i0] = x0_orig;
+            if (copy_detected) {
+                throw std::runtime_error("type_caster for Eigen::Ref made a copy.");
+            }
+            return x.block(start_row, start_col, block_rows, block_cols);
+        },
+        py::keep_alive<0, 1>());
 
     // test_eigen_return_references, test_eigen_keepalive
     // return value referencing/copying tests:

tests/test_eigen.py

@@ -217,15 +217,24 @@ def test_negative_stride_from_python(msg):
     )
 
 
+def test_block_runtime_error_type_caster_eigen_ref_made_a_copy():
+    with pytest.raises(RuntimeError) as excinfo:
+        m.block(ref, 0, 0, 0, 0)
+    assert str(excinfo.value) == "type_caster for Eigen::Ref made a copy."
+
+
 def test_nonunit_stride_to_python():
     assert np.all(m.diagonal(ref) == ref.diagonal())
     assert np.all(m.diagonal_1(ref) == ref.diagonal(1))
     for i in range(-5, 7):
         assert np.all(m.diagonal_n(ref, i) == ref.diagonal(i)), f"m.diagonal_n({i})"
 
-    assert np.all(m.block(ref, 2, 1, 3, 3) == ref[2:5, 1:4])
-    assert np.all(m.block(ref, 1, 4, 4, 2) == ref[1:, 4:])
-    assert np.all(m.block(ref, 1, 4, 3, 2) == ref[1:4, 4:])
+    # Must be order="F", otherwise the type_caster will make a copy and
+    # m.block() will return a dangling reference (heap-use-after-free).
+    rof = np.asarray(ref, order="F")
+    assert np.all(m.block(rof, 2, 1, 3, 3) == rof[2:5, 1:4])
+    assert np.all(m.block(rof, 1, 4, 4, 2) == rof[1:, 4:])
+    assert np.all(m.block(rof, 1, 4, 3, 2) == rof[1:4, 4:])
 
 
 def test_eigen_ref_to_python():