Add default convention to CallingConvention. (#344)

* Added default convention to ICallingConvention.

* Changed constructor to create the default convention from MakeDynamicHooksConvention.

* Removed "ICallingConventionWrapper *" from the held type.

* Fixed removal of setting null.

* Changed m_pCallingConvention to m_pDefaultCallingConvention.

* Simplified CFunction destructor.

* Fixed a crash introduced by removing "ICallingConventionWrapper *".

* Removed the convention headers.

* Fixed a crash on unloading Source.Python.

* Fix for VC++ 2010.

* Fix2 for VC++ 2010.

* Added an overload for C-type functions to CFunction::AddHook.

* Fixed CallingConvention's leaks/issues.

* Minor fixes.

* Modified CFunction to initialize CallingConvention with default_convention class variable.

* Added custom_convention(m_oCallingConvention) attribute to Function.

* Added post-construction initialization support.
Fixed unmanaged CallingConvention's wrapper not being initialized.
Changed retrieval priority of default_convention.

* Fixed built-in conventions leaking when they are being unhooked while no longer being bound to a CFunction instance.

* Replaced the post-construction wrapper with stackable policies.

* Fixed namespace issue.

Co-authored-by: Jordan Brière <github@jordanbriere.com>

Author: CookStar

src/core/modules/memory/memory_function.cpp

@@ -35,6 +35,7 @@
 #include "memory_function.h"
 #include "memory_utilities.h"
 #include "memory_hooks.h"
+#include "memory_wrap.h"
 
 // DynamicHooks
 #include "conventions/x86MsCdecl.h"
@@ -80,7 +81,7 @@ int GetDynCallConvention(Convention_t eConv)
 		case CONV_FASTCALL: return DC_CALL_C_X86_WIN32_FAST_MS;
 #endif
 	}
-	
+
 	BOOST_RAISE_EXCEPTION(PyExc_ValueError, "Unsupported calling convention.")
 	return -1;
 }
@@ -89,7 +90,7 @@ int GetDynCallConvention(Convention_t eConv)
 // ============================================================================
 // >> MakeDynamicHooksConvention
 // ============================================================================
-ICallingConvention* MakeDynamicHooksConvention(Convention_t eConv, std::vector<DataType_t> vecArgTypes, DataType_t returnType, int iAlignment=4)
+ICallingConvention* MakeDynamicHooksConvention(Convention_t eConv, std::vector<DataType_t> vecArgTypes, DataType_t returnType, int iAlignment)
 {
 #ifdef _WIN32
 	switch (eConv)
@@ -145,27 +146,19 @@ CFunction::CFunction(unsigned long ulAddr, object oCallingConvention, object oAr
 		// If this line succeeds the user wants to create a function with the built-in calling conventions
 		m_eCallingConvention = extract<Convention_t>(oCallingConvention);
 		m_pCallingConvention = MakeDynamicHooksConvention(m_eCallingConvention, ObjectToDataTypeVector(m_tArgs), m_eReturnType);
-
-		// We allocated the calling convention, we are responsible to cleanup.
-		m_bAllocatedCallingConvention = true;
+		m_oCallingConvention = object();
 	}
 	catch( ... )
 	{
 		PyErr_Clear();
-	
+
 		// A custom calling convention will be used...
 		m_eCallingConvention = CONV_CUSTOM;
-		object _oCallingConvention = oCallingConvention(m_tArgs, m_eReturnType);
-
-		// FIXME:
-		// This is required to fix a crash, but it will also cause a memory leak,
-		// because no calling convention object that is created via this method will ever be deleted.
-		// TODO: Pretty sure this was required due to the missing held type definition. It was added, but wasn't tested yet.
-		Py_INCREF(_oCallingConvention.ptr());
-		m_pCallingConvention = extract<ICallingConvention*>(_oCallingConvention);
+		m_oCallingConvention = oCallingConvention(m_tArgs, m_eReturnType);
+		m_pCallingConvention = extract<ICallingConvention*>(m_oCallingConvention);
 
-		// We didn't allocate the calling convention, someone else is responsible for it.
-		m_bAllocatedCallingConvention = false;
+		// Reserve a Python reference for DynamicHooks.
+		Py_INCREF(m_oCallingConvention.ptr());
 	}
 
 	// Step 4: Get the DynCall calling convention
@@ -179,9 +172,7 @@ CFunction::CFunction(unsigned long ulAddr, Convention_t eCallingConvention,
 	m_eCallingConvention = eCallingConvention;
 	m_iCallingConvention = iCallingConvention;
 	m_pCallingConvention = NULL;
-
-	// We didn't allocate the calling convention, someone else is responsible for it.
-	m_bAllocatedCallingConvention = false;
+	m_oCallingConvention = object();
 
 	m_tArgs = tArgs;
 	m_eReturnType = eReturnType;
@@ -190,18 +181,24 @@ CFunction::CFunction(unsigned long ulAddr, Convention_t eCallingConvention,
 
 CFunction::~CFunction()
 {
-	// If we didn't allocate the calling convention, then it is not our responsibility.
-	if (!m_bAllocatedCallingConvention)
+	if (!m_pCallingConvention)
 		return;
 
-	CHook* pHook = GetHookManager()->FindHook((void *) m_ulAddr);
-
-	// DynamicHooks will take care of it for us from there.
-	if (pHook && pHook->m_pCallingConvention == m_pCallingConvention)
-		return;
+	// If the convention isn't flagged as hooked, then we need to take care of it.
+	if (!m_pCallingConvention->m_bHooked)
+	{
+		// If we don't have a Python instance, then we can safely delete it.
+		if (m_oCallingConvention.is_none())
+			delete m_pCallingConvention;
+		// Otherwise, just release our reference and let Python take care of it.
+		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<ICallingConventionWrapper *>(m_pCallingConvention))
+		m_pCallingConvention->m_bHooked = false;
 
-	// Cleanup.
-	delete m_pCallingConvention;
 	m_pCallingConvention = NULL;
 }
 
@@ -287,7 +284,7 @@ object CFunction::Call(tuple args, dict kw)
 
 				dcArgPointer(g_pCallVM, ulAddr);
 				break;
-			} 
+			}
 			case DATA_TYPE_STRING:		dcArgPointer(g_pCallVM, (unsigned long) (void *) extract<char *>(arg)); break;
 			default:					BOOST_RAISE_EXCEPTION(PyExc_ValueError, "Unknown argument type.")
 		}
