From 282bf03edd280f2ac9a9d96fb9cce60de51d0b43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordan=20Bri=C3=A8re?= Date: Mon, 5 Oct 2020 10:59:56 -0400 Subject: [PATCH 1/3] Added post-construction initialization support. Fixed unmanaged CallingConvention's wrapper not being initialized. Changed retrieval priority of default_convention. --- src/core/modules/memory/memory_function.cpp | 17 +------- src/core/modules/memory/memory_wrap.cpp | 14 +++++-- src/core/modules/memory/memory_wrap.h | 24 ++++++++++++ src/core/utilities/wrap_macros.h | 43 +++++++++++++++++++++ 4 files changed, 79 insertions(+), 19 deletions(-) diff --git a/src/core/modules/memory/memory_function.cpp b/src/core/modules/memory/memory_function.cpp index 290dc8099..70b714cfb 100755 --- a/src/core/modules/memory/memory_function.cpp +++ b/src/core/modules/memory/memory_function.cpp @@ -154,26 +154,11 @@ CFunction::CFunction(unsigned long ulAddr, object oCallingConvention, object oAr // A custom calling convention will be used... m_eCallingConvention = CONV_CUSTOM; - - // Check if default_convention is defined. - if (PyObject_HasAttrString(oCallingConvention.ptr(), "default_convention")) - { - // Extract default_convention and pass it to oCallingConvention. - Convention_t eCallingConvention = extract(oCallingConvention.attr("default_convention")); - m_oCallingConvention = oCallingConvention(m_tArgs, m_eReturnType, 4, eCallingConvention); - } - else - { - m_oCallingConvention = oCallingConvention(m_tArgs, m_eReturnType); - } - + m_oCallingConvention = oCallingConvention(m_tArgs, m_eReturnType); m_pCallingConvention = extract(m_oCallingConvention); // Reserve a Python reference for DynamicHooks. Py_INCREF(m_oCallingConvention.ptr()); - - // Initialize our wrapper so that Python overrides are properly resolved. - detail::initialize_wrapper(m_oCallingConvention.ptr(), get_pointer((ICallingConventionWrapper *)m_pCallingConvention)); } // Step 4: Get the DynCall calling convention diff --git a/src/core/modules/memory/memory_wrap.cpp b/src/core/modules/memory/memory_wrap.cpp index f5d5d3c3e..597cc5cf0 100755 --- a/src/core/modules/memory/memory_wrap.cpp +++ b/src/core/modules/memory/memory_wrap.cpp @@ -839,9 +839,17 @@ void export_calling_convention(scope _memory) no_init) .def("__init__", - make_constructor(&ICallingConventionWrapper::__init__, - default_call_policies(), - (arg("arg_types"), arg("return_type"), arg("alignment")=4, arg("default_convention")=CONV_CUSTOM) + make_constructor_initializer( + make_constructor( + &ICallingConventionWrapper::__init__, + default_call_policies(), + ("arg_types", "return_type", arg("alignment")=4, arg("default_convention")=CONV_CUSTOM) + ), + make_function( + &ICallingConventionWrapper::Initialize, + initializer_call_policies<>(), + ("self", "arg_types", "return_type", arg("alignment")=4, arg("default_convention")=CONV_CUSTOM) + ) ), "Initialize the calling convention.\n" "\n" diff --git a/src/core/modules/memory/memory_wrap.h b/src/core/modules/memory/memory_wrap.h index f6d265ef7..a2e1a07c8 100755 --- a/src/core/modules/memory/memory_wrap.h +++ b/src/core/modules/memory/memory_wrap.h @@ -95,6 +95,30 @@ class ICallingConventionWrapper: public ICallingConvention, public wrapper(self.attr("default_convention")), m_vecArgTypes, m_returnType, m_iAlignment + ); + } + catch (error_already_set &) + { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) + throw_error_already_set(); + + PyErr_Clear(); + } + } + } + virtual std::list GetRegisters() { override get_registers = get_override("get_registers"); diff --git a/src/core/utilities/wrap_macros.h b/src/core/utilities/wrap_macros.h index d70c420b3..382e354ac 100644 --- a/src/core/utilities/wrap_macros.h +++ b/src/core/utilities/wrap_macros.h @@ -248,4 +248,47 @@ typedef return_value_policy copy_const_reference_policy; typedef return_value_policy return_by_value_policy; +//--------------------------------------------------------------------------------- +// Provides post-construction initialization support of the Python instances. +//--------------------------------------------------------------------------------- +template +struct initializer_call_policies : BasePolicies +{ + template + static PyObject *postcall(const ArgumentPackage &args, PyObject *pResult) + { + return incref(Py_None); // __init__ should always return None + } +}; + +template +struct constructor_initializer +{ +public: + constructor_initializer(Constructor constructor, Initializer initializer): + m_constructor(constructor), + m_initializer(initializer) + { + } + + object operator()(boost::python::tuple args, dict kwargs) + { + m_constructor(*args, **kwargs); + return m_initializer(*(make_tuple(args[0]) + args), **kwargs); + } + +private: + object m_constructor; + object m_initializer; +}; + +template +object make_constructor_initializer(Constructor constructor, Initializer initializer) +{ + return raw_function( + constructor_initializer(constructor, initializer), + 1 // self + ); +}; + #endif // _WRAP_MACROS_H From 6d26bca14429b498e5862ccbbfbae57fd0bb7c8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordan=20Bri=C3=A8re?= Date: Mon, 5 Oct 2020 15:35:20 -0400 Subject: [PATCH 2/3] Fixed built-in conventions leaking when they are being unhooked while no longer being bound to a CFunction instance. --- src/core/modules/memory/memory_function.cpp | 30 +++++++++++++++------ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/core/modules/memory/memory_function.cpp b/src/core/modules/memory/memory_function.cpp index 70b714cfb..cc2e5571f 100755 --- a/src/core/modules/memory/memory_function.cpp +++ b/src/core/modules/memory/memory_function.cpp @@ -181,8 +181,11 @@ CFunction::CFunction(unsigned long ulAddr, Convention_t eCallingConvention, CFunction::~CFunction() { + if (!m_pCallingConvention) + return; + // If the convention isn't flagged as hooked, then we need to take care of it. - if (m_pCallingConvention && !m_pCallingConvention->m_bHooked) + if (!m_pCallingConvention->m_bHooked) { // If we don't have a Python instance, then we can safely delete it. if (m_oCallingConvention.is_none()) @@ -191,6 +194,10 @@ CFunction::~CFunction() else if (Py_REFCNT(m_oCallingConvention.ptr()) > 1) Py_DECREF(m_oCallingConvention.ptr()); } + // If we are using a built-in convention that is currently hooked, let's flag it as no longer hooked + // so that we know we are not bound to a CFunction anymore and can be deleted. + else if (m_eCallingConvention != CONV_CUSTOM && !dynamic_cast(m_pCallingConvention)) + m_pCallingConvention->m_bHooked = false; m_pCallingConvention = NULL; } @@ -424,17 +431,24 @@ void CFunction::DeleteHook() g_mapCallbacks.erase(pHook); - // Flag the convention as no longer hooked and being taken care of by DynamicHooks. - pHook->m_pCallingConvention->m_bHooked = false; - - // Release the Python reference we reserved for DynamicHooks. ICallingConventionWrapper *pConv = dynamic_cast(pHook->m_pCallingConvention); if (pConv) { - PyObject *pOwner = detail::wrapper_base_::owner(pConv); - if (pOwner && Py_REFCNT(pOwner)) - Py_DECREF(pOwner); + if (pConv->m_bHooked) + { + // Flag the convention as no longer hooked and being taken care of by DynamicHooks. + pHook->m_pCallingConvention->m_bHooked = false; + + // Release the Python reference we reserved for DynamicHooks. + PyObject *pOwner = detail::wrapper_base_::owner(pConv); + if (pOwner && Py_REFCNT(pOwner)) + Py_DECREF(pOwner); + } } + // If we are a built-in convention bound to a CHook instance but not flagged as hooked anymore, then that + // means we are no longer bound to a CFunction instance and can be safely deleted. + else if (!pHook->m_pCallingConvention->m_bHooked) + delete pHook->m_pCallingConvention; // Set the calling convention to NULL, because DynamicHooks will delete it otherwise. pHook->m_pCallingConvention = NULL; From 98d985348d1269a745e751eb8475be85f5500039 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordan=20Bri=C3=A8re?= Date: Tue, 6 Oct 2020 17:59:51 -0400 Subject: [PATCH 3/3] Replaced the post-construction wrapper with stackable policies. --- src/core/modules/memory/memory_wrap.cpp | 21 ++++++----- src/core/modules/memory/memory_wrap.h | 5 +-- src/core/utilities/wrap_macros.h | 49 ++++++++++++++----------- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/src/core/modules/memory/memory_wrap.cpp b/src/core/modules/memory/memory_wrap.cpp index 597cc5cf0..593e27f94 100755 --- a/src/core/modules/memory/memory_wrap.cpp +++ b/src/core/modules/memory/memory_wrap.cpp @@ -839,17 +839,18 @@ void export_calling_convention(scope _memory) no_init) .def("__init__", - make_constructor_initializer( - make_constructor( - &ICallingConventionWrapper::__init__, - default_call_policies(), - ("arg_types", "return_type", arg("alignment")=4, arg("default_convention")=CONV_CUSTOM) + make_constructor( + &ICallingConventionWrapper::__init__, + post_constructor_policies< + initialize_wrapper_policies > + >( + make_function( + &ICallingConventionWrapper::Initialize, + default_call_policies(), + args("self", "arg_types", "return_type", "alignment", "default_convention") + ) ), - make_function( - &ICallingConventionWrapper::Initialize, - initializer_call_policies<>(), - ("self", "arg_types", "return_type", arg("alignment")=4, arg("default_convention")=CONV_CUSTOM) - ) + ("arg_types", "return_type", arg("alignment")=4, arg("default_convention")=CONV_CUSTOM) ), "Initialize the calling convention.\n" "\n" diff --git a/src/core/modules/memory/memory_wrap.h b/src/core/modules/memory/memory_wrap.h index a2e1a07c8..ee1ac713a 100755 --- a/src/core/modules/memory/memory_wrap.h +++ b/src/core/modules/memory/memory_wrap.h @@ -95,11 +95,8 @@ class ICallingConventionWrapper: public ICallingConvention, public wrapper copy_const_reference_policy; typedef return_value_policy return_by_value_policy; //--------------------------------------------------------------------------------- -// Provides post-construction initialization support of the Python instances. +// Call policies that initializes the wrapper hierarchy. //--------------------------------------------------------------------------------- -template -struct initializer_call_policies : BasePolicies +template +struct initialize_wrapper_policies : BasePolicies { template static PyObject *postcall(const ArgumentPackage &args, PyObject *pResult) { - return incref(Py_None); // __init__ should always return None + PyObject *pSelf = detail::get(boost::mpl::int_(), args); + detail::initialize_wrapper( + pSelf, + get_pointer((HeldType)extract(pSelf)) + ); + + return BasePolicies::postcall(args, pResult); } }; -template -struct constructor_initializer +//--------------------------------------------------------------------------------- +// Provides post-construction initialization support of the Python instances. +//--------------------------------------------------------------------------------- +template +struct post_constructor_policies : BasePolicies { public: - constructor_initializer(Constructor constructor, Initializer initializer): - m_constructor(constructor), + post_constructor_policies(object initializer): m_initializer(initializer) { } - object operator()(boost::python::tuple args, dict kwargs) + template + PyObject *postcall(const ArgumentPackage &args, PyObject *pResult) { - m_constructor(*args, **kwargs); - return m_initializer(*(make_tuple(args[0]) + args), **kwargs); + BasePolicies::postcall(args, pResult); + m_initializer( + *(make_tuple( + object(handle<>(incref(detail::get(boost::mpl::int_(), args))))) + + boost::python::tuple(handle<>(args.base)) + ) + ); + + decref(pResult); + return incref(Py_None); // __init__ should always return None } private: - object m_constructor; object m_initializer; }; -template -object make_constructor_initializer(Constructor constructor, Initializer initializer) -{ - return raw_function( - constructor_initializer(constructor, initializer), - 1 // self - ); -}; - #endif // _WRAP_MACROS_H