Skip to content

Commit 87380b5

Browse files
[cppyy] Fix hash collision in TemplateProxy dispatch for CPPOverload objects
Distinct CPPOverload objects (e.g:- different global functions) previously generated identical hash signatures because HashSignature relied solely on Py_TYPE and Py_REFCNT. This caused the TemplateProxy to retrieve incorrect cached instantiations. This patch adds object identity (pointer address) to the hash calculation for CPPOverload and TemplateProxy types using golden ratio bit mixing. Fixes #20526.
1 parent e933ef7 commit 87380b5

File tree

3 files changed

+72
-24
lines changed

3 files changed

+72
-24
lines changed

bindings/pyroot/cppyy/CPyCppyy/src/CPPOverload.cxx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,63 @@
1717
#include "CPPInstance.h"
1818
#include "CallContext.h"
1919
#include "PyStrings.h"
20+
#include "TemplateProxy.h"
2021
#include "Utility.h"
2122

2223
// Standard
2324
#include <algorithm>
25+
#include <functional>
2426
#include <sstream>
2527
#include <vector>
2628

2729

2830
namespace CPyCppyy {
2931

32+
//----------------------------------------------------------------------------
33+
// Combine name hash and method count for stable identity
34+
35+
namespace {
36+
constexpr uint64_t kGoldenRatioHash = 0x9e3779b9ULL;
37+
38+
inline void hash_combine(uint64_t &seed, uint64_t value)
39+
{
40+
seed ^= value + kGoldenRatioHash + (seed << 6) + (seed >> 2);
41+
}
42+
} // anonymous namespace
43+
44+
uint64_t HashSignature(CPyCppyy_PyArgs_t args, size_t nargsf)
45+
{
46+
uint64_t hash = 0;
47+
std::hash<std::string> str_hash;
48+
49+
Py_ssize_t nargs = CPyCppyy_PyArgs_GET_SIZE(args, nargsf);
50+
for (Py_ssize_t i = 0; i < nargs; ++i) {
51+
PyObject *pyobj = CPyCppyy_PyArgs_GET_ITEM(args, i);
52+
53+
if (CPPOverload_Check(pyobj)) {
54+
// Hash the method name and overload count for uniqueness
55+
// All O(1) access - no RTTI, no string construction
56+
auto *ol = static_cast<CPPOverload *>(pyobj);
57+
hash_combine(hash, str_hash(ol->fMethodInfo->fName));
58+
hash_combine(hash, static_cast<uint64_t>(ol->fMethodInfo->fMethods.size()));
59+
} else if (TemplateProxy_Check(pyobj)) {
60+
// Hash the stable template name (fCppName includes scope for templates)
61+
auto *tp = static_cast<TemplateProxy *>(pyobj);
62+
hash_combine(hash, str_hash(tp->fTI->fCppName));
63+
} else {
64+
// Standard type-based hashing for other objects
65+
hash_combine(hash, reinterpret_cast<uint64_t>(Py_TYPE(pyobj)));
66+
#if PY_VERSION_HEX >= 0x030e0000
67+
hash_combine(hash, PyUnstable_Object_IsUniqueReferencedTemporary(pyobj) ? 1ULL : 0ULL);
68+
#else
69+
hash_combine(hash, Py_REFCNT(pyobj) == 1 ? 1ULL : 0ULL);
70+
#endif
71+
}
72+
}
73+
74+
return hash;
75+
}
76+
3077
namespace {
3178

3279
// from CPython's instancemethod: Free list for method objects to safe malloc/free overhead

bindings/pyroot/cppyy/CPyCppyy/src/CPPOverload.h

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "PyCallable.h"
66

77
// Standard
8+
#include <cstdint>
89
#include <map>
910
#include <string>
1011
#include <utility>
@@ -13,30 +14,8 @@
1314

1415
namespace CPyCppyy {
1516

16-
// signature hashes are also used by TemplateProxy
17-
inline uint64_t HashSignature(CPyCppyy_PyArgs_t args, size_t nargsf)
18-
{
19-
// Build a hash from the types of the given python function arguments.
20-
uint64_t hash = 0;
21-
22-
Py_ssize_t nargs = CPyCppyy_PyArgs_GET_SIZE(args, nargsf);
23-
for (Py_ssize_t i = 0; i < nargs; ++i) {
24-
// TODO: hashing in the ref-count is for moves; resolve this together with the
25-
// improved overloads for implicit conversions
26-
PyObject* pyobj = CPyCppyy_PyArgs_GET_ITEM(args, i);
27-
hash += (uint64_t)Py_TYPE(pyobj);
28-
#if PY_VERSION_HEX >= 0x030e0000
29-
hash += (uint64_t)(PyUnstable_Object_IsUniqueReferencedTemporary(pyobj) ? 1 : 0);
30-
#else
31-
hash += (uint64_t)(Py_REFCNT(pyobj) == 1 ? 1 : 0);
32-
#endif
33-
hash += (hash << 10); hash ^= (hash >> 6);
34-
}
35-
36-
hash += (hash << 3); hash ^= (hash >> 11); hash += (hash << 15);
37-
38-
return hash;
39-
}
17+
// Signature hashing for memoization (implementation in CPPOverload.cxx)
18+
uint64_t HashSignature(CPyCppyy_PyArgs_t args, size_t nargsf);
4019

4120
class CPPOverload {
4221
public:

bindings/pyroot/cppyy/cppyy/test/test_templates.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,6 +1159,28 @@ def test34_cstring_template_argument(self):
11591159
assert ns.stringify["const char*"]("Aap") == "Aap "
11601160
assert ns.stringify(ctypes.c_char_p(bytes("Noot", "ascii"))) == "Noot "
11611161

1162+
def test35_issue_20526_hash_collision(self):
1163+
"""Fix #20526: Ensure distinct global functions have distinct hashes"""
1164+
1165+
import cppyy
1166+
1167+
cppyy.cppdef("""
1168+
namespace Issue20526 {
1169+
int funcA() { return 101; }
1170+
int funcB() { return 202; }
1171+
template<typename T> int call_it(T f) { return f(); }
1172+
}
1173+
""")
1174+
1175+
ns = cppyy.gbl.Issue20526
1176+
1177+
# If hashes collide, the cache returns the wrong instantiation
1178+
resA = ns.call_it(ns.funcA)
1179+
resB = ns.call_it(ns.funcB)
1180+
1181+
assert resA == 101
1182+
assert resB == 202
1183+
11621184

11631185
class TestTEMPLATED_TYPEDEFS:
11641186
def setup_class(cls):

0 commit comments

Comments
 (0)