@@ -357,10 +354,10 @@ void CFunction::AddHook(HookType_t eType, PyObject* pCallable)
 {
 	if (!IsHookable())
 		BOOST_RAISE_EXCEPTION(PyExc_ValueError, "Function is not hookable.")
-		
+
 	Validate();
 	CHook* pHook = GetHookManager()->FindHook((void *) m_ulAddr);
-	
+
 	// Prepare arguments for log message
 	str type = str(eType);
 	const char* szType = extract<const char*>(type);
@@ -391,16 +388,31 @@ void CFunction::AddHook(HookType_t eType, PyObject* pCallable)
 
 	if (!pHook) {
 		pHook = HookFunctionHelper((void *) m_ulAddr, m_pCallingConvention);
-
-		// DynamicHooks will handle our convention from there, regardless if we allocated it or not.
-		m_bAllocatedCallingConvention = false;
 	}
-	
+
 	// Add the hook handler. If it's already added, it won't be added twice
 	pHook->AddCallback(eType, (HookHandlerFn *) (void *) &SP_HookHandler);
 	g_mapCallbacks[pHook][eType].push_back(object(handle<>(borrowed(pCallable))));
 }
 
+bool CFunction::AddHook(HookType_t eType, HookHandlerFn* pFunc)
+{
+	if (!IsHookable())
+		return false;
+
+	CHook* pHook = GetHookManager()->FindHook((void*) m_ulAddr);
+
+	if (!pHook) {
+		pHook = GetHookManager()->HookFunction((void*) m_ulAddr, m_pCallingConvention);
+
+		if (!pHook)
+			return false;
+	}
+
+	pHook->AddCallback(eType, pFunc);
+	return true;
+}
+
 void CFunction::RemoveHook(HookType_t eType, PyObject* pCallable)
 {
 	Validate();
@@ -418,6 +430,26 @@ void CFunction::DeleteHook()
 		return;
 
 	g_mapCallbacks.erase(pHook);
+
+	ICallingConventionWrapper *pConv = dynamic_cast<ICallingConventionWrapper *>(pHook->m_pCallingConvention);
+	if (pConv)
+	{
+		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;
 	GetHookManager()->UnhookFunction((void *) m_ulAddr);

src/core/modules/memory/memory_function.h

@@ -90,6 +90,8 @@ class CFunction: public CPointer, private boost::noncopyable
 
 	void DeleteHook();
 
+	bool AddHook(HookType_t eType, HookHandlerFn* pFunc);
+
 public:
 	boost::python::tuple	m_tArgs;
 	object					m_oConverter;
@@ -104,7 +106,15 @@ class CFunction: public CPointer, private boost::noncopyable
 	// DynamicHooks calling convention (built-in and custom)
 	ICallingConvention*		m_pCallingConvention;
 	bool					m_bAllocatedCallingConvention;
+
+	// Custom calling convention
+	object					m_oCallingConvention;
 };
 
 
+//---------------------------------------------------------------------------------
+// Functions
+//---------------------------------------------------------------------------------
+ICallingConvention* MakeDynamicHooksConvention(Convention_t eConv, std::vector<DataType_t> vecArgTypes, DataType_t returnType, int iAlignment=4);
+
 #endif // _MEMORY_FUNCTION_H

src/core/modules/memory/memory_wrap.cpp

@@ -487,7 +487,7 @@ void export_function(scope _memory)
 		)
 
 		.def("add_hook",
-			&CFunction::AddHook,
+			GET_METHOD(void, CFunction, AddHook, HookType_t eType, PyObject*),
 			"Adds a hook callback.",
 			args("hook_type", "callback")
 		)
