Fix null pointer dereference in attr_with_type_hint (#5576)

* Fix regression

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* style: pre-commit fixes

* Request changes and fix for func_rec nullptr

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* style: pre-commit fixes

* Fix function_record call

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* style: pre-commit fixes

* try typedef forward declaration

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* refactor

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* style: pre-commit fixes

* remove from .py file

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* address feedback

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* style: pre-commit fixes

* Fix `e.j.` → `e.g.` typo.

---------

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>

include/pybind11/cast.h

@@ -1606,15 +1606,9 @@ inline void object::cast() && {
 
 PYBIND11_NAMESPACE_BEGIN(detail)
 
-// forward declaration (definition in attr.h)
-struct function_record;
-
 // forward declaration (definition in pybind11.h)
-std::string generate_function_signature(const char *type_caster_name_field,
+template <typename T>
-                                        function_record *func_rec,
+std::string generate_type_signature();
-                                        const std::type_info *const *types,
-                                        size_t &type_index,
-                                        size_t &arg_index);
 
 // Declared in pytypes.h:
 template <typename T, enable_if_t<!is_pyobject<T>::value, int>>
@@ -1637,10 +1631,7 @@ str_attr_accessor object_api<D>::attr_with_type_hint(const char *key) const {
         throw std::runtime_error("__annotations__[\"" + std::string(key) + "\"] was set already.");
     }
 
-    const char *text = make_caster<T>::name.text;
+    ann[key] = generate_type_signature<T>();
-
-    size_t unused = 0;
-    ann[key] = generate_function_signature(text, nullptr, nullptr, unused, unused);
     return {derived(), key};
 }
 

include/pybind11/pybind11.h

@@ -112,7 +112,6 @@ inline std::string generate_function_signature(const char *type_caster_name_fiel
                                                size_t &arg_index) {
     std::string signature;
     bool is_starred = false;
-    bool is_annotation = func_rec == nullptr;
     // `is_return_value.top()` is true if we are currently inside the return type of the
     // signature. Using `@^`/`@$` we can force types to be arg/return types while `@!` pops
     // back to the previous state.
@@ -199,9 +198,7 @@ inline std::string generate_function_signature(const char *type_caster_name_fiel
             // For named arguments (py::arg()) with noconvert set, return value type is used.
             ++pc;
             if (!is_return_value.top()
-                && (is_annotation
+                && (!(arg_index < func_rec->args.size() && !func_rec->args[arg_index].convert))) {
-                    || !(arg_index < func_rec->args.size()
-                         && !func_rec->args[arg_index].convert))) {
                 while (*pc != '\0' && *pc != '@') {
                     signature += *pc++;
                 }
@@ -232,6 +229,19 @@ inline std::string generate_function_signature(const char *type_caster_name_fiel
     return signature;
 }
 
+template <typename T>
+inline std::string generate_type_signature() {
+    static constexpr auto caster_name_field = make_caster<T>::name;
+    PYBIND11_DESCR_CONSTEXPR auto descr_types = decltype(caster_name_field)::types();
+    // Create a default function_record to ensure the function signature has the proper
+    // configuration e.g. no_convert.
+    auto func_rec = function_record();
+    size_t type_index = 0;
+    size_t arg_index = 0;
+    return generate_function_signature(
+        caster_name_field.text, &func_rec, descr_types.data(), type_index, arg_index);
+}
+
 #if defined(_MSC_VER)
 #    define PYBIND11_COMPAT_STRDUP _strdup
 #else

tests/test_pytypes.cpp

@@ -1058,6 +1058,21 @@ TEST_SUBMODULE(pytypes, m) {
     // Exercises py::handle overload:
     m.attr_with_type_hint<py::typing::Set<py::str>>(py::str("set_str")) = py::set();
 
+    struct foo_t {};
+    struct foo2 {};
+    struct foo3 {};
+
+    pybind11::class_<foo_t>(m, "foo");
+    pybind11::class_<foo2>(m, "foo2");
+    pybind11::class_<foo3>(m, "foo3");
+    m.attr_with_type_hint<foo_t>("foo") = foo_t{};
+
+    m.attr_with_type_hint<py::typing::Union<foo_t, foo2, foo3>>("foo_union") = foo_t{};
+
+    // Include to ensure this does not crash
+    struct foo4 {};
+    m.attr_with_type_hint<foo4>("foo4") = 3;
+
     struct Empty {};
     py::class_<Empty>(m, "EmptyAnnotationClass");
 

tests/test_pytypes.py

@@ -1157,6 +1157,11 @@ def test_module_attribute_types() -> None:
 
     assert module_annotations["list_int"] == "list[typing.SupportsInt]"
     assert module_annotations["set_str"] == "set[str]"
+    assert module_annotations["foo"] == "pybind11_tests.pytypes.foo"
+    assert (
+        module_annotations["foo_union"]
+        == "Union[pybind11_tests.pytypes.foo, pybind11_tests.pytypes.foo2, pybind11_tests.pytypes.foo3]"
+    )
 
 
 @pytest.mark.skipif(