[BUG]: Coverity scan issue: Possible dereferencing iterator pos though it is already past the end of its container.

[oleksandr-pavlyk]

Required prerequisites

• Make sure you've read the documentation. Your issue may be addressed there.
• Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
• Consider asking first in the Gitter chat room or in a Discussion.

What version (or hash if on master) of pybind11 are you using?

2.10.2

Problem description

Scan highlights possible issue in function clear_patients in "class.h" file:

I assume the tools expects explicit check before dereferencing:

if (pos != internals.patients.end()) {
    // Clearing the patients can cause more Python code to run, which
    // can invalidate the iterator. Extract the vector of patients
    // from the unordered_map first.
    auto patients = std::move(pos->second);
    internals.patients.erase(pos);
    instance->has_patients = false;
    for (PyObject *&patient : patients) {
       Py_CLEAR(patient);
    }
} else {
    assert(pos != internals.patients.end());
}

If such a change is agreeable, I'd be happy to open a PR.

Feel free to resolve this issue if you are against this change and believe this issue is a false positive in the Coverity scan.

Reproducible example code

No response

Is this a regression? Put the last known working version here if it is.

Not a regression

[rwgk]

Did you try this with -UNDEBUG already?

[oleksandr-pavlyk]

Yes, doing -UNDEBUG does help.

[cliffburdick]

This appears to be happening with g++13 (not just coverity) now where it wasn't happening with older compilers.

In member function 'std::__detail::_Hash_code_base<_Key, _Value, _ExtractKey, _Hash, _RangeHash, _Unused, __cache_hash_code>::__hash_code std::__detail::_Hash_code_base<_Key, _Value, _ExtractKey, _Hash, _RangeHash, _Unused, __cache_hash_code>::_M_hash_code(const _Key&) const [with _Key = const _object*; _Value = std::pair<const _object* const, std::vector<_object*> >; _ExtractKey = std::__detail::_Select1st; _Hash = std::hash<const _object*>; _RangeHash = std::__detail::_Mod_range_hashing; _Unused = std::__detail::_Default_ranged_hash; bool __cache_hash_code = false]',
    inlined from 'std::size_t std::__detail::_Hash_code_base<_Key, _Value, _ExtractKey, _Hash, _RangeHash, _Unused, __cache_hash_code>::_M_bucket_index(const std::__detail::_Hash_node_value<_Value, false>&, std::size_t) const [with _Key = const _object*; _Value = std::pair<const _object* const, std::vector<_object*> >; _ExtractKey = std::__detail::_Select1st; _Hash = std::hash<const _object*>; _RangeHash = std::__detail::_Mod_range_hashing; _Unused = std::__detail::_Default_ranged_hash; bool __cache_hash_code = false]' at /usr/include/c++/13/bits/hashtable_policy.h:1341:20,
    inlined from 'std::_Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>::size_type std::_Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>::_M_bucket_index(const __node_value_type&) const [with _Key = const _object*; _Value = std::pair<const _object* const, std::vector<_object*> >; _Alloc = std::allocator<std::pair<const _object* const, std::vector<_object*> > >; _ExtractKey = std::__detail::_Select1st; _Equal = std::equal_to<const _object*>; _Hash = std::hash<const _object*>; _RangeHash = std::__detail::_Mod_range_hashing; _Unused = std::__detail::_Default_ranged_hash; _RehashPolicy = std::__detail::_Prime_rehash_policy; _Traits = std::__detail::_Hashtable_traits<false, false, true>]' at /usr/include/c++/13/bits/hashtable.h:793:43,
    inlined from 'std::_Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>::iterator std::_Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>::erase(const_iterator) [with _Key = const _object*; _Value = std::pair<const _object* const, std::vector<_object*> >; _Alloc = std::allocator<std::pair<const _object* const, std::vector<_object*> > >; _ExtractKey = std::__detail::_Select1st; _Equal = std::equal_to<const _object*>; _Hash = std::hash<const _object*>; _RangeHash = std::__detail::_Mod_range_hashing; _Unused = std::__detail::_Default_ranged_hash; _RehashPolicy = std::__detail::_Prime_rehash_policy; _Traits = std::__detail::_Hashtable_traits<false, false, true>]' at /usr/include/c++/13/bits/hashtable.h:2322:36,
    inlined from 'std::_Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>::iterator std::_Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>::erase(iterator) [with _Key = const _object*; _Value = std::pair<const _object* const, std::vector<_object*> >; _Alloc = std::allocator<std::pair<const _object* const, std::vector<_object*> > >; _ExtractKey = std::__detail::_Select1st; _Equal = std::equal_to<const _object*>; _Hash = std::hash<const _object*>; _RangeHash = std::__detail::_Mod_range_hashing; _Unused = std::__detail::_Default_ranged_hash; _RehashPolicy = std::__detail::_Prime_rehash_policy; _Traits = std::__detail::_Hashtable_traits<false, false, true>]' at /usr/include/c++/13/bits/hashtable.h:980:15,
    inlined from 'std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::iterator std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::erase(iterator) [with _Key = const _object*; _Tp = std::vector<_object*>; _Hash = std::hash<const _object*>; _Pred = std::equal_to<const _object*>; _Alloc = std::allocator<std::pair<const _object* const, std::vector<_object*> > >]' at /usr/include/c++/13/bits/unordered_map.h:753:22,
    inlined from 'void pybind11::detail::clear_patients(PyObject*)' at /pybind11-src/include/pybind11/detail/class.h:396:27,
    inlined from 'void pybind11::detail::clear_instance(PyObject*)' at /pybind11-src/include/pybind11/detail/class.h:438:15:
/usr/include/c++/13/bits/hashtable_policy.h:1310:24: error: potential null pointer dereference [-Werror=null-dereference]
 1310 |         return _M_hash()(__k);
      |               ~~~~~~~~~^~~~~
In constructor 'std::_Vector_base<_Tp, _Alloc>::_Vector_impl_data::_Vector_impl_data(std::_Vector_base<_Tp, _Alloc>::_Vector_impl_data&&) [with _Tp = _object*; _Alloc = std::allocator<_object*>]',
    inlined from 'std::_Vector_base<_Tp, _Alloc>::_Vector_impl::_Vector_impl(std::_Vector_base<_Tp, _Alloc>::_Vector_impl&&) [with _Tp = _object*; _Alloc = std::allocator<_object*>]' at /usr/include/c++/13/bits/stl_vector.h:154:167,
    inlined from 'std::_Vector_base<_Tp, _Alloc>::_Vector_base(std::_Vector_base<_Tp, _Alloc>&&) [with _Tp = _object*; _Alloc = std::allocator<_object*>]' at /usr/include/c++/13/bits/stl_vector.h:338:1,
    inlined from 'std::vector<_Tp, _Alloc>::vector(std::vector<_Tp, _Alloc>&&) [with _Tp = _object*; _Alloc = std::allocator<_object*>]' at /usr/include/c++/13/bits/stl_vector.h:620:1,
    inlined from 'void pybind11::detail::clear_patients(PyObject*)' at /pybind11-src/include/pybind11/detail/class.h:395:38,
    inlined from 'void pybind11::detail::clear_instance(PyObject*)' at /pybind11-src/include/pybind11/detail/class.h:438:15: