Bug 337785 - Haskell intendation segmentation fault
Summary: Haskell intendation segmentation fault
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: indentation (show other bugs)
Version: 3.13.2
Platform: Kubuntu Linux
: NOR crash
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-25 02:12 UTC by womgli
Modified: 2014-11-09 15:55 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.14.1


Attachments
New crash information added by DrKonqi (3.85 KB, text/plain)
2014-07-26 13:05 UTC, Martin Walch
Details
proposal for a fix by adding one default KateHlContext (1.97 KB, patch)
2014-08-10 20:38 UTC, Martin Walch
Details

Note You need to log in before you can comment on or make changes to this bug.
Description womgli 2014-07-25 02:12:37 UTC
I can reproduce a kate crash by
* setting intendation to spaces
* creating a new document and selecting haskell intendation
* typing

case a of[ENTER]
| b[ENTER]

Notice the space between "|" and "b" as it is required to reproduce.

Stacktrace:
===========

Application: Kate (kate), signal: Segmentation fault
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[Current thread is 1 (Thread 0x7f1bd5d887c0 (LWP 4400))]

Thread 3 (Thread 0x7f1bc0bf1700 (LWP 4402)):
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x00007f1bc2302ffb in QTWTF::TCMalloc_PageHeap::scavengerThread (this=0x7f1bc2600f00 <QTWTF::pageheap_memory>) at ../3rdparty/javascriptcore/JavaScriptCore/wtf/FastMalloc.cpp:2359
#2  0x00007f1bc2303039 in QTWTF::TCMalloc_PageHeap::runScavengerThread (context=<optimized out>) at ../3rdparty/javascriptcore/JavaScriptCore/wtf/FastMalloc.cpp:1464
#3  0x00007f1bd2ce7182 in start_thread (arg=0x7f1bc0bf1700) at pthread_create.c:312
#4  0x00007f1bd56d730d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Thread 2 (Thread 0x7f1b3da78700 (LWP 4403)):
#0  0x00007f1bd56c86bd in read () at ../sysdeps/unix/syscall-template.S:81
#1  0x00007f1bd2848c20 in read (__nbytes=16, __buf=0x7f1b3da77be0, __fd=<optimized out>) at /usr/include/x86_64-linux-gnu/bits/unistd.h:44
#2  g_wakeup_acknowledge (wakeup=0xa913c0) at /build/buildd/glib2.0-2.40.0/./glib/gwakeup.c:210
#3  0x00007f1bd2807b14 in g_main_context_check (context=context@entry=0x7f1b380009a0, max_priority=2147483647, fds=fds@entry=0x7f1b38003480, n_fds=n_fds@entry=1) at /build/buildd/glib2.0-2.40.0/./glib/gmain.c:3532
#4  0x00007f1bd2807f7b in g_main_context_iterate (context=context@entry=0x7f1b380009a0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at /build/buildd/glib2.0-2.40.0/./glib/gmain.c:3731
#5  0x00007f1bd28080ec in g_main_context_iteration (context=0x7f1b380009a0, may_block=1) at /build/buildd/glib2.0-2.40.0/./glib/gmain.c:3795
#6  0x00007f1bd35e77be in QEventDispatcherGlib::processEvents (this=0x7f1b380008c0, flags=...) at kernel/qeventdispatcher_glib.cpp:436
#7  0x00007f1bd35b90af in QEventLoop::processEvents (this=this@entry=0x7f1b3da77de0, flags=...) at kernel/qeventloop.cpp:149
#8  0x00007f1bd35b93a5 in QEventLoop::exec (this=this@entry=0x7f1b3da77de0, flags=...) at kernel/qeventloop.cpp:204
#9  0x00007f1bd34b5c5f in QThread::exec (this=this@entry=0x1657770) at thread/qthread.cpp:537
#10 0x00007f1bd359a823 in QInotifyFileSystemWatcherEngine::run (this=0x1657770) at io/qfilesystemwatcher_inotify.cpp:265
#11 0x00007f1bd34b832f in QThreadPrivate::start (arg=0x1657770) at thread/qthread_unix.cpp:349
#12 0x00007f1bd2ce7182 in start_thread (arg=0x7f1b3da78700) at pthread_create.c:312
#13 0x00007f1bd56d730d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Thread 1 (Thread 0x7f1bd5d887c0 (LWP 4400)):
[KCrash Handler]
#6  KateDocument::defStyleNum (this=0xec64f0, line=<optimized out>, column=<optimized out>) at ../../part/document/katedocument.cpp:5487
#7  0x00007f1bc297df19 in KateScriptDocument::defStyleNum (this=this@entry=0x1570260, line=<optimized out>, column=<optimized out>) at ../../part/script/katescriptdocument.cpp:51
#8  0x00007f1bc297f819 in KateScriptDocument::isCode (this=0x1570260, line=<optimized out>, column=<optimized out>) at ../../part/script/katescriptdocument.cpp:62
#9  0x00007f1bc28fa41d in KateScriptDocument::qt_static_metacall (_o=0x1570260, _id=-745404096, _id@entry=86, _a=0x7ffff11ebfd0, _c=<optimized out>) at moc_katescriptdocument.cpp:397
#10 0x00007f1bc28fb413 in qt_static_metacall (_a=0x7ffff11ebfd0, _id=86, _c=QMetaObject::InvokeMetaMethod, _o=0x1570260) at moc_katescriptdocument.cpp:466
#11 KateScriptDocument::qt_metacall (this=0x1570260, _c=QMetaObject::InvokeMetaMethod, _id=86, _a=0x7ffff11ebfd0) at moc_katescriptdocument.cpp:467
#12 0x00007f1bc236eb32 in QScript::callQtMethod (exec=exec@entry=0x7f1b36d45130, callType=callType@entry=QMetaMethod::Method, thisQObject=thisQObject@entry=0x1570260, scriptArgs=..., meta=meta@entry=0x7f1bc2d19fc0 <KateScriptDocument::staticMetaObject>, initialIndex=91, maybeOverloaded=true) at bridge/qscriptqobject.cpp:960
#13 0x00007f1bc236fc59 in QScript::QtFunction::execute (this=this@entry=0x7f1b3c045300, exec=0x7f1b36d45130, thisValue=..., thisValue@entry=..., scriptArgs=...) at bridge/qscriptqobject.cpp:1015
#14 0x00007f1bc236fee9 in QScript::QtFunction::call (exec=0x7f1b36d45130, callee=0x7f1b3c045300, thisValue=..., args=...) at bridge/qscriptqobject.cpp:1030
#15 0x00007f1bc2275118 in QTJSC::NativeFuncWrapper::operator() (this=this@entry=0x7ffff11ec200, exec=0x7f1b36d45130, jsobj=jsobj@entry=0x7f1b3c045300, thisValue=..., argList=...) at ../3rdparty/javascriptcore/JavaScriptCore/runtime/CallData.cpp:46
#16 0x00007f1bc2252fc0 in QTJSC::cti_op_call_NotJSFunction (args=0x7ffff11ec250) at ../3rdparty/javascriptcore/JavaScriptCore/jit/JITStubs.cpp:1780
#17 0x00007f1b40389245 in ?? ()
#18 0x00007f1bd0020020 in ?? ()
#19 0x00007f1b3c045300 in ?? ()
#20 0x00007f1b0000001a in ?? ()
#21 0x0000000000000003 in ?? ()
#22 0x00000000018d4110 in ?? ()
#23 0x0000000700000001 in ?? ()
#24 0x00000000012fcb30 in ?? ()
#25 0x0000000000000000 in ?? ()
Comment 1 Martin Walch 2014-07-26 13:05:06 UTC
Created attachment 87960 [details]
New crash information added by DrKonqi

