Skip to content

Commit

Permalink
Fix for managed dicts with a resolved PyDictObject instead of separat…
Browse files Browse the repository at this point in the history
…e values
  • Loading branch information
radoering authored and SeanCline committed Mar 18, 2024
1 parent e57ea4f commit 2d68aa1
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 16 deletions.
4 changes: 2 additions & 2 deletions include/PyDictObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ namespace PyExt::Remote {
{

public: // Construction/Destruction.
explicit PyManagedDict(RemoteType::Offset keysPtr, RemoteType::Offset valuesPtrPtr);
explicit PyManagedDict(RemoteType::Offset keysPtr, RemoteType::Offset valuesPtr);

public: // Members.
auto pairValues() const->std::vector<std::pair<std::unique_ptr<PyObject>, std::unique_ptr<PyObject>>> override;

private:
RemoteType::Offset keysPtr;
RemoteType::Offset valuesPtrPtr;
RemoteType::Offset valuesPtr;

};

Expand Down
10 changes: 5 additions & 5 deletions src/objects/PyDictObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ namespace PyExt::Remote {
}


PyManagedDict::PyManagedDict(RemoteType::Offset keysPtr, RemoteType::Offset valuesPtrPtr)
: keysPtr(keysPtr), valuesPtrPtr(valuesPtrPtr)
PyManagedDict::PyManagedDict(RemoteType::Offset keysPtr, RemoteType::Offset valuesPtr)
: keysPtr(keysPtr), valuesPtr(valuesPtr)
{
}

Expand All @@ -100,14 +100,14 @@ namespace PyExt::Remote {
auto keys = make_unique<PyDictKeysObject>(keysPtr);
auto table = keys->getEntriesTable();
auto tableSize = keys->getEntriesTableSize();
auto valuesPtr = ExtRemoteTyped("(PyObject***)@$extin", valuesPtrPtr).Dereference().GetPtr();
auto nextValue = valuesPtr;
auto ptrSize = utils::getPointerSize();

for (auto i = 0; i < tableSize; ++i, valuesPtr += ptrSize) {
for (auto i = 0; i < tableSize; ++i, nextValue += ptrSize) {
auto dictEntry = table.ArrayElement(i);

auto keyPtr = dictEntry.Field("me_key").GetPtr();
auto valuePtr = ExtRemoteTyped("(PyObject**)@$extin", valuesPtr).Dereference().GetPtr();
auto valuePtr = ExtRemoteTyped("(PyObject**)@$extin", nextValue).Dereference().GetPtr();

if (keyPtr == 0 || valuePtr == 0) //< The hash bucket might be empty.
continue;
Expand Down
9 changes: 8 additions & 1 deletion src/objects/PyObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,16 @@ namespace PyExt::Remote {
{
if (type().isManagedDict()) {
// Python >= 3.11, see PyObject_GenericGetDict
auto pointerSize = utils::getPointerSize();
auto valuesPtrPtr = offset() - 4 * pointerSize;
auto valuesPtr = ExtRemoteTyped("(PyObject***)@$extin", valuesPtrPtr).Dereference().GetPtr();
if (valuesPtr == 0) {
auto dictPtr = ExtRemoteTyped("(PyDictObject**)@$extin", valuesPtrPtr + pointerSize).Dereference().GetPtr();
return dictPtr ? make_unique<PyDictObject>(dictPtr) : nullptr;
}
auto ht = ExtRemoteTyped("PyHeapTypeObject", type().offset(), true);
auto cachedKeys = ht.Field("ht_cached_keys");
return make_unique<PyManagedDict>(cachedKeys.GetPtr(), offset() - 4 * utils::getPointerSize());
return make_unique<PyManagedDict>(cachedKeys.GetPtr(), valuesPtr);
}

// see https://docs.python.org/3.10/c-api/typeobj.html#c.PyTypeObject.tp_dictoffset
Expand Down
17 changes: 9 additions & 8 deletions test/PyExtTest/ObjectDetailsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,15 @@ TEST_CASE("object_details.py has a stack frame with expected locals.", "[integra

vector<vector<string>> expectations{
// Regex only necessary due to Python 2 (dicts not sorted)
{ "d" , "D" , R"(dict: \{\n\t'(d1': 1,\n\t'd2': 2,|d2': 2,\n\t'd1': 1,)\n\})" },
{ "s" , "S" , R"(slots: \{\n\tslot1: 1,\n\tslot2: 2,\n\})" },
{ "dsubd" , "DsubD" , R"(dict: \{\n(\t('d1': 1|'d2': 2|'d3': 3),\n){3}\})" },
{ "ssubs" , "SsubS" , R"(slots: \{\n\tslot3: 3,\n\tslot1: 1,\n\tslot2: 2,\n\})" },
{ "dsubs" , "DsubS" , R"(slots: \{\n\tslot1: 1,\n\tslot2: 2,\n\}\ndict: \{\n\t'd3': 3,\n\})" },
{ "ssubd" , "SsubD" , R"(slots: \{\n\tslot3: 3,\n\}\ndict: \{\n(\t('d1': 1|'d2': 2),\n){2}\})" },
{ "ssubds" , "SsubDS" , R"(slots: \{\n\tslot3: 5,\n\tslot1: 3,\n\tslot2: 4,\n\}\ndict: \{\n(\t('d1': 1|'d2': 2),\n){2}\})" },
{ "negDictOffset", "NegDictOffset", R"(tuple repr: \(1, 2, 3\)\ndict: \{\n\t'attr': 'test',\n\})" },
{ "d" , "D" , R"(dict: \{\n\t'(d1': 1,\n\t'd2': 2,|d2': 2,\n\t'd1': 1,)\n\})" },
{ "s" , "S" , R"(slots: \{\n\tslot1: 1,\n\tslot2: 2,\n\})" },
{ "dsubd" , "DsubD" , R"(dict: \{\n(\t('d1': 1|'d2': 2|'d3': 3),\n){3}\})" },
{ "ssubs" , "SsubS" , R"(slots: \{\n\tslot3: 3,\n\tslot1: 1,\n\tslot2: 2,\n\})" },
{ "dsubs" , "DsubS" , R"(slots: \{\n\tslot1: 1,\n\tslot2: 2,\n\}\ndict: \{\n\t'd3': 3,\n\})" },
{ "ssubd" , "SsubD" , R"(slots: \{\n\tslot3: 3,\n\}\ndict: \{\n(\t('d1': 1|'d2': 2),\n){2}\})" },
{ "ssubds" , "SsubDS" , R"(slots: \{\n\tslot3: 5,\n\tslot1: 3,\n\tslot2: 4,\n\}\ndict: \{\n(\t('d1': 1|'d2': 2),\n){2}\})" },
{ "negDictOffset", "NegDictOffset" , R"(tuple repr: \(1, 2, 3\)\ndict: \{\n\t'attr': 'test',\n\})" },
{ "manDictRes" , "ManagedDictResolved", R"(dict: \{(\n\t'a\d+': \d+,){32}\n\})" },
};

for (auto& objExp : expectations) {
Expand Down
15 changes: 15 additions & 0 deletions test/scripts/object_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@ def __init__(self, tupleValue):
self.attr = 'test'


class ManagedDictResolved(object):
"""managed dict has two modes:
- only values
- fully resolved PyDictObject
We are trying to trigger the second one with this class.
"""
def __init__(self):
# Observation: The more attributes an object has, the more likely the dict is not stored
# as values only but as PyDictObject, which triggers another branch in the code.
for i in range(32):
setattr(self, 'a%s' % i, i)


d = D(1, 2)
s = S(1, 2)
dsubd = DsubD(1, 2, 3)
Expand All @@ -79,4 +93,5 @@ def __init__(self, tupleValue):
ssubd = SsubD(1, 2, 3)
ssubds = SsubDS(1, 2, 3, 4, 5)
negDictOffset = NegDictOffset((1, 2, 3))
manDictRes = ManagedDictResolved()
win32debug.dump_process("object_details.dmp")

0 comments on commit 2d68aa1

Please sign in to comment.