shiboken6: No longer crash on fatal errors

shiboken has some fatal errors which are too deeply nested to catch with return values. Introduce exception handling to handle this. Add an exception class and replace the existing qFatal() calls by a exceptions. Wrap a try/catch around main() and the tester helper. Change-Id: I03243849dc299f7696ef73d9d4123480ac5cbd66 Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
author: Friedemann Kleint <Friedemann.Kleint@qt.io> 2021-02-26 14:47:59 +0100
committer: Friedemann Kleint <Friedemann.Kleint@qt.io> 2021-03-10 08:48:07 +0100
commit: f7901b0b29e831a3799bd7fc39dbdf5e67271ae8 (patch)
tree: dfe67473b4bca93cb3eb7e2c7ca41ed121611175
parent: 7cd7141e43e98d4862de3aadf128b0d0080090c9 (diff)
7 files changed, 122 insertions, 30 deletions
diff --git a/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp
index 8216a5e8e..73e974386 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp
@@ -31,6 +31,7 @@
 #include "abstractmetafield.h"
 #include "abstractmetafunction.h"
 #include "graph.h"
+#include "exception.h"
 #include "messages.h"
 #include "propertyspec.h"
 #include "reporthandler.h"
@@ -606,7 +607,7 @@ void AbstractMetaBuilderPrivate::traverseDom(const FileModelItem &dom)
     const AddedFunctionList &globalUserFunctions = types->globalUserFunctions();
     for (const AddedFunctionPtr &addedFunc : globalUserFunctions) {
         if (!traverseAddedGlobalFunction(addedFunc, &errorMessage))
-            qFatal("%s", qPrintable(errorMessage));
+            throw Exception(errorMessage);
     }
 
     m_itemToClass.clear();
@@ -1342,7 +1343,7 @@ void AbstractMetaBuilderPrivate::fillAddedFunctions(AbstractMetaClass *metaClass
     const AddedFunctionList &addedFunctions = metaClass->typeEntry()->addedFunctions();
     for (const AddedFunctionPtr &addedFunc : addedFunctions) {
         if (!traverseAddedMemberFunction(addedFunc, metaClass, &errorMessage))
-            qFatal("%s", qPrintable(errorMessage));
+            throw Exception(qPrintable(errorMessage));
     }
 }
 
