Kate: New contributors, Valgrind, and a fixed bug
In the recent months Kate saw quite some contributions from developers other than “the usual kate developers”. This is really cool, areas include more work on python plugins, a lot of bug fixing and fine tuning in the vi input mode, an improved python indenter, and more unit tests. Especially one unit test turned out to be immensely useful and resulted in a fix for the infamous “rangesForLine” crash (about 40 times reported). We even decided to backport this to KDE 4.10.2, so KDevelopers and Kile LaTeXers please keep an eye open on whether this has any side effects!
Basically, the unit test provided us with a valgrind trace. Since valgrind is a very useful tool for all developers, we’ll have a closer look at the valgrind trace:
==10276== Invalid read of size 4
==10276== at 0x62F2116: Kate::TextCursor::lineInternal() const (katetextcursor.h:134)
==10276== by 0x62F9035: Kate::TextBlock::updateRange(Kate::TextRange*) (katetextblock.cpp:572)
==10276== by 0x62F8C57: Kate::TextBlock::mergeBlock(Kate::TextBlock*) (katetextblock.cpp:522)
==10276== by 0x62EFAA1: Kate::TextBuffer::balanceBlock(int) (katetextbuffer.cpp:494)
==10276== by 0x62EF1B2: Kate::TextBuffer::unwrapLine(int) (katetextbuffer.cpp:298)
==10276== by 0x637BB9C: KateBuffer::unwrapLines(int, int) (katebuffer.cpp:292)
==10276== by 0x6359DE9: KateDocument::editRemoveLines(int, int) (katedocument.cpp:1321)
==10276== by 0x6359AD6: KateDocument::editRemoveLine(int) (katedocument.cpp:1293)
==10276== by 0x6357841: KateDocument::removeLine(int) (katedocument.cpp:732)
==10276== by 0x6394C6A: KateScriptDocument::removeLine(int) (katescriptdocument.cpp:513)
==10276== Address 0x14353470 is 96 bytes inside a block of size 152 free’d
==10276== at 0x4C2A44B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10276== by 0x62FDE4D: Kate::TextRange::~TextRange() (katetextrange.cpp:66)
==10276== by 0x6470B1E: KateOnTheFlyChecker::deleteMovingRange(KTextEditor::MovingRange*) (ontheflycheck.cpp:535)
==10276== by 0x6470657: KateOnTheFlyChecker::rangeEmpty(KTextEditor::MovingRange*) (ontheflycheck.cpp:489)
==10276== by 0x62FE641: Kate::TextRange::checkValidity(int, int, bool) (katetextrange.cpp:196)
==10276== by 0x62F6C84: Kate::TextBlock::wrapLine(KTextEditor::Cursor const&) (katetextblock.cpp:168)
==10276== by 0x62EEF1D: Kate::TextBuffer::wrapLine(KTextEditor::Cursor const&) (katetextbuffer.cpp:231)
==10276== by 0x637BA6D: KateBuffer::wrapLine(KTextEditor::Cursor const&) (katebuffer.cpp:275)
==10276== by 0x635972E: KateDocument::editInsertLine(int, QString const&) (katedocument.cpp:1246)
==10276== by 0x6357690: KateDocument::insertLine(int, QString const&) (katedocument.cpp:706)
==10276== by 0x6394C32: KateScriptDocument::insertLine(int, QString const&) (katescriptdocument.cpp:508)
The trace tells us that somewhere in TextCursor::lineIntern() we read a variable/address that is invalid. In fact, we are reading a pointer called m_block that should be valid. In this case, as we know the pointer should be valid, we can conclude that the entire TextCursor object is then corrupted/invalid. Since the cursor is part of a TextRange, it means the TextRange is already invalid. Interestingly, this is exactly what valgrind is telling us in the lower part: The TextRange destructor that lead to the invalid read above is called in the 2nd half of the valgrind trace. From this, we can conclude that the function KateTextBuffer::mergeBlock() accesses a dangling pointer to a TextRange.
That’s a good start, but in fact, this is where the hard work begins. I’ve invesed about 2 hours to find that probably the dangling pointer is in a cache that is supposed to make the access of the TextRanges (MovingRanges to be precise) faster. Christoph then looked into it and tried several attempts to fix the nasty bug, finally showing us that the cache was indeed wrong but due to a too-late fixup of the start/end text cursors with respect to the TextBlocks. In total, Christoph probably invested about 6 hours, and given that Gerald certainly took quite a lot of time too to write the unit test, we probably have more than 10 ours just to get this bug fixed. Still, the good news is that we got it! And if we are lucky, this was the last bug in Kate’s text buffer. So on to the next crashes… Help welcome!