From e254c3c2aa140016e298107a0297885234abfde7 Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Thu, 13 Dec 2018 14:03:32 +0100 Subject: Fix crash related to multiple inheritance In the _PTR_CppToPython_ converter function (written by CppGenerator::writeConverterFunctions()), the generated code used typeid(*ptr).name() to retrieve the name to use for the SbkObjectTypes. This construct returns the name of the outermost class (for example, "QWidget" for a QWidget-type paint device returned by QPainter::device()), as opposed to "QPaintDevice *" returned by typeid(ptr).name(). This caused a crash with multiple inheritance since QWidget inherits QObject and QPaintDevice and the "QWidget" type was associated with the QPaintDevice pointer. To fix this: - Add API to libshiboken to obtain the SbkObjectType* by name and check for the presence of a special cast function (multiple inheritance). - Generate the code of _PTR_CppToPython_ as follows: Check whether the outermost type obtained by typeid(*ptr).name() has a special cast function. If that is the case, use the type name obtained by typeid(ptr).name() (base class) to create the wrapper. Change-Id: I8ee6b4c084e9dafa434623433661809b83aedee5 Fixes: PYSIDE-868 Reviewed-by: Cristian Maureira-Fredes --- .../pyside2/tests/QtWidgets/qgraphicsscene_test.py | 25 +++++++++++- .../shiboken2/generator/shiboken2/cppgenerator.cpp | 44 ++++++++++++++++++++-- sources/shiboken2/libshiboken/basewrapper.cpp | 28 ++++++++++---- sources/shiboken2/libshiboken/basewrapper.h | 14 +++++++ .../tests/otherbinding/typediscovery_test.py | 10 +++-- 5 files changed, 103 insertions(+), 18 deletions(-) diff --git a/sources/pyside2/tests/QtWidgets/qgraphicsscene_test.py b/sources/pyside2/tests/QtWidgets/qgraphicsscene_test.py index 07926d177..80a09dc57 100644 --- a/sources/pyside2/tests/QtWidgets/qgraphicsscene_test.py +++ b/sources/pyside2/tests/QtWidgets/qgraphicsscene_test.py @@ -32,14 +32,14 @@ import unittest import gc from PySide2.QtCore import QPointF -from PySide2.QtGui import QPolygonF, QPixmap, QPainterPath, QTransform +from PySide2.QtGui import QPolygonF, QPixmap, QPainterPath, QTransform, QWindow from PySide2.QtWidgets import QApplication, QPushButton from PySide2.QtWidgets import QGraphicsScene from PySide2.QtWidgets import QGraphicsEllipseItem, QGraphicsLineItem from PySide2.QtWidgets import QGraphicsPathItem, QGraphicsPixmapItem from PySide2.QtWidgets import QGraphicsPolygonItem, QGraphicsRectItem from PySide2.QtWidgets import QGraphicsSimpleTextItem, QGraphicsTextItem -from PySide2.QtWidgets import QGraphicsProxyWidget +from PySide2.QtWidgets import QGraphicsProxyWidget, QGraphicsView from helper import UsesQApplication @@ -51,6 +51,19 @@ class Constructor(unittest.TestCase): obj = QGraphicsScene() self.assertTrue(isinstance(obj, QGraphicsScene)) +# Test for PYSIDE-868: Test whether painter.device() can be accessed +# correctly. This was crashing when the underlying QPaintDevice was a +# QWidget due to handling multiple inheritance incorrectly. +class CustomScene(QGraphicsScene): + def __init__(self, parent = None): + super(CustomScene, self).__init__(parent) + self.dpi = 0 + + def drawBackground(self, painter, rect): + self.dpi = painter.device().physicalDpiX() + + def drawForeground(self, painter, rect): + self.dpi = painter.device().physicalDpiX() class ConstructorWithRect(unittest.TestCase): '''QGraphicsScene qrect constructor and related sizes''' @@ -192,6 +205,14 @@ class TestGraphicsGroup(UsesQApplication): scene.destroyItemGroup(group) self.assertRaises(RuntimeError, group.type) + def testCustomScene(self): # For PYSIDE-868, see above + scene = CustomScene() + view = QGraphicsView(scene) + view.show() + while scene.dpi == 0: + QApplication.instance().processEvents() + view.hide() + if __name__ == '__main__': unittest.main() diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp index e46e0b968..9df0d61c5 100644 --- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp +++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp @@ -50,6 +50,30 @@ QHash CppGenerator::m_sqFuncs = QHash(); QHash CppGenerator::m_mpFuncs = QHash(); QString CppGenerator::m_currentErrorCode(QLatin1String("0")); +static const char typeNameFunc[] = R"CPP( +template +static const char *typeNameOf(const T &t) +{ + const char *typeName = typeid(t).name(); + auto size = std::strlen(typeName); +#if defined(Q_CC_MSVC) // MSVC: "class QPaintDevice * __ptr64" + if (auto lastStar = strchr(typeName, '*')) { + // MSVC: "class QPaintDevice * __ptr64" + while (*--lastStar == ' ') { + } + size = lastStar - typeName + 1; + } +#else // g++, Clang: "QPaintDevice *" -> "P12QPaintDevice" + if (size > 2 && typeName[0] == 'P' && std::isdigit(typeName[1])) + ++typeName; +#endif + char *result = new char[size + 1]; + result[size] = '\0'; + strncpy(result, typeName, size); + return result; +} +)CPP"; + // utility functions inline AbstractMetaType* getTypeWithoutContainer(AbstractMetaType* arg) { @@ -336,6 +360,8 @@ void CppGenerator::generateClass(QTextStream &s, GeneratorContext &classContext) s << inc.toString() << endl; s << endl; + s << "\n#include \n#include \n"; + if (metaClass->typeEntry()->typeFlags() & ComplexTypeEntry::Deprecated) s << "#Deprecated" << endl; @@ -351,7 +377,7 @@ void CppGenerator::generateClass(QTextStream &s, GeneratorContext &classContext) } } - s << endl; + s << endl << endl << typeNameFunc << endl; // Create string literal for smart pointer getter method. if (classContext.forSmartPointer()) { @@ -1226,9 +1252,19 @@ void CppGenerator::writeConverterFunctions(QTextStream &s, const AbstractMetaCla c << INDENT << "return pyOut;" << endl; } c << INDENT << '}' << endl; - c << INDENT << "const char* typeName = typeid(*((" << typeName << "*)cppIn)).name();" << endl; - c << INDENT << "return Shiboken::Object::newObject(" << cpythonType; - c << ", const_cast(cppIn), false, false, typeName);"; + c << INDENT << "bool changedTypeName = false;\n" + << INDENT << "auto tCppIn = reinterpret_cast(cppIn);\n" + << INDENT << "const char *typeName = typeid(*tCppIn).name();\n" + << INDENT << "auto sbkType = Shiboken::ObjectType::typeForTypeName(typeName);\n" + << INDENT << "if (sbkType && Shiboken::ObjectType::hasSpecialCastFunction(sbkType)) {\n" + << INDENT << " typeName = typeNameOf(tCppIn);\n" + << INDENT << " changedTypeName = true;\n" + << INDENT << " }\n" + << INDENT << "PyObject *result = Shiboken::Object::newObject(" << cpythonType + << ", const_cast(cppIn), false, /* exactType */ changedTypeName, typeName);\n" + << INDENT << "if (changedTypeName)\n" + << INDENT << " delete [] typeName;\n" + << INDENT << "return result;"; } std::swap(targetTypeName, sourceTypeName); writeCppToPythonFunction(s, code, sourceTypeName, targetTypeName); diff --git a/sources/shiboken2/libshiboken/basewrapper.cpp b/sources/shiboken2/libshiboken/basewrapper.cpp index c9e3b9d1b..e820e749b 100644 --- a/sources/shiboken2/libshiboken/basewrapper.cpp +++ b/sources/shiboken2/libshiboken/basewrapper.cpp @@ -831,6 +831,23 @@ void setTypeUserData(SbkObjectType* type, void* userData, DeleteUserDataFunc d_f sotp->d_func = d_func; } +// Try to find the exact type of cptr. +SbkObjectType *typeForTypeName(const char *typeName) +{ + SbkObjectType *result{}; + if (typeName) { + if (PyTypeObject *pyType = Shiboken::Conversions::getPythonTypeObject(typeName)) + result = reinterpret_cast(pyType); + } + return result; +} + +bool hasSpecialCastFunction(SbkObjectType *sbkType) +{ + const SbkObjectTypePrivate *d = PepType_SOTP(sbkType); + return d != nullptr && d->mi_specialcast != nullptr; +} + } // namespace ObjectType @@ -1191,7 +1208,6 @@ SbkObject *findColocatedChild(SbkObject *wrapper, return 0; } - PyObject *newObject(SbkObjectType* instanceType, void* cptr, bool hasOwnership, @@ -1200,13 +1216,9 @@ PyObject *newObject(SbkObjectType* instanceType, { // Try to find the exact type of cptr. if (!isExactType) { - PyTypeObject* exactType = 0; - if (typeName) { - exactType = Shiboken::Conversions::getPythonTypeObject(typeName); - if (exactType) - instanceType = reinterpret_cast(exactType); - } - if (!exactType) + if (SbkObjectType *exactType = ObjectType::typeForTypeName(typeName)) + instanceType = exactType; + else instanceType = BindingManager::instance().resolveType(&cptr, instanceType); } diff --git a/sources/shiboken2/libshiboken/basewrapper.h b/sources/shiboken2/libshiboken/basewrapper.h index 65849d783..15682c600 100644 --- a/sources/shiboken2/libshiboken/basewrapper.h +++ b/sources/shiboken2/libshiboken/basewrapper.h @@ -238,6 +238,20 @@ LIBSHIBOKEN_API void setSubTypeInitHook(SbkObjectType* self, SubTypeInitH LIBSHIBOKEN_API void* getTypeUserData(SbkObjectType* self); LIBSHIBOKEN_API void setTypeUserData(SbkObjectType* self, void* userData, DeleteUserDataFunc d_func); +/** + * Return an instance of SbkObjectType for a C++ type name as determined by + * typeinfo().name(). + * \param typeName Type name + * \since 5.12 + */ +LIBSHIBOKEN_API SbkObjectType *typeForTypeName(const char *typeName); + +/** + * Returns whether SbkObjectType has a special cast function (multiple inheritance) + * \param sbkType Sbk type + * \since 5.12 + */ +LIBSHIBOKEN_API bool hasSpecialCastFunction(SbkObjectType *sbkType); } namespace Object { diff --git a/sources/shiboken2/tests/otherbinding/typediscovery_test.py b/sources/shiboken2/tests/otherbinding/typediscovery_test.py index 6816b158a..a9eb88d80 100644 --- a/sources/shiboken2/tests/otherbinding/typediscovery_test.py +++ b/sources/shiboken2/tests/otherbinding/typediscovery_test.py @@ -51,14 +51,16 @@ class TypeDiscoveryTest(unittest.TestCase): def testMultipleInheritance(self): obj = OtherMultipleDerived.createObject("Base1"); self.assertEqual(type(obj), Base1) + # PYSIDE-868: In case of multiple inheritance, a factory + # function will return the base class wrapper. obj = OtherMultipleDerived.createObject("MDerived1"); - self.assertEqual(type(obj), MDerived1) + self.assertEqual(type(obj), Base1) obj = OtherMultipleDerived.createObject("SonOfMDerived1"); - self.assertEqual(type(obj), SonOfMDerived1) + self.assertEqual(type(obj), Base1) obj = OtherMultipleDerived.createObject("MDerived3"); - self.assertEqual(type(obj), MDerived3) + self.assertEqual(type(obj), Base1) obj = OtherMultipleDerived.createObject("OtherMultipleDerived"); - self.assertEqual(type(obj), OtherMultipleDerived) + self.assertEqual(type(obj), Base1) if __name__ == '__main__': unittest.main() -- cgit v1.2.3