diff --git a/sources/shiboken6/ApiExtractor/exception.h b/sources/shiboken6/ApiExtractor/exception.h
new file mode 100644
index 000000000..36e94638a
--- /dev/null
+++ b/sources/shiboken6/ApiExtractor/exception.h
@@ -0,0 +1,52 @@
+/****************************************************************************
+**
+** Copyright (C) 2021 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt for Python.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef EXCEPTION_H
+#define EXCEPTION_H
+
+#include <QString>
+
+#include <string>
+#include <exception>
+
+class Exception : public std::exception
+{
+public:
+    explicit Exception(const QString &message) : m_message(message.toUtf8()) {}
+    explicit Exception(const char *message) : m_message(message) {}
+
+    const char *what() const noexcept override
+    {
+        return m_message.c_str();
+    }
+
+private:
+    const std::string m_message;
+};
+
+#endif // EXCEPTION
diff --git a/sources/shiboken6/ApiExtractor/modifications.cpp b/sources/shiboken6/ApiExtractor/modifications.cpp
index 9e5071a33..e4e9a7d02 100644
--- a/sources/shiboken6/ApiExtractor/modifications.cpp
+++ b/sources/shiboken6/ApiExtractor/modifications.cpp
@@ -28,6 +28,7 @@
 
 #include "modifications.h"
 #include "modifications_p.h"
+#include "exception.h"
 #include "typedatabase.h"
 #include "typeparser.h"
 #include "typesystem.h"
@@ -42,8 +43,11 @@ static inline QString callOperator() { return QStringLiteral("operator()"); }
 QString TemplateInstance::expandCode() const
 {
     TemplateEntry *templateEntry = TypeDatabase::instance()->findTemplate(m_name);
-    if (!templateEntry)
-        qFatal("<insert-template> referring to non-existing template '%s'.", qPrintable(m_name));
+    if (!templateEntry) {
+        const QString m = QLatin1String("<insert-template> referring to non-existing template '")
+                          + m_name + QLatin1String("'.");
+        throw Exception(m);
+    }
 
     QString code = templateEntry->code();
     for (auto it = replaceRules.cbegin(), end = replaceRules.cend(); it != end; ++it)
diff --git a/sources/shiboken6/ApiExtractor/tests/testutil.h b/sources/shiboken6/ApiExtractor/tests/testutil.h
index e24e54365..56ce8a72e 100644
--- a/sources/shiboken6/ApiExtractor/tests/testutil.h
+++ b/sources/shiboken6/ApiExtractor/tests/testutil.h
@@ -36,6 +36,9 @@
 #include "reporthandler.h"
 #include "typedatabase.h"
 
+#include <exception>
+#include <memory>
+
 namespace TestUtil
 {
     static AbstractMetaBuilder *parse(const char *cppCode, const char *xmlCode,
@@ -70,12 +73,16 @@ namespace TestUtil
         arguments.append(QFile::encodeName(tempSource.fileName()));
         tempSource.write(cppCode, qint64(strlen(cppCode)));
         tempSource.close();
-        auto *builder = new AbstractMetaBuilder;
-        if (!builder->build(arguments)) {
-            delete builder;
-            return Q_NULLPTR;
+
+        auto builder = std::make_unique<AbstractMetaBuilder>();
+        try {
+            if (!builder->build(arguments))
+                return nullptr;
+        } catch (const std::exception &e) {
+            qWarning("%s", e.what());
+            return nullptr;
         }
-        return builder;
+        return builder.release();
     }
 } // namespace TestUtil
 
diff --git a/sources/shiboken6/generator/main.cpp b/sources/shiboken6/generator/main.cpp
index 049ffaf18..feb9cbc46 100644
--- a/sources/shiboken6/generator/main.cpp
+++ b/sources/shiboken6/generator/main.cpp
@@ -44,6 +44,8 @@
 #include "headergenerator.h"
 #include "qtdocgenerator.h"
 
+#include <exception>
+
 static const QChar clangOptionsSplitter = u',';
 static const QChar dropTypeEntriesSplitter = u';';
 static const QChar apiVersionSplitter = u'|';
@@ -421,7 +423,7 @@ static void parseIncludePathOption(const QString &option, HeaderType headerType,
     }
 }
 
-int main(int argc, char *argv[])
+int shibokenMain(int argc, char *argv[])
 {
     // PYSIDE-757: Request a deterministic ordering of QHash in the code model
     // and type system.
@@ -699,3 +701,15 @@ int main(int argc, char *argv[])
 
     return EXIT_SUCCESS;
 }
+
+int main(int argc, char *argv[])
+{
+    int ex = EXIT_SUCCESS;
+    try {
+        ex = shibokenMain(argc, argv);
+    }  catch (const std::exception &e) {
+        std::cerr << e.what() << std::endl;
+        ex = EXIT_FAILURE;
+    }
+    return ex;
+}
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
index 426f7f6fd..14781527d 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
@@ -31,6 +31,7 @@
 #include "cppgenerator.h"
 #include "apiextractorresult.h"
 #include "ctypenames.h"
+#include <exception.h>
 #include "pytypenames.h"
 #include "fileout.h"
 #include "overloaddata.h"
@@ -2927,8 +2928,11 @@ void CppGenerator::writeSingleFunctionCall(TextStream &s,
             } else if (!injectCodeCallsFunc && !func->isUserAdded() && !hasConversionRule) {
                 // When an argument is removed from a method signature and no other means of calling
                 // the method are provided (as with code injection) the generator must abort.
-                qFatal("No way to call '%s::%s' with the modifications described in the type system.",
-                       qPrintable(func->ownerClass()->name()), qPrintable(func->signature()));
+                QString m;
+                QTextStream(&m) << "No way to call '" << func->ownerClass()->name()
+                    << "::" << func->signature()
+                    << "' with the modifications described in the type system.";
+                throw Exception(m);
             }
             removedArgs++;
             continue;
@@ -3027,9 +3031,11 @@ void CppGenerator::writeCppToPythonFunction(TextStream &s, const AbstractMetaTyp
 {
     const CustomConversion *customConversion = containerType.typeEntry()->customConversion();
     if (!customConversion) {
-        qFatal("Can't write the C++ to Python conversion function for container type '%s' - "\
-               "no conversion rule was defined for it in the type system.",
-               qPrintable(containerType.typeEntry()->qualifiedCppName()));
+        QString m;
+        QTextStream(&m) << "Can't write the C++ to Python conversion function for container type '"
+             << containerType.typeEntry()->qualifiedCppName()
+             << "' - no conversion rule was defined for it in the type system.";
+        throw Exception(m);
     }
     if (!containerType.typeEntry()->isContainer()) {
         writeCppToPythonFunction(s, customConversion);
@@ -3157,10 +3163,10 @@ void CppGenerator::writePythonToCppConversionFunctions(TextStream &s,
     }
     if (typeCheck.isEmpty()) {
         if (!toNative->sourceType() || toNative->sourceType()->isPrimitive()) {
-            qFatal("User added implicit conversion for C++ type '%s' must provide either an input "\
-                   "type check function or a non primitive type entry.",
-                   qPrintable(targetType->qualifiedCppName()));
-
+            QString m;
+            QTextStream(&m) << "User added implicit conversion for C++ type '" << targetType->qualifiedCppName()
+                << "' must provide either an input type check function or a non primitive type entry.";
+            throw Exception(m);
         }
         typeCheck = QString::fromLatin1("PyObject_TypeCheck(%in, %1)").arg(cpythonTypeNameExt(toNative->sourceType()));
     }
@@ -6388,8 +6394,11 @@ void CppGenerator::writeDefaultSequenceMethods(TextStream &s,
 
     const AbstractMetaTypeList &instantiations = metaClass->templateBaseClassInstantiations();
     if (instantiations.isEmpty()) {
-        qFatal("shiboken: %s: Internal error, no instantiations of \"%s\" were found.",
-               __FUNCTION__, qPrintable(metaClass->qualifiedCppName()));
+        QString m;
+        QTextStream(&m) << "shiboken: " << __FUNCTION__
+            << ": Internal error, no instantiations of \"" << metaClass->qualifiedCppName()
+            << "\" were found.";
+        throw Exception(m);
     }
     const AbstractMetaType &itemType = instantiations.constFirst();
 
diff --git a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
index dc4648875..75b3fe890 100644
--- a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
@@ -33,6 +33,7 @@
 #include <abstractmetafield.h>
 #include <abstractmetafunction.h>
 #include <abstractmetalang.h>
+#include <exception.h>
 #include <messages.h>
 #include <modifications.h>
 #include "overloaddata.h"
@@ -1892,7 +1893,7 @@ static QString getConverterTypeSystemVariableArgument(const QString &code, int p
         ++count;
     }
     if (parenthesisDepth != 0)
-        qFatal("Unbalanced parenthesis on type system converter variable call.");
+        throw Exception("Unbalanced parenthesis on type system converter variable call.");
     return arg;
 }
 
@@ -1922,9 +1923,9 @@ void ShibokenGenerator::replaceConverterTypeSystemVariable(TypeSystemConverterVa
         QString message;
         const auto conversionTypeO = buildAbstractMetaTypeFromString(conversionTypeName, &message);
         if (!conversionTypeO.has_value()) {
-            qFatal("%s", qPrintable(msgCannotFindType(conversionTypeName,
-                                                      typeSystemConvName().value(converterVariable),
-                                                      message)));
+            throw Exception(msgCannotFindType(conversionTypeName,
+                                              typeSystemConvName().value(converterVariable),
+                                              message));
         }
         const auto conversionType = conversionTypeO.value();
         QString conversion;
@@ -1944,7 +1945,7 @@ void ShibokenGenerator::replaceConverterTypeSystemVariable(TypeSystemConverterVa
                 if (!varType.isEmpty()) {
                     const QString conversionSignature = conversionType.cppSignature();
                     if (varType != QLatin1String("auto") && varType != conversionSignature)
-                        qFatal("%s", qPrintable(msgConversionTypesDiffer(varType, conversionSignature)));
+                        throw Exception(msgConversionTypesDiffer(varType, conversionSignature));
                     c << getFullTypeName(conversionType) << ' ' << varName;
                     writeMinimalConstructorExpression(c, api(), conversionType);
                     c << ";\n";
@@ -1984,8 +1985,10 @@ void ShibokenGenerator::replaceConverterTypeSystemVariable(TypeSystemConverterVa
                 QString arg = getConverterTypeSystemVariableArgument(code, match.capturedEnd());
                 conversionString += arg;
                 if (converterVariable == TypeSystemToPythonFunction && !isVariable(arg)) {
-                    qFatal("Only variables are acceptable as argument to %%CONVERTTOPYTHON type system variable on code snippet: '%s'",
-                           qPrintable(code));
+                    QString m;
+                    QTextStream(&m) << "Only variables are acceptable as argument to %%CONVERTTOPYTHON type system variable on code snippet: '"
+                        << code << '\'';
+                    throw Exception(m);
                 }
                 if (conversion.contains(QLatin1String("%in"))) {
                     conversion.prepend(QLatin1Char('('));
@@ -2430,8 +2433,10 @@ void ShibokenGenerator::collectContainerTypesFromConverterMacros(const QString &
             if (type.has_value()) {
                 addInstantiatedContainersAndSmartPointers(type.value(), type->originalTypeDescription());
             } else {
-                qFatal("%s: Cannot translate type \"%s\": %s", __FUNCTION__,
-                       qPrintable(typeString), qPrintable(errorMessage));
+                QString m;
+                QTextStream(&m) << __FUNCTION__ << ": Cannot translate type \""
+                    << typeString << "\": " << errorMessage;
+                throw Exception(m);
             }
         }
         start = end;
author	Friedemann Kleint <Friedemann.Kleint@qt.io>	2021-02-26 14:47:59 +0100
committer	Friedemann Kleint <Friedemann.Kleint@qt.io>	2021-03-10 08:48:07 +0100
commit	f7901b0b29e831a3799bd7fc39dbdf5e67271ae8 (patch)
tree	dfe67473b4bca93cb3eb7e2c7ca41ed121611175
parent	7cd7141e43e98d4862de3aadf128b0d0080090c9 (diff)