kate (3.13.3) on KDE Platform 4.13.3 using Qt 4.8.5

I can confirm this crash.

This stacktrace should help getting more insight.

What is not visible in the stack: the invocation of document.isCode is in the haskell indenter haskell.js in line 337:

document.isCode(line - 1, document.lineLength(lastLine) - 1)

Taking a closer look soon.

-- Backtrace (Reduced):
#6  0x00007fa79eb682b3 in KateDocument::defStyleNum (this=0x1a23330, line=1, column=8) at /var/tmp/portage/kde-base/katepart-4.13.3/work/katepart-4.13.3/part/document/katedocument.cpp:5487
#7  0x00007fa79eb89492 in KateScriptDocument::defStyleNum (this=0x206a520, line=1, column=8) at /var/tmp/portage/kde-base/katepart-4.13.3/work/katepart-4.13.3/part/script/katescriptdocument.cpp:51
#8  0x00007fa79eb8950a in KateScriptDocument::isCode (this=0x206a520, line=1, column=8) at /var/tmp/portage/kde-base/katepart-4.13.3/work/katepart-4.13.3/part/script/katescriptdocument.cpp:62
#9  0x00007fa79ead7855 in KateScriptDocument::qt_static_metacall (_o=0x206a520, _c=QMetaObject::InvokeMetaMethod, _id=86, _a=0x7fff1f1c7150) at /var/tmp/portage/kde-base/katepart-4.13.3/work/katepart-4.13.3_build/part/moc_katescriptdocument.cpp:397
#10 0x00007fa79ead7f90 in KateScriptDocument::qt_metacall (this=0x206a520, _c=QMetaObject::InvokeMetaMethod, _id=86, _a=0x7fff1f1c7150) at /var/tmp/portage/kde-base/katepart-4.13.3/work/katepart-4.13.3_build/part/moc_katescriptdocument.cpp:467
Comment 2 Martin Walch 2014-07-26 17:27:57 UTC
In katedocument.cpp:5486, there is a bad call to highlight()->contextNum(0), when using no highlighting, as in that case the KateHighlighting object highlight() has no KateHlContext at all. I wonder whether a proper fix involves adding some "default" context to the "None" highlighter or not. Maybe I get to look into this tomorrow.
Comment 3 Christoph Cullmann 2014-07-26 18:08:25 UTC
Actually, even the "none" hl should have a default context:

 if (def == 0) {
        noHl = true;
        iName = QString::fromLatin1("None"); // not translated internal name (for config and more)
        iNameTranslated = i18nc("Syntax highlighting", "None"); // user visible name
        iSection = QString();
        makeNoneContext();
    }
