From 01331893adf91a3bb96152b65dc113fd57cb8ed2 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Fri, 20 Nov 2020 14:46:10 +0100 Subject: [PATCH] Don't call virtual method from Exception c'tor In case a static s_shouldAssert is true, we were calling virtual description() method from the c'tor of base Exception class, which was reimplemented in subclasses. This could cause a crash, since reimplementations were using not initialized yet private fields. We fix it by de-virtualizing the description() method and instead we pass the description in overloaded, protected c'tors. Original c'tors still use the default description that was used for base Exception class. In addition we make all private fields const, and make showException method non-virtual, as it didn't have any overrides. Change-Id: I7ebae1809ab423e990276e500f9c0db569d10e2d Reviewed-by: Thomas Hartmann --- .../designercore/exceptions/exception.cpp | 51 ++++++++++++------- .../exceptions/invalidargumentexception.cpp | 29 ++++++++--- .../exceptions/invalididexception.cpp | 47 +++++++++-------- .../exceptions/invalidqmlsourceexception.cpp | 8 +-- .../exceptions/rewritingexception.cpp | 10 ++-- .../designercore/include/exception.h | 21 +++++--- .../include/invalidargumentexception.h | 11 +++- .../designercore/include/invalididexception.h | 5 -- .../include/invalidqmlsourceexception.h | 4 -- .../designercore/include/rewritingexception.h | 5 +- 10 files changed, 115 insertions(+), 76 deletions(-) diff --git a/src/plugins/qmldesigner/designercore/exceptions/exception.cpp b/src/plugins/qmldesigner/designercore/exceptions/exception.cpp index 37252f6358b..a3a9806f7e6 100644 --- a/src/plugins/qmldesigner/designercore/exceptions/exception.cpp +++ b/src/plugins/qmldesigner/designercore/exceptions/exception.cpp @@ -99,33 +99,50 @@ bool Exception::warnAboutException() #endif } -/*! - Constructs an exception. \a line uses the __LINE__ macro, \a function uses - the __FUNCTION__ or the Q_FUNC_INFO macro, and \a file uses - the __FILE__ macro. -*/ -Exception::Exception(int line, - const QByteArray &_function, - const QByteArray &_file) - : m_line(line), - m_function(QString::fromUtf8(_function)), - m_file(QString::fromUtf8(_file)) -{ #ifdef Q_OS_LINUX +static QString getBackTrace() +{ + QString backTrace; void * array[50]; int nSize = backtrace(array, 50); char ** symbols = backtrace_symbols(array, nSize); for (int i = 0; i < nSize; i++) - { - m_backTrace.append(QString("%1\n").arg(QLatin1String(symbols[i]))); - } + backTrace.append(QString("%1\n").arg(QLatin1String(symbols[i]))); free(symbols); + + return backTrace; +} #endif +QString Exception::defaultDescription(int line, const QByteArray &function, const QByteArray &file) +{ + return QString(QStringLiteral("file: %1, function: %2, line: %3")) + .arg(QString::fromUtf8(file), QString::fromUtf8(function), QString::number(line)); +} + +/*! + Constructs an exception. \a line uses the __LINE__ macro, \a function uses + the __FUNCTION__ or the Q_FUNC_INFO macro, and \a file uses + the __FILE__ macro. +*/ +Exception::Exception(int line, const QByteArray &function, const QByteArray &file) + : Exception(line, function, file, Exception::defaultDescription(line, function, file)) +{ } + +Exception::Exception(int line, const QByteArray &function, + const QByteArray &file, const QString &description) + : m_line(line) + , m_function(QString::fromUtf8(function)) + , m_file(QString::fromUtf8(file)) + , m_description(description) + #ifdef Q_OS_LINUX + , m_backTrace(getBackTrace()) + #endif +{ if (s_shouldAssert) { - qDebug() << description(); + qDebug() << Exception::description(); QTC_ASSERT(false, ;); Q_ASSERT(false); } @@ -152,7 +169,7 @@ void Exception::createWarning() const */ QString Exception::description() const { - return QString(QStringLiteral("file: %1, function: %2, line: %3")).arg(m_file, m_function, QString::number(m_line)); + return m_description; } /*! diff --git a/src/plugins/qmldesigner/designercore/exceptions/invalidargumentexception.cpp b/src/plugins/qmldesigner/designercore/exceptions/invalidargumentexception.cpp index 86ffd4bea72..d4182eeca27 100644 --- a/src/plugins/qmldesigner/designercore/exceptions/invalidargumentexception.cpp +++ b/src/plugins/qmldesigner/designercore/exceptions/invalidargumentexception.cpp @@ -35,6 +35,19 @@ argument. */ namespace QmlDesigner { +QString InvalidArgumentException::invalidArgumentDescription(int line, + const QByteArray &function, + const QByteArray &file, + const QByteArray &argument) +{ + if (QString::fromUtf8(function) == QLatin1String("createNode")) { + return QCoreApplication::translate("QmlDesigner::InvalidArgumentException", + "Failed to create item of type %1").arg(QString::fromUtf8(argument)); + } + + return Exception::defaultDescription(line, function, file); +} + /*! Constructs the exception for \a argument. \a line uses the __LINE__ macro, \a function uses the __FUNCTION__ or the Q_FUNC_INFO macro, and \a file uses @@ -44,17 +57,21 @@ InvalidArgumentException::InvalidArgumentException(int line, const QByteArray &function, const QByteArray &file, const QByteArray &argument) - : Exception(line, function, file), m_argument(QString::fromUtf8(argument)) + : InvalidArgumentException(line, function, file, argument, + invalidArgumentDescription(line, function, file, argument)) { createWarning(); } -QString InvalidArgumentException::description() const +InvalidArgumentException::InvalidArgumentException(int line, + const QByteArray &function, + const QByteArray &file, + const QByteArray &argument, + const QString &description) + : Exception(line, function, file, description) + , m_argument(QString::fromUtf8(argument)) { - if (function() == QLatin1String("createNode")) - return QCoreApplication::translate("QmlDesigner::InvalidArgumentException", "Failed to create item of type %1").arg(m_argument); - - return Exception::description(); + createWarning(); } /*! diff --git a/src/plugins/qmldesigner/designercore/exceptions/invalididexception.cpp b/src/plugins/qmldesigner/designercore/exceptions/invalididexception.cpp index 5e4e87712c0..92c75717e53 100644 --- a/src/plugins/qmldesigner/designercore/exceptions/invalididexception.cpp +++ b/src/plugins/qmldesigner/designercore/exceptions/invalididexception.cpp @@ -29,28 +29,40 @@ namespace QmlDesigner { -InvalidIdException::InvalidIdException(int line, - const QByteArray &function, - const QByteArray &file, - const QByteArray &id, - Reason reason) : - InvalidArgumentException(line, function, file, "id"), - m_id(QString::fromUtf8(id)) +static QString descriptionBasedOnReason(InvalidIdException::Reason reason) { - if (reason == InvalidCharacters) - m_description = QCoreApplication::translate("InvalidIdException", "Only alphanumeric characters and underscore allowed.\nIds must begin with a lowercase letter."); - else - m_description = QCoreApplication::translate("InvalidIdException", "Ids have to be unique."); + if (reason == InvalidIdException::InvalidCharacters) + return QCoreApplication::translate("InvalidIdException", + "Only alphanumeric characters and underscore allowed.\n" + "Ids must begin with a lowercase letter."); + + return QCoreApplication::translate("InvalidIdException", "Ids have to be unique."); +} + +static QString decorateDescriptionWithId(const QString &id, const QString &description) +{ + return QCoreApplication::translate("InvalidIdException", "Invalid Id: %1\n%2") + .arg(id, description); } InvalidIdException::InvalidIdException(int line, const QByteArray &function, const QByteArray &file, const QByteArray &id, - const QByteArray &description) : - InvalidArgumentException(line, function, file, "id"), - m_id(QString::fromUtf8(id)), - m_description(QString::fromUtf8(description)) + Reason reason) + : InvalidArgumentException(line, function, file, "id", + decorateDescriptionWithId(QString::fromUtf8(id), + descriptionBasedOnReason(reason))) +{ } + +InvalidIdException::InvalidIdException(int line, + const QByteArray &function, + const QByteArray &file, + const QByteArray &id, + const QByteArray &description) + : InvalidArgumentException(line, function, file, "id", + decorateDescriptionWithId(QString::fromUtf8(id), + QString::fromUtf8(description))) { createWarning(); } @@ -60,9 +72,4 @@ QString InvalidIdException::type() const return QLatin1String("InvalidIdException"); } -QString InvalidIdException::description() const -{ - return QCoreApplication::translate("InvalidIdException", "Invalid Id: %1\n%2").arg(m_id, m_description); -} - } diff --git a/src/plugins/qmldesigner/designercore/exceptions/invalidqmlsourceexception.cpp b/src/plugins/qmldesigner/designercore/exceptions/invalidqmlsourceexception.cpp index 398aade16a0..b063a71b89d 100644 --- a/src/plugins/qmldesigner/designercore/exceptions/invalidqmlsourceexception.cpp +++ b/src/plugins/qmldesigner/designercore/exceptions/invalidqmlsourceexception.cpp @@ -42,8 +42,7 @@ InvalidQmlSourceException::InvalidQmlSourceException(int line, const QByteArray &function, const QByteArray &file, const QByteArray &qmlSource) - : Exception(line, function, file), - m_qmlSource(QString::fromUtf8(qmlSource)) + : Exception(line, function, file, QString::fromUtf8(qmlSource)) { createWarning(); } @@ -56,9 +55,4 @@ QString InvalidQmlSourceException::type() const return QLatin1String("InvalidQmlSourceException"); } -QString InvalidQmlSourceException::description() const -{ - return m_qmlSource; -} - } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/exceptions/rewritingexception.cpp b/src/plugins/qmldesigner/designercore/exceptions/rewritingexception.cpp index a4c326acccd..3ed058e1868 100644 --- a/src/plugins/qmldesigner/designercore/exceptions/rewritingexception.cpp +++ b/src/plugins/qmldesigner/designercore/exceptions/rewritingexception.cpp @@ -31,8 +31,9 @@ RewritingException::RewritingException(int line, const QByteArray &function, const QByteArray &file, const QByteArray &description, - const QString &documentTextContent): - Exception(line, function, file), m_description(QString::fromUtf8(description)), m_documentTextContent(documentTextContent) + const QString &documentTextContent) + : Exception(line, function, file, QString::fromUtf8(description)) + , m_documentTextContent(documentTextContent) { createWarning(); } @@ -42,11 +43,6 @@ QString RewritingException::type() const return QLatin1String("RewritingException"); } -QString RewritingException::description() const -{ - return m_description; -} - QString RewritingException::documentTextContent() const { return m_documentTextContent; diff --git a/src/plugins/qmldesigner/designercore/include/exception.h b/src/plugins/qmldesigner/designercore/include/exception.h index ea9a5ac8768..d716b45b597 100644 --- a/src/plugins/qmldesigner/designercore/include/exception.h +++ b/src/plugins/qmldesigner/designercore/include/exception.h @@ -41,8 +41,8 @@ public: virtual ~Exception(); virtual QString type() const = 0; - virtual QString description() const; - virtual void showException(const QString &title = QString()) const; + QString description() const; + void showException(const QString &title = QString()) const; int line() const; QString function() const; @@ -55,11 +55,20 @@ public: static bool shouldAssert(); static bool warnAboutException(); +protected: + Exception(int line, + const QByteArray &function, + const QByteArray &file, + const QString &description); + static QString defaultDescription(int line, const QByteArray &function, const QByteArray &file); + QString defaultDescription(); + private: - int m_line; - QString m_function; - QString m_file; - QString m_backTrace; + const int m_line; + const QString m_function; + const QString m_file; + const QString m_description; + const QString m_backTrace; static bool s_shouldAssert; }; diff --git a/src/plugins/qmldesigner/designercore/include/invalidargumentexception.h b/src/plugins/qmldesigner/designercore/include/invalidargumentexception.h index ab85eea9bb4..2d1719fcdb0 100644 --- a/src/plugins/qmldesigner/designercore/include/invalidargumentexception.h +++ b/src/plugins/qmldesigner/designercore/include/invalidargumentexception.h @@ -40,8 +40,17 @@ public: QString type() const override; QString argument() const; - QString description() const override; +protected: + InvalidArgumentException(int line, + const QByteArray &function, + const QByteArray &file, + const QByteArray &argument, + const QString &description); + static QString invalidArgumentDescription(int line, + const QByteArray &function, + const QByteArray &file, + const QByteArray &argument); private: const QString m_argument; }; diff --git a/src/plugins/qmldesigner/designercore/include/invalididexception.h b/src/plugins/qmldesigner/designercore/include/invalididexception.h index c44ed1f4c74..25acc992bda 100644 --- a/src/plugins/qmldesigner/designercore/include/invalididexception.h +++ b/src/plugins/qmldesigner/designercore/include/invalididexception.h @@ -47,11 +47,6 @@ public: const QByteArray &description); QString type() const override; - QString description() const override; - -private: - QString m_id; - QString m_description; }; } diff --git a/src/plugins/qmldesigner/designercore/include/invalidqmlsourceexception.h b/src/plugins/qmldesigner/designercore/include/invalidqmlsourceexception.h index 8a207dfdeca..c3bc1e5460a 100644 --- a/src/plugins/qmldesigner/designercore/include/invalidqmlsourceexception.h +++ b/src/plugins/qmldesigner/designercore/include/invalidqmlsourceexception.h @@ -38,10 +38,6 @@ public: const QByteArray &qmlSource = QByteArray()); QString type() const override; - QString description() const override; - -private: - QString m_qmlSource; }; } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/include/rewritingexception.h b/src/plugins/qmldesigner/designercore/include/rewritingexception.h index 95b034b32ea..4342bd337fb 100644 --- a/src/plugins/qmldesigner/designercore/include/rewritingexception.h +++ b/src/plugins/qmldesigner/designercore/include/rewritingexception.h @@ -39,11 +39,10 @@ public: const QString &documentTextContent); QString type() const override; - QString description() const override; QString documentTextContent() const; + private: - QString m_description; - QString m_documentTextContent; + const QString m_documentTextContent; }; } // namespace QmlDesigner