summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFriedemann Kleint <Friedemann.Kleint@qt.io>2018-12-13 14:03:32 +0100
committerFriedemann Kleint <Friedemann.Kleint@qt.io>2019-01-03 18:50:54 +0000
commite254c3c2aa140016e298107a0297885234abfde7 (patch)
treedb15ce3a6329ec3cc87a1b53f91e62ce754aeadc
parent0b352fca7391d01ce410ec0c04c285326e465dc1 (diff)
Fix crash related to multiple inheritance
In the <class>_PTR_CppToPython_<class> 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 <class>_PTR_CppToPython_<class> 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 <cristian.maureira-fredes@qt.io>
-rw-r--r--sources/pyside2/tests/QtWidgets/qgraphicsscene_test.py25
-rw-r--r--sources/shiboken2/generator/shiboken2/cppgenerator.cpp44
-rw-r--r--sources/shiboken2/libshiboken/basewrapper.cpp28
-rw-r--r--sources/shiboken2/libshiboken/basewrapper.h14
-rw-r--r--sources/shiboken2/tests/otherbinding/typediscovery_test.py10
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 07926d17..80a09dc5 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 e46e0b96..9df0d61c 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -50,6 +50,30 @@ QHash<QString, QString> CppGenerator::m_sqFuncs = QHash<QString, QString>();
QHash<QString, QString> CppGenerator::m_mpFuncs = QHash<QString, QString>();
QString CppGenerator::m_currentErrorCode(QLatin1String("0"));
+static const char typeNameFunc[] = R"CPP(
+template <class T>
+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 <cctype>\n#include <cstring>\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<void*>(cppIn), false, false, typeName);";
+ c << INDENT << "bool changedTypeName = false;\n"
+ << INDENT << "auto tCppIn = reinterpret_cast<const " << typeName << " *>(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<void*>(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 c9e3b9d1..e820e749 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<SbkObjectType*>(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<SbkObjectType*>(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 65849d78..15682c60 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 6816b158..a9eb88d8 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()