@@ -540,6 +540,10 @@ void export_function(scope _memory)
 			&CFunction::m_eCallingConvention
 		)
 
+		.def_readonly("custom_convention",
+			&CFunction::m_oCallingConvention
+		)
+
 		// Properties
 		.add_property("trampoline",
 			make_function(&CFunction::GetTrampoline, manage_new_object_policy()),
@@ -827,19 +831,33 @@ void export_registers(scope _memory)
 // ============================================================================
 void export_calling_convention(scope _memory)
 {
-	class_<ICallingConventionWrapper, ICallingConventionWrapper *, boost::noncopyable>(
+	class_<ICallingConventionWrapper, boost::shared_ptr<ICallingConventionWrapper>, boost::noncopyable>(
 		"CallingConvention",
 		"An an abstract class that is used to create custom calling "
 		"conventions (only available for hooking function and not for"
 		" calling functions).\n",
-		init< object, DataType_t, optional<int> >(
-			(arg("arg_types"), arg("return_type"), arg("alignment")),
+		no_init)
+
+		.def("__init__",
+			make_constructor(
+				&ICallingConventionWrapper::__init__,
+				post_constructor_policies<
+					initialize_wrapper_policies<boost::shared_ptr<ICallingConventionWrapper> >
+				>(
+					make_function(
+						&ICallingConventionWrapper::Initialize,
+						default_call_policies(),
+						args("self", "arg_types", "return_type", "alignment", "default_convention")
+					)
+				),
+				("arg_types", "return_type", arg("alignment")=4, arg("default_convention")=CONV_CUSTOM)
+			),
 			"Initialize the calling convention.\n"
 			"\n"
 			":param iterable arg_types: A list of :class:`DataType` values that define the argument types of a function.\n"
 			":param DataType return_type: The return type of a function.\n"
-			":param int alignment: The stack alignment."
-			)
+			":param int alignment: The stack alignment.\n"
+			":param Convention_t default_convention: The default convention for un override function."
 		)
 
 		.def("get_registers",
@@ -851,14 +869,15 @@ void export_calling_convention(scope _memory)
 			&ICallingConventionWrapper::GetPopSize,
 			"Return the number of bytes that should be added to the stack to clean up."
 		)
-		
+
 		.def("get_argument_ptr",
-			&ICallingConventionWrapper::GetArgumentPtrWrapper,
+			&ICallingConventionWrapper::GetArgumentPtr,
 			(arg("index"), arg("registers")),
 			"Return a pointer to the argument at the given index.\n"
 			"\n"
 			":param int index: The index of the argument.\n"
-			":param Registers registers: A snapshot of all saved registers."
+			":param Registers registers: A snapshot of all saved registers.",
+			return_by_value_policy()
 		)
 
 		.def("argument_ptr_changed",
@@ -872,11 +891,12 @@ void export_calling_convention(scope _memory)
 		)
 
 		.def("get_return_ptr",
-			&ICallingConventionWrapper::GetReturnPtrWrapper,
+			&ICallingConventionWrapper::GetReturnPtr,
 			(arg("registers")),
 			"Return a pointer to the return value.\n"
 			"\n"
-			":param Registers registers: A snapshot of all saved registers."
+			":param Registers registers: A snapshot of all saved registers.",
+			return_by_value_policy()
 		)
 
 		.def("return_ptr_changed",