Comment 4 Martin Walch 2014-07-28 00:11:05 UTC
(In reply to Christoph Cullmann from comment #3)
> Actually, even the "none" hl should have a default context:
> 
>  if (def == 0) {
>         noHl = true;
>         iName = QString::fromLatin1("None"); // not translated internal name
> (for config and more)
>         iNameTranslated = i18nc("Syntax highlighting", "None"); // user
> visible name
>         iSection = QString();
>         makeNoneContext();
>     }

As far as I see, this never adds anything to m_contexts. Any code path that would add anything to m_contexts prevents this with

if (noHl)
  return;

However, KateHighlighting::contextNum(int n) expects that m_contexts.size() > 0. (Maybe "Q_ASSERT (0);" should be changed to something like that?)

However, I do not fully understand what the code as a whole is doing, so I am not sure how to properly fix it. My guess is that one could create a minimal KateHlContext and extend KateHighlighting::makeNoneContext() to put this KateHlContext to m_contexts[0].
Comment 5 Christoph Cullmann 2014-07-28 06:19:42 UTC
makeNoneContext() should add the dummy 0 context.
Comment 6 Martin Walch 2014-07-28 09:41:22 UTC
(In reply to Christoph Cullmann from comment #5)
> makeNoneContext() should add the dummy 0 context.

You mean like m_contexts.push_back(0)?

This would be dereferenced (i.e. segfaulting) at katedocument.cpp:5487

attribute = context->attr;

This could probably be solved with a null check. However, there might be other places with the same problem. I think having a "real" KateHlContext object would be a better solution. That object could be completely defined at compile time.
Comment 7 Martin Walch 2014-08-10 20:38:01 UTC
Created attachment 88208 [details]
proposal for a fix by adding one default KateHlContext

This patch adds a minimalistic KateHlContext to m_contexts in makeNoneContext().

With this patch applied, I cannot reproduce the crash any more.

(as far as I understand the code, freeing the memory of the new KateHlContext object already happens in other KateHighlighting methods)

I would like to hear some opinions about it. Should I add it directly to the review board?
Comment 8 Joseph Wenninger 2014-08-11 14:36:16 UTC
I think the patch should be fine, please commit
Comment 9 Martin Walch 2014-08-12 00:02:18 UTC
Now on review board:

> https://git.reviewboard.kde.org/r/119724/
Comment 10 Dominik Haumann 2014-08-13 19:48:44 UTC
The patch looks good, but as I understand, the origin of the crash comes from the fact that the Haskell indenter haskell.js *requires* haskell highlighting.

If the highlighting is not Haskell, then the haskell indenter shouldn't be chosable at all.

The correct fix for this is:
Add in haskell.js a style, say "
<language ... style="haskell">

And then in haskell.js add to the header:
required-syntax-style: haskell

Martin, can you confirm this would also fix the crash?
Comment 11 Dominik Haumann 2014-08-13 19:56:29 UTC
Git commit 5144dfeb7c5cc6575ab3f7b63eb32c00134c51ff by Dominik Haumann.
Committed on 13/08/2014 at 19:53.
Pushed by dhaumann into branch 'master'.

fix crash: Allow Haskell indenter only if the Highlighting provides the "haskell" style

M  +3    -2    src/script/data/indentation/haskell.js
M  +1    -1    src/syntax/data/haskell.xml

http://commits.kde.org/ktexteditor/5144dfeb7c5cc6575ab3f7b63eb32c00134c51ff
Comment 12 Martin Walch 2014-08-13 20:29:08 UTC
(In reply to Dominik Haumann from comment #10)
> The correct fix for this is:
> Add in haskell.js a style, say "
> <language ... style="haskell">
> 
> And then in haskell.js add to the header:
> required-syntax-style: haskell
> 
> Martin, can you confirm this would also fix the crash?

Basically yes. But at least for KDE/4.14 it must be

style="Haskell"

in haskell.xml and

* required-syntax-style: Haskell

in haskell.js. Note the capitalized H which is mandatory because the name is also capitalized.

With that, it is not possible any more to select the Haskell indenter without Haskell highlighting in Kate 3.13.3, and hence the crash cannot be triggered that way any more.

However I see two drawbacks:

* It prevents the Curry highlighter from using the Haskell indenter.
* I expect other indenters to suffer from the same problem. We just have not seen other crash reports submitted, yet. They should all be fixed.
Comment 13 Christoph Cullmann 2014-08-14 21:37:38 UTC
Git commit 362217788eecab105140f2790ce7a81506f2de58 by Christoph Cullmann.
Committed on 14/08/2014 at 21:36.
Pushed by cullmann into branch 'master'.

add dummy context, always

M  +10   -7    src/syntax/katehighlight.cpp

http://commits.kde.org/ktexteditor/362217788eecab105140f2790ce7a81506f2de58
Comment 14 Christoph Cullmann 2014-08-14 21:43:41 UTC
Git commit 1876a1939df5c196e0c9afb05e4182b3859e0e47 by Christoph Cullmann.
Committed on 14/08/2014 at 21:42.
Pushed by cullmann into branch 'KDE/4.14'.

KateHighlighting is expected to always have at least one default KateHlContext (stored in m_contexts), but with highlighting turned off, there is no such context.

As a fix, add a minimalistic KateHlContext to m_contexts in makeNoneContext.

Also fix two typos ("falltrhough", "resolove") in code comments when touching part/syntax/katehighlight.cpp anyway.

REVIEW: 119724

M  +11   -2    part/syntax/katehighlight.cpp

http://commits.kde.org/kate/1876a1939df5c196e0c9afb05e4182b3859e0e47
Comment 15 Christoph Cullmann 2014-08-14 21:44:19 UTC
Git commit b739b7a67882408b4378d901c38b2c88108f1312 by Christoph Cullmann.
Committed on 14/08/2014 at 21:42.
Pushed by cullmann into branch 'KDE/4.13'.

KateHighlighting is expected to always have at least one default KateHlContext (stored in m_contexts), but with highlighting turned off, there is no such context.

As a fix, add a minimalistic KateHlContext to m_contexts in makeNoneContext.

Also fix two typos ("falltrhough", "resolove") in code comments when touching part/syntax/katehighlight.cpp anyway.

REVIEW: 119724

M  +11   -2    part/syntax/katehighlight.cpp

http://commits.kde.org/kate/b739b7a67882408b4378d901c38b2c88108f1312
Comment 16 Christoph Cullmann 2014-08-14 21:47:27 UTC
Do we need to do things with the hl/js file again?
The context 0 fix is now in all branches, I hope, a bit different in ktexteditor.git master
Comment 17 Martin Walch 2014-08-15 03:19:01 UTC
(In reply to Christoph Cullmann from comment #16)
> Do we need to do things with the hl/js file again?
> The context 0 fix is now in all branches, I hope, a bit different in
> ktexteditor.git master

Thanks for pushing the patches.

With the style="haskell" attribute in the Haskell indenter, it is not possible to select the Haskell indenter together with any other highlighting any more, is it? This affects in particular the "Curry" and the "None" highlighting.

So, I would just remove that attribute from haskell.js again.
Comment 18 Dominik Haumann 2014-08-15 12:04:01 UTC
Well, what's important to understand is that the string in "style=" does not have to match the indenter or hl name. Example:
haskell.js could say: required-syntax-style=abcd
haskell.xml could say: style="abcd"
curry.xml could say: style="abcd"

And then you can choose the Haskell indenter again with Curry. Would that help already?

The idea behind the "style" attribute is that indenters that rely on highlighting information won't work without highlighting.
Comment 19 Martin Walch 2014-08-16 15:02:33 UTC
(In reply to Dominik Haumann from comment #18)
> Well, what's important to understand is that the string in "style=" does not
> have to match the indenter or hl name. Example:
> haskell.js could say: required-syntax-style=abcd
> haskell.xml could say: style="abcd"
> curry.xml could say: style="abcd"

Thank you for this explanation. Makes things a lot clearer. I got this totally wrong, because there was some cruft in my ~/.kde4 folder, causing strange effects like the thing about needing to capitalize the "H" of "haskell" in comment #12.

> And then you can choose the Haskell indenter again with Curry. Would that
> help already?

Yes, this would definitely help.

> The idea behind the "style" attribute is that indenters that rely on
> highlighting information won't work without highlighting.

In the particular case of the haskell indenter, it looks like it also works mostly with highlighting completely turned off. I guess one can argue against as well as for limiting the Haskell indenter to Haskell and Curry highlighting.

Where I still see some conflicts are the Literate Curry and Literate Haskell modes. As far as I see, the haskell indenter yields bad results with those two highlighters, but I do not know enough about Haskell programming to know for sure. However, I guess there should be a clear decision whether to use the haskell indenter with the Literate modes or not. Currently, literate-curry.xml contains the attribute indenter="haskell" while literate-haskell does not (and none of them contains a style="..." attribute).
Comment 20 Christoph Cullmann 2014-08-17 19:29:44 UTC
Actually I think just allowing the indenter to work for any language is ok, if it doesn't crash, which is now fixed.
Will just revert the syntax/js change.
Comment 21 Christoph Cullmann 2014-08-17 19:32:16 UTC
Git commit c1ac3e1913df556b54646eb1efc2aaa13379bb95 by Christoph Cullmann.
Committed on 17/08/2014 at 19:30.
Pushed by cullmann into branch 'master'.

don't check for style in js file

M  +2    -3    src/script/data/indentation/haskell.js

http://commits.kde.org/ktexteditor/c1ac3e1913df556b54646eb1efc2aaa13379bb95
Comment 22 P. Christeas 2014-11-08 20:48:11 UTC
Please, can you backport to KDE SC 4.12 ?

Had the same issue, with a non-highlighted file. Whenever the cursor would reach the end of line, after some auto-completion, editor would crash.

Just cherry-picking 44beb7d342309 and b739b7a67882408b43 fixed my 4.12 build.
Comment 23 Dominik Haumann 2014-11-09 14:04:41 UTC
Git commit c2a1c24f28613342985aa40573fb922370900a3a by Dominik Haumann, on behalf of Christoph Cullmann.
Committed on 14/08/2014 at 21:42.
Pushed by dhaumann into branch 'KDE/4.12'.

KateHighlighting is expected to always have at least one default KateHlContext (stored in m_contexts), but with highlighting turned off, there is no such context.

As a fix, add a minimalistic KateHlContext to m_contexts in makeNoneContext.

Also fix two typos ("falltrhough", "resolove") in code comments when touching part/syntax/katehighlight.cpp anyway.

REVIEW: 119724

M  +11   -2    part/syntax/katehighlight.cpp

http://commits.kde.org/kate/c2a1c24f28613342985aa40573fb922370900a3a
Comment 24 Dominik Haumann 2014-11-09 14:05:34 UTC
> Just cherry-picking 44beb7d342309 and b739b7a67882408b43 fixed my 4.12 build.

Done. There probably will not be a KDE 4.12.6, though... So the update will most probably not arrive in distributions.
Comment 25 P. Christeas 2014-11-09 15:55:33 UTC
(In reply to Dominik Haumann from comment #24)
> Done. There probably will not be a KDE 4.12.6, though... So the update will
> most probably not arrive in distributions.

Thank you, I have filed a bug in my distro about that.