From 5178fb9523dcd3990e7666fc7083509831a0a256 Mon Sep 17 00:00:00 2001 From: David Rose Date: Mon, 25 Mar 2013 15:15:08 +0000 Subject: [PATCH] fix problem with infinite recursion on coercion within constructors --- .../interfaceMakerPythonNative.cxx | 93 +++++++++++++------ .../interrogate/interfaceMakerPythonNative.h | 7 +- dtool/src/interrogatedb/dtool_super_base.cxx | 5 + dtool/src/interrogatedb/py_panda.cxx | 18 +++- dtool/src/interrogatedb/py_panda.h | 9 +- 5 files changed, 97 insertions(+), 35 deletions(-) diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.cxx b/dtool/src/interrogate/interfaceMakerPythonNative.cxx index 38d7ddb06d..6e3cc107c8 100755 --- a/dtool/src/interrogate/interfaceMakerPythonNative.cxx +++ b/dtool/src/interrogate/interfaceMakerPythonNative.cxx @@ -856,12 +856,37 @@ write_class_details(ostream &out, Object * obj) { out << " return -1;\n" ; out << "}\n"; + out + << "int Dtool_InitNoCoerce_" << ClassName << "(PyObject *self, PyObject *args, PyObject *kwds) {\n" + << " return Dtool_Init_" << ClassName << "(self, args, kwds);\n" + << "}\n\n"; + } else { + bool coercion_attempted = false; for (fi = obj->_constructors.begin(); fi != obj->_constructors.end(); ++fi) { Function *func = (*fi); std::string fname = "int Dtool_Init_"+ClassName+"(PyObject *self, PyObject *args, PyObject *kwds)"; - write_function_for_name(out, obj, func,fname,"",ClassName); + write_function_for_name(out, obj, func,fname,"",ClassName, true, coercion_attempted); + } + if (coercion_attempted) { + // If a coercion attempt was written into the above constructor, + // then write a secondary constructor, that won't attempt any + // coercion. We'll need this for nested coercion calls. + for (fi = obj->_constructors.begin(); fi != obj->_constructors.end(); ++fi) { + Function *func = (*fi); + std::string fname = "int Dtool_InitNoCoerce_"+ClassName+"(PyObject *self, PyObject *args, PyObject *kwds)"; + + write_function_for_name(out, obj, func,fname,"",ClassName, false, coercion_attempted); + } + } else { + // Otherwise, since the above constructor didn't involve any + // coercion anyway, we can use the same function for both + // purposes. Construct a trivial wrapper. + out + << "int Dtool_InitNoCoerce_" << ClassName << "(PyObject *self, PyObject *args, PyObject *kwds) {\n" + << " return Dtool_Init_" << ClassName << "(self, args, kwds);\n" + << "}\n\n"; } } @@ -1657,7 +1682,8 @@ write_module_class(ostream &out, Object *obj) { } ostringstream forward_decl; string expected_params; - write_function_forset(out, obj, func, remaps, expected_params, 2, forward_decl, false, args_cleanup); + bool coercion_attempted = false; + write_function_forset(out, obj, func, remaps, expected_params, 2, forward_decl, false, true, coercion_attempted, args_cleanup); out << " if (PyErr_Occurred() && PyErr_ExceptionMatches(PyExc_TypeError)) {\n"; out << " PyErr_Clear();\n"; out << " }\n"; @@ -1966,7 +1992,8 @@ void InterfaceMakerPythonNative:: write_function_for_top(ostream &out, InterfaceMaker::Object *obj, InterfaceMaker::Function *func, const std::string &PreProcess) { std::string fname = "static PyObject *" + func->_name + "(PyObject *self, PyObject *args, PyObject *kwds)"; - write_function_for_name(out, obj, func, fname, PreProcess, ""); + bool coercion_attempted = false; + write_function_for_name(out, obj, func, fname, PreProcess, "", true, coercion_attempted); } //////////////////////////////////////////////////////////////////// @@ -1978,7 +2005,7 @@ void InterfaceMakerPythonNative:: write_function_for_name(ostream &out1, InterfaceMaker::Object *obj, InterfaceMaker::Function *func, const std::string &function_name, const std::string &PreProcess, - const std::string &ClassName) { + const std::string &ClassName, bool coercion_allowed, bool &coercion_attempted) { ostringstream forward_decl; ostringstream out; @@ -2042,7 +2069,7 @@ write_function_for_name(ostream &out1, InterfaceMaker::Object *obj, InterfaceMak for (mii = MapSets.begin(); mii != MapSets.end(); mii ++) { indent(out, 2) << "case " << mii->first << ": {\n"; - write_function_forset(out, obj, func, mii->second, expected_params, 4, forward_decl, is_inplace, ""); + write_function_forset(out, obj, func, mii->second, expected_params, 4, forward_decl, is_inplace, coercion_allowed, coercion_attempted, ""); if ((*mii->second.begin())->_type == FunctionRemap::T_constructor) { constructor = true; } @@ -2107,7 +2134,7 @@ write_function_for_name(ostream &out1, InterfaceMaker::Object *obj, InterfaceMak } else { string expected_params = ""; for (mii = MapSets.begin(); mii != MapSets.end(); mii++) { - write_function_forset(out, obj, func, mii->second, expected_params, 2, forward_decl, is_inplace, ""); + write_function_forset(out, obj, func, mii->second, expected_params, 2, forward_decl, is_inplace, coercion_allowed, coercion_attempted, ""); if ((*mii->second.begin())->_type == FunctionRemap::T_constructor) { constructor = true; } @@ -2256,32 +2283,36 @@ write_function_forset(ostream &out, InterfaceMaker::Object *obj, std::set &remapsin, string &expected_params, int indent_level, ostream &forward_decl, bool is_inplace, + bool coercion_allowed, + bool &coercion_attempted, const string &args_cleanup) { // Do we accept any parameters that are class objects? If so, we // might need to check for parameter coercion. bool coercion_possible = false; - std::set::const_iterator sii; - for (sii = remapsin.begin(); sii != remapsin.end() && !coercion_possible; ++sii) { - FunctionRemap *remap = (*sii); - if (isRemapLegal(*remap)) { - int pn = 0; - if (remap->_has_this) { - // Skip the "this" parameter. It's never coercible. - ++pn; - } - while (pn < (int)remap->_parameters.size()) { - CPPType *type = remap->_parameters[pn]._remap->get_new_type(); - - if (TypeManager::is_char_pointer(type)) { - } else if (TypeManager::is_wchar_pointer(type)) { - } else if (TypeManager::is_pointer_to_PyObject(type)) { - } else if (TypeManager::is_pointer(type)) { - // This is a pointer to an object, so we - // might be able to coerce a parameter to it. - coercion_possible = true; - break; + if (coercion_allowed) { + std::set::const_iterator sii; + for (sii = remapsin.begin(); sii != remapsin.end() && !coercion_possible; ++sii) { + FunctionRemap *remap = (*sii); + if (isRemapLegal(*remap)) { + int pn = 0; + if (remap->_has_this) { + // Skip the "this" parameter. It's never coercible. + ++pn; + } + while (pn < (int)remap->_parameters.size()) { + CPPType *type = remap->_parameters[pn]._remap->get_new_type(); + + if (TypeManager::is_char_pointer(type)) { + } else if (TypeManager::is_wchar_pointer(type)) { + } else if (TypeManager::is_pointer_to_PyObject(type)) { + } else if (TypeManager::is_pointer(type)) { + // This is a pointer to an object, so we + // might be able to coerce a parameter to it. + coercion_possible = true; + break; + } + ++pn; } - ++pn; } } } @@ -2326,7 +2357,7 @@ write_function_forset(ostream &out, InterfaceMaker::Object *obj, indent(out, indent_level) << "// -2 "; remap->write_orig_prototype(out, 0); out << "\n"; - write_function_instance(out, obj, func, remap, expected_params, indent_level + 2, false, forward_decl, func->_name, is_inplace, coercion_possible, args_cleanup); + write_function_instance(out, obj, func, remap, expected_params, indent_level + 2, false, forward_decl, func->_name, is_inplace, coercion_possible, coercion_attempted, args_cleanup); indent(out, indent_level + 2) << "PyErr_Clear(); \n"; indent(out, indent_level) << "}\n\n"; @@ -2353,7 +2384,7 @@ write_function_forset(ostream &out, InterfaceMaker::Object *obj, << "// 1-" ; remap->write_orig_prototype(out, 0); out << "\n" ; - write_function_instance(out, obj, func, remap, expected_params, indent_level + 2, true, forward_decl, func->_name, is_inplace, coercion_possible, args_cleanup); + write_function_instance(out, obj, func, remap, expected_params, indent_level + 2, true, forward_decl, func->_name, is_inplace, coercion_possible, coercion_attempted, args_cleanup); if (remap->_has_this && !remap->_const_method) { indent(out, indent_level) @@ -2435,7 +2466,8 @@ write_function_instance(ostream &out, InterfaceMaker::Object *obj, int indent_level, bool errors_fatal, ostream &ForwardDeclrs, const std::string &functionnamestr, bool is_inplace, - bool coercion_possible, const string &args_cleanup) { + bool coercion_possible, bool &coercion_attempted, + const string &args_cleanup) { string format_specifiers; std::string keyword_list; string parameter_list; @@ -2704,6 +2736,7 @@ write_function_instance(ostream &out, InterfaceMaker::Object *obj, // We never attempt to coerce a copy constructor parameter. // That would lead to infinite recursion. str << ", coerced_ptr, report_errors"; + coercion_attempted = true; } else { str << ", NULL, true"; } diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.h b/dtool/src/interrogate/interfaceMakerPythonNative.h index 2787784f95..2d9beb0b19 100755 --- a/dtool/src/interrogate/interfaceMakerPythonNative.h +++ b/dtool/src/interrogate/interfaceMakerPythonNative.h @@ -86,18 +86,21 @@ private: void write_prototype_for_name(ostream &out, Function *func, const std::string &name); void write_prototype_for(ostream &out, Function *func); - void write_function_for_name(ostream &out, Object *obj, Function *func, const std::string &name, const std::string &PreProcess, const std::string &ClassName); + void write_function_for_name(ostream &out, Object *obj, Function *func, const std::string &name, const std::string &PreProcess, const std::string &ClassName, + bool coercion_allowed, bool &coercion_attempted); void write_function_for_top(ostream &out, Object *obj, Function *func, const std::string &PreProcess); void write_function_instance(ostream &out, Object *obj, Function *func, FunctionRemap *remap, string &expected_params, int indent_level, bool errors_fatal, ostream &forwarddecl, const std::string &functionnamestr, - bool is_inplace, bool coercion_possible, + bool is_inplace, bool coercion_allowed, + bool &coercion_attempted, const string &args_cleanup); void write_function_forset(ostream &out, Object *obj, Function *func, std::set &remaps, string &expected_params, int indent_level, ostream &forwarddecl, bool inplace, + bool coercion_allowed, bool &coercion_attempted, const string &args_cleanup); void pack_return_value(ostream &out, int indent_level, diff --git a/dtool/src/interrogatedb/dtool_super_base.cxx b/dtool/src/interrogatedb/dtool_super_base.cxx index 669748fb11..8b780e11f2 100644 --- a/dtool/src/interrogatedb/dtool_super_base.cxx +++ b/dtool/src/interrogatedb/dtool_super_base.cxx @@ -83,4 +83,9 @@ int Dtool_Init_DTOOL_SUPER_BASE(PyObject *self, PyObject *args, PyObject *kwds) return -1; } +int Dtool_InitNoCoerce_DTOOL_SUPER_BASE(PyObject *self, PyObject *args, PyObject *kwds) { + PyErr_SetString(PyExc_TypeError, "cannot init super base"); + return -1; +} + #endif // HAVE_PYTHON diff --git a/dtool/src/interrogatedb/py_panda.cxx b/dtool/src/interrogatedb/py_panda.cxx index 24d32183bb..48c9d42563 100644 --- a/dtool/src/interrogatedb/py_panda.cxx +++ b/dtool/src/interrogatedb/py_panda.cxx @@ -71,9 +71,25 @@ attempt_coercion(PyObject *self, Dtool_PyTypedObject *classdef, if (coerced != NULL) { // Attempt coercion: try to create a temporary instance of the // required class using the supplied parameter. - PyObject *obj = PyObject_Call((PyObject *)classdef, self, NULL); + // Because we want to use the special InitNoCoerce constructor + // here instead of the regular constructor (we don't want to risk + // recursive coercion on the nested type we're creating), we have + // to call the constructor with a few more steps. + PyObject *obj = NULL; + if (classdef->_PyType.tp_new != NULL) { + obj = classdef->_PyType.tp_new(&classdef->_PyType, self, NULL); + assert(obj != NULL); + if (classdef->_Dtool_InitNoCoerce(obj, self, NULL) != 0) { + Py_DECREF(obj); + obj = NULL; + } + } if (obj == NULL) { // That didn't work; try to call a static "make" method instead. + // Presently, we don't bother filtering this for coercion, + // because none of our classes suffer from a recursion danger + // here. Maybe one day we will need to also construct a + // makeNoCoerce wrapper? PyObject *make = PyObject_GetAttrString((PyObject *)classdef, "make"); if (make != NULL) { PyErr_Clear(); diff --git a/dtool/src/interrogatedb/py_panda.h b/dtool/src/interrogatedb/py_panda.h index 4c6518e962..86b6888bef 100755 --- a/dtool/src/interrogatedb/py_panda.h +++ b/dtool/src/interrogatedb/py_panda.h @@ -139,6 +139,8 @@ typedef void * ( * ConvertFunctionType )(PyObject *,Dtool_PyTypedObject * ); typedef void * ( * ConvertFunctionType1 )(void *, Dtool_PyTypedObject *); typedef void ( *FreeFunction )(PyObject *); typedef void ( *PyModuleClassInit)(PyObject *module); +typedef int ( *InitNoCoerce)(PyObject *self, PyObject *args, PyObject *kwds); + //inline Dtool_PyTypedObject * Dtool_RuntimeTypeDtoolType(int type); inline void Dtool_Deallocate_General(PyObject * self); //inline int DTOOL_PyObject_Compare(PyObject *v1, PyObject *v2); @@ -186,7 +188,8 @@ struct Dtool_PyTypedObject { ConvertFunctionType _Dtool_UpcastInterface; // The Upcast Function By Slot ConvertFunctionType1 _Dtool_DowncastInterface; // The Downcast Function By Slot FreeFunction _Dtool_FreeInstance; - PyModuleClassInit _Dtool_ClassInit; // The init function pointer + PyModuleClassInit _Dtool_ClassInit; // The module init function pointer + InitNoCoerce _Dtool_InitNoCoerce; // A variant of the constructor that does not attempt to perform coercion of its arguments. // some convenience functions.. inline PyTypeObject &As_PyTypeObject() { return _PyType; }; @@ -245,7 +248,8 @@ struct Dtool_PyTypedObject { Dtool_UpcastInterface_##CLASS_NAME, \ Dtool_DowncastInterface_##CLASS_NAME, \ Dtool_FreeInstance_##CLASS_NAME, \ - Dtool_PyModuleClassInit_##CLASS_NAME \ + Dtool_PyModuleClassInit_##CLASS_NAME, \ + Dtool_InitNoCoerce_##CLASS_NAME \ }; #if PY_MAJOR_VERSION >= 3 @@ -482,6 +486,7 @@ EXPCL_DTOOLCONFIG PyObject *DTool_CreatePyInstance(void *local_this, Dtool_PyTyp extern EXPORT_THIS Dtool_PyTypedObject Dtool_##CLASS_NAME;\ extern struct PyMethodDef Dtool_Methods_##CLASS_NAME[];\ int Dtool_Init_##CLASS_NAME(PyObject *self, PyObject *args, PyObject *kwds);\ +int Dtool_InitNoCoerce_##CLASS_NAME(PyObject *self, PyObject *args, PyObject *kwds);\ PyObject * Dtool_new_##CLASS_NAME(PyTypeObject *type, PyObject *args, PyObject *kwds);\ void * Dtool_UpcastInterface_##CLASS_NAME(PyObject *self, Dtool_PyTypedObject *requested_type);\ void * Dtool_DowncastInterface_##CLASS_NAME(void *self, Dtool_PyTypedObject *requested_type);\