Bug 404820 - default construction SyntaxHighlighter() crashes
Summary: default construction SyntaxHighlighter() crashes
Status: RESOLVED FIXED
Alias: None
Product: frameworks-syntax-highlighting
Classification: Frameworks and Libraries
Component: framework (show other bugs)
Version: 5.55.0
Platform: Other All
: NOR crash
Target Milestone: ---
Assignee: KWrite Developers
URL: https://codereview.qt-project.org/#/c...
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-25 22:19 UTC by Jos van den Oever
Modified: 2019-02-27 20:47 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jos van den Oever 2019-02-25 22:19:31 UTC
SUMMARY

SyntaxHighlighter(QObject* parent = nullptr) calls QSyntaxHighlighter(parent).

If parent == nullptr, this crashes because QSyntaxHighlighter(QObject *parent) dereferences parent without checking for nullptr. This could be considered a bug in QSyntaxHighlighter. However, QSyntaxHighlighter does not have a default argument nullptr but SyntaxHighlighter does.

I do not see a way to fix SyntaxHighlighter so that SyntaxHighlighter() can be called without crashing. Ideally, one would do this:

SyntaxHighlighter::SyntaxHighlighter(QObject* parent) :
    QSyntaxHighlighter(parent ?QSyntaxHighlighter(parent) :QSyntaxHighlighter((QTextDocument*)0)),
    AbstractHighlighter(new SyntaxHighlighterPrivate)
{
    qRegisterMetaType<QTextBlock>();
}

but that does not compile.

Is there a way to choose what superclass constructor to call at runtime?
Comment 1 Dominik Haumann 2019-02-26 06:42:46 UTC
No there is not.

This should be fixed upstream. Relevant code upstream:
https://code.woboq.org/qt5/qtbase/src/gui/text/qsyntaxhighlighter.cpp.html#300
Comment 2 Volker Krause 2019-02-26 07:54:29 UTC
Sounds like the default to nullptr is wrong in our code then? Should be ok to remove as that only affects SC of code that will crash immediately anyway.
Comment 3 Dominik Haumann 2019-02-26 20:41:09 UTC
Proposed upstream patch for Qt 5.12: https://codereview.qt-project.org/#/c/254448/
Comment 4 Jos van den Oever 2019-02-27 20:17:14 UTC
Upstream patch and removing the '= nullptr' is enough to close this bug imo.
Comment 5 Dominik Haumann 2019-02-27 20:47:18 UTC
Removing the = nullptr is not required.
nullptr will be ok with the next Qt 5.12.x upgrade. Closing as fixed, see:
https://codereview.qt-project.org/#/c/254448/