Created attachment 108193 [details] combined revert of commits #fc510adaecf1dc83416eaf4392925ee4f3c5a1e0 and #878797830dbd3b29bd5dcd8241c17ea20fb6914e (in that order). The migration from QtScript to QtQML triggered a bug in QtQML versions < 5.9.x which causes a severe regression for anyone who cannot upgrade to a Qt version in which the issue has been fixed. I'm thinking of certain Mac users in particular: Qt 5.9 runs only on 10.10 and newer, locking in certain Mac ranges to 5.8 or earlier. In short, the QtScript or QtQML question is akin to QtWebKit or QtWebEngine, and merits a build option to make KTextEditor build against QtScript - if there is no way to work around the issue in a less invasive way like tweaking the function called when hitting Enter and that triggers the crash. In the meantime affected users can apply the attached patch (applies cleanly to 5.38.0 and git/master).
Sorry Rene, but this is not a solution we can maintain. If you want to maintain a patch yourself for using QtScript, that's fine. But it does not make much sense to share this here as bug report. I'll close as invalid, since the particular bugs are tracked in other reports (and also being fixed there).
Exactly what's being fixed (and where)? If there's a fix for the upstream bug that doesn't involve doing the impossible I'd love to know it. Keeping the old code afloat isn't an option (in my experience that only becomes a burden when it's become just about impossible) AND going back to a solution that just worked on all platforms isn't an option either? Then you could at least introduce a workaround that makes the component(s) triggering the bug optional (supposing you can't be more specific and disable only the scripts triggering the bug). A somewhat crippled editor component is better than no editor at all, or one that crashes randomly. It may not have occurred to you, but forcing the same version number on all frameworks also means that one burnt bean ruins the whole pot. There's a reason why the minimum Qt version number is being kept as low as possible, you're essentially overriding that here for all frameworks.
We can't maintain both variants. That old Qt versions have evil bugs hidden inside the script engine is bad, but we waited for "years" after the initial V4 release before we ported. And now QtScript is really "dead" and we need to switch. To be honest: On most distros you will not get today's frameworks but no new Qt. I can understand that this is not nice, but realistic.
I agree that the code doesn't appear to be designed to swap backends easily (like the rendering backend in KDevelop's documentation feature, for instance). It looks like the transition could have been prepared by first introducing some sort of backend interface class (apparently you had "years" to do that) but you probably realised that yourselves by now. I don't get what's dead about QtScript that would oblige you to switch. It's still in recent Qt versions, and it's not like you're using it for rocket science or security related things here. I guess I'll see what happens when I upgrade to 5.40.0 or so, but 5.38.0 still works perfectly fine with QtScript (= that need to switch isn't so urgent at all). Anyway, I guess I'll end up spoofing an newer version number on patched old KTE code at some point, crossing my fingers that none of the applications I rely on start requiring 5.38+ during my lifetime (or until I get to upgrade all my systems to Qt 5.9+).
> I don't get what's dead about QtScript that would oblige you to switch. QtScript is deprecated since Qt 5.5. It is only still included because it has some features that are not available in QtQML. http://wiki.qt.io/New-Features-in-Qt-5.5
Rene, if you want to invest your time effectively, fix the remaining bugs in the current implementation: Look into the indentation scripts and see what line exactly crashes, and then find a workaround. Everything else is wasted time.
I agree that would be the proper approach. It's what I tried to do initially, when I noticed that I know exactly how to trigger the crash (pretty certain I described that in the bug I filed, too). But I couldn't find any scripts, nor does the backtrace give me any indication about a script line or expression that crashes. If somebody can give me some pointers how to figure this out I'll be happy to give it a try (preferably without having to build a full-debug Qt install though ;) )
See: https://docs.kde.org/trunk5/en/applications/katepart/dev-scripting.html Scripts on disk have higher priority that scripts compiled into the katepart library. That means: If you copy the cstyle.js indenter into your $XDG_DATA_HOME/katepart5/script/indentation/cstyle.js, it should be picked up. To be sure, increase the revision of the cstyle.js file in the json header. Then, you can use debug("string") to dump text to the console. Additionally, you can set the variable in var debugMode = false; to true to get more debug output, see https://github.com/KDE/ktexteditor/blob/master/src/script/data/indentation/cstyle.js The same holds also for other indenters that crash KTextEditor. I think some combination with CSS was also an issue. Does that help?
Yes, thanks. That should at least help in identifying where things go wrong.
(In reply to Dominik Haumann from comment #8) > To be sure, increase the revision of the cstyle.js file in the json > header. Sorry, which json header would that be? https://bugreports.qt.io/browse/QTBUG-63045 now presents a workaround implemented by Gentoo: disabling the QML JIT compiler. That seems a bit overkill (performance implications?), but could be a viable workaround if it can be done transiently just where/when necessary. I haven't yet look if that's the case. Come to think of it, maybe that in this case it's not the JIT compiler that causes the crash, but the generator that translates the script into C++ . In that case I'd just have to figure out how to install the .js file in a system-wide location that's takes precedence over the builtin code.
The json header in the cstyle.js file itself.
> Come to think of it, maybe that in this case it's not the JIT compiler that causes the crash, but the generator that translates the script into C++ . In that case I'd just have to figure out how to install the .js file in a system-wide location that's takes precedence over the builtin code. There is no such generated, the only difference is that bundled files are put into qresources. But that has zero impact on the later parsing/execution
Ok, so you're not doing exactly as described at http://doc.qt.io/QtQuickCompiler/? (I only took a casual look, to me this falls under "everything you never wanted to know but were not too afraid to ask" ... if you get the pun ;))
No, we don't use a QtQuickCompiler.
@Rene: Any news? As said, we don't use any Qt Quick Compiler. To drive this forward, simply copy the cstyle.js file into the respective file and add debugging output. This essentially is printf-debugging. Don't make it more complicated, you have all details you need to go forward here. Also you don't need any backtraces into Qt, no debug build is required. That said, you can start right away as is, without any modifications and what not.
Sorry, I just haven't had time. Hopefully next week.
Created attachment 108500 [details] test cstyle.js I had some time over my morning coffee. When using QtScript and just the stock cstyle.js with debug mode activated I get output lines for tryComment and tryStatement. When using QML, I only get the tryComment debug output, and none of the probes I added print, including the one in indent(). Indent() seems to be the module's entry point (note that I know nothing of javascript). If that's correct, I'll probably need to look at another file?
Created attachment 108501 [details] test cstyle.js Doh, my bad, I didn't modify the right version of the file. (I had tried first in $prefix/share/katepart5/script/indentation, not realising it's overridden by the copy in the resource. I think that should be the opposite, btw., builtin copies should always be the fallback option). I also should have waited until after I discovered and tried the QV4_FORCE_INTERPRETER env. var. Gentoo's trick of disabling the JIT works, and can also be obtained with said env. var. That was helpful getting my probes to work... and could possibly be used as a workaround when building against affected Qt versions (set and restore the env.var around calls into the QML script engine, if all else fails). Here's what I'm seeing with the current cstyle.js when I hit enter at the end of a function's final line that says `return decl;` : indent line=84 indentWidth=4 ch= indent linenr=850 indent linenr=852 indent linenr= indentLine line=84 linenr=731 isComment: Check mode @ Cursor(83,4): ISO C++:Control Flow Process 26907 stopped * thread #1, name = 'kate', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
And now you have to add more debug("xyz...") statements in the js file to identify exactly the line of the crash. Which line in the js code crashes Qt?
Created attachment 108507 [details] test cstyle.js Prints indent line=84 indentWidth=4 ch= indent linenr=880 indent linenr=882 indent linenr=886 indentLine line=84 linenr=746 isComment: Check mode @ Cursor(83,4): ISO C++:Control Flow tryCComment linenr=292 indentLine filler=-1 linenr=757 indentLine filler=-1 linenr=761 indentLine filler=-1 linenr=765 indentLine filler=-1 linenr=769 indentLine filler=-1 linenr=773 indentLine filler=-1 linenr=777 tryCondition currentLine=83 linenr=488 tryCondition currentString=" return decl;" lastPos=15 lastChar=; linenr=495 tryCondition currentIndentation=4 linenr=509 tryCondition decr currentLine=83 lineDelimiter=10 linenr=513 tryCondition firstPosVirtual=4 linenr=519 tryCondition decr currentLine=82 lineDelimiter=9 linenr=513 tryCondition firstPosVirtual=8 linenr=519 tryCondition decr currentLine=81 lineDelimiter=8 linenr=513 tryCondition firstPosVirtual=12 linenr=519 tryCondition decr currentLine=80 lineDelimiter=7 linenr=513 tryCondition firstPosVirtual=8 linenr=519 tryCondition decr currentLine=79 lineDelimiter=6 linenr=513 tryCondition firstPosVirtual=8 linenr=519 tryCondition decr currentLine=78 lineDelimiter=5 linenr=513 tryCondition firstPosVirtual=8 linenr=519 tryCondition decr currentLine=77 lineDelimiter=4 linenr=513 tryCondition firstPosVirtual=8 linenr=519 tryCondition decr currentLine=76 lineDelimiter=3 linenr=513 tryCondition firstPosVirtual=4 linenr=519 tryCondition decr currentLine=75 lineDelimiter=2 linenr=513 tryCondition firstPosVirtual=4 linenr=519 tryCondition decr currentLine=74 lineDelimiter=1 linenr=513 tryCondition firstPosVirtual=4 linenr=519 Process 28235 stopped * thread #1, name = 'kate', stop reason = signal SIGSEGV: invalid address (fault address: 0x0) The crashing line would be either the `if (firstPosVirtual < currentIndentation)` expression, or else something happens unwinding the stack when coming out of the while loop. If it helps, this is the C++ function in which I'm testing (lines 66-85): ``` 66 Declaration* usefulDeclaration(Declaration* decl) 67 { 68 if (!decl) 69 return nullptr; 70 71 // First: Attempt to find the declaration of a definition 72 decl = DUChainUtils::declarationForDefinition(decl); 73 74 // Convenience feature: Retrieve the type declaration of instances, 75 // it makes no sense to pass the declaration pointer of instances of types 76 if (decl->kind() == Declaration::Instance) { 77 AbstractType::Ptr type = TypeUtils::targetTypeKeepAliases(decl->abstractType(), decl->topContext()); 78 IdentifiedType* idType = dynamic_cast<IdentifiedType*>(type.data()); 79 Declaration* idDecl = idType ? idType->declaration(decl->topContext()) : nullptr; 80 if (idDecl) { 81 decl = idDecl; 82 } 83 } 84 return decl; 85 } ```
> The crashing line would be either the `if (firstPosVirtual < > currentIndentation)` expression, or else something happens unwinding the > stack when coming out of the while loop. Probably the latter: decreasing lineDelimiter doesn't prevent the crash. Would there be any possibility to include/access node.js features from KTE scripts, so we can deactivate the V4 JIT from the QML code itself, where and when needed? (https://stackoverflow.com/questions/4870328/read-environment-variables-in-node-js)
Using a for instead of a while also crashes: // var lineDelimiter = 10; // 10 limit search, hope this is a sane value for ( var lineDelimiter = 9 ; currentLine > 0 && lineDelimiter >= 0 ; --lineDelimiter) { dbg("tryCondition decr currentLine=" + currentLine + " lineDelimiter=" + lineDelimiter + " linenr=" + (new Error()).lineNumber); --currentLine; // --lineDelimiter;
Created attachment 108512 [details] alternative workaround patch This is a much easier (maintainable) workaround pending a proper solution. It disables QML's V4 JIT for all KTextEditor scripts when using an older Qt version. Care is taken to restore the environment value. This is as fine-grained a control over the JIT compiler that I've been able to obtain.
Created attachment 108514 [details] polished up workaround patch Qt prints a single warning when the JIT is disabled; print a single explanation under this warning.
Created attachment 108516 [details] tryCondition() version that seems to avoid the stack unwind crash I tried a bit of a kludge to avoid popping the "while stack" just before returning from the function. Lo and behold, returning from within the while loop seems to something sufficiently differently internally that the crash is avoided. That "break" does cause processing to continue after the while, right? Leaving the break in place and just adding the `else if` check of the 2 loop variables works too, btw.
If this patch works, I'm all for it. Could anyone else test this already? René, could you post this on Phabricator?
Done: https://phabricator.kde.org/D8544
Git commit e427f6d4bdf95d426b6ba9c49114dcb09af0d6e0 by René J.V. Bertin. Committed on 03/11/2017 at 12:35. Pushed by rjvbb into branch 'master'. Avoid (certain) crashes while executing QML scripts Certain of KTextEditor's indentation scripts trigger a bug in Qt versions before 5.9.1 . Disabling the QML JIT compiler for those Qt versions prevents that. Differential Revision: https://phabricator.kde.org/D8544 M +10 -0 src/utils/kateglobal.cpp https://commits.kde.org/ktexteditor/e427f6d4bdf95d426b6ba9c49114dcb09af0d6e0