diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe57e7ce9..99caabf09 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -324,8 +324,8 @@ jobs: # Testing NVCC; forces sources to behave like .cu files cuda: runs-on: ubuntu-latest - name: "🐍 3.8 • CUDA 11.2 • Ubuntu 20.04" - container: nvidia/cuda:11.2.2-devel-ubuntu20.04 + name: "🐍 3.10 • CUDA 11.7 • Ubuntu 22.04" + container: nvidia/cuda:11.7.0-devel-ubuntu22.04 steps: - uses: actions/checkout@v3 diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index a0e32281b..b89e4946a 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1545,7 +1545,7 @@ private: throw cast_error_unable_to_convert_call_arg(a.name, a.type); #endif } - m_kwargs[a.name] = a.value; + m_kwargs[a.name] = std::move(a.value); } void process(list & /*args_list*/, detail::kwargs_proxy kp) { diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index c889dc416..3f6e27b75 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1578,6 +1578,22 @@ public: return *this; } +// Nvidia's NVCC is broken between 11.4.0 and 11.8.0 +// https://github.com/pybind/pybind11/issues/4193 +#if defined(__CUDACC__) && (__CUDACC_VER_MAJOR__ == 11) && (__CUDACC_VER_MINOR__ >= 4) \ + && (__CUDACC_VER_MINOR__ <= 8) + template + class_ &def(const T &op, const Extra &...extra) { + op.execute(*this, extra...); + return *this; + } + + template + class_ &def_cast(const T &op, const Extra &...extra) { + op.execute_cast(*this, extra...); + return *this; + } +#else template class_ &def(const detail::op_ &op, const Extra &...extra) { op.execute(*this, extra...); @@ -1589,6 +1605,7 @@ public: op.execute_cast(*this, extra...); return *this; } +#endif template class_ &def(const detail::initimpl::constructor &init, const Extra &...extra) { diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 565df4375..29506b7ea 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1829,18 +1829,18 @@ public: // guard if destructor called while err indicator is set error_scope error_guard; auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); - if (destructor == nullptr) { - if (PyErr_Occurred()) { - throw error_already_set(); - } - pybind11_fail("Unable to get capsule context"); + if (PyErr_Occurred()) { + throw error_already_set(); } const char *name = get_name_in_error_scope(o); void *ptr = PyCapsule_GetPointer(o, name); if (ptr == nullptr) { throw error_already_set(); } - destructor(ptr); + + if (destructor != nullptr) { + destructor(ptr); + } }); if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index 591dacc62..b0c7bdb48 100644 --- a/tests/test_eigen.cpp +++ b/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 &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 &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(); + 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: diff --git a/tests/test_eigen.py b/tests/test_eigen.py index 9c6485de3..713432a81 100644 --- a/tests/test_eigen.py +++ b/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(): diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index da0dc8f6b..c95ff8230 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -289,6 +289,12 @@ TEST_SUBMODULE(pytypes, m) { return capsule; }); + m.def("return_capsule_with_explicit_nullptr_dtor", []() { + py::print("creating capsule with explicit nullptr dtor"); + return py::capsule(reinterpret_cast(1234), + static_cast(nullptr)); // PR #4221 + }); + // test_accessors m.def("accessor_api", [](const py::object &o) { auto d = py::dict(); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index a34eaa59e..070849fc5 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -299,6 +299,17 @@ def test_capsule(capture): """ ) + with capture: + a = m.return_capsule_with_explicit_nullptr_dtor() + del a + pytest.gc_collect() + assert ( + capture.unordered + == """ + creating capsule with explicit nullptr dtor + """ + ) + def test_accessors(): class SubTestObject: