Bug 137555

I just did another test, and it seems to happen every time I perform the [un]commenting pattern on that particular piece of code. I moved it into another existing file and Kate seg faulted again.

paste the backtrace:

[KCrash handler]
#6  0xb75a6da2 in QString::insert () from /usr/lib/libqt-mt.so.3
#7  0xb6401332 in KateTextLine::insertText ()
   from /usr/lib/kde3/libkatepart.so
#8  0xb648a81a in KateDocument::editInsertText ()
   from /usr/lib/kde3/libkatepart.so
#9  0xb6496dc0 in KateDocument::insertText ()
   from /usr/lib/kde3/libkatepart.so
#10 0xb649733a in KateDocument::insertText ()
   from /usr/lib/kde3/libkatepart.so
#11 0xb646bdd5 in KateDocument::addStartStopCommentToSelection ()
   from /usr/lib/kde3/libkatepart.so
#12 0xb64b549e in KateDocument::comment () from /usr/lib/kde3/libkatepart.so
#13 0xb64d6037 in KateView::comment () from /usr/lib/kde3/libkatepart.so
#14 0xb64bb576 in KateView::qt_invoke () from /usr/lib/kde3/libkatepart.so
#15 0xb7287957 in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#16 0xb72883fc in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#17 0xb6cd03b9 in KAction::activated () from /usr/lib/libkdeui.so.4
#18 0xb6d0dc02 in KAction::slotActivated () from /usr/lib/libkdeui.so.4
#19 0xb6dd654f in KAction::qt_invoke () from /usr/lib/libkdeui.so.4
#20 0xb7287957 in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#21 0xb72883fc in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#22 0xb782f249 in KAccelPrivate::menuItemActivated ()
   from /usr/lib/libkdecore.so.4
#23 0xb787e3d7 in KAccelPrivate::emitActivatedSignal ()
   from /usr/lib/libkdecore.so.4
#24 0xb791c46e in KAccelPrivate::eventFilter () from /usr/lib/libkdecore.so.4
#25 0xb7286f04 in QObject::activate_filters () from /usr/lib/libqt-mt.so.3
#26 0xb7286f82 in QObject::event () from /usr/lib/libqt-mt.so.3
#27 0xb72be65b in QWidget::event () from /usr/lib/libqt-mt.so.3
#28 0xb7394c5e in QMainWindow::event () from /usr/lib/libqt-mt.so.3
#29 0xb672497c in KateMainWindow::event ()
   from /usr/lib/libkateinterfaces.so.0
#30 0xb721eb88 in QApplication::internalNotify () from /usr/lib/libqt-mt.so.3
#31 0xb7220b58 in QApplication::notify () from /usr/lib/libqt-mt.so.3
#32 0xb7914db2 in KApplication::notify () from /usr/lib/libkdecore.so.4
#33 0xb788a777 in KAccelEventHandler::x11Event ()
   from /usr/lib/libkdecore.so.4
#34 0xb79131e3 in KApplication::x11EventFilter ()
   from /usr/lib/libkdecore.so.4
#35 0xb719d2d3 in qt_set_x11_event_filter () from /usr/lib/libqt-mt.so.3
#36 0xb71ada62 in QApplication::x11ProcessEvent () from /usr/lib/libqt-mt.so.3
#37 0xb71c5320 in QEventLoop::processEvents () from /usr/lib/libqt-mt.so.3
#38 0xb723925e in QEventLoop::enterLoop () from /usr/lib/libqt-mt.so.3
#39 0xb723906e in QEventLoop::exec () from /usr/lib/libqt-mt.so.3
#40 0xb7220731 in QApplication::exec () from /usr/lib/libqt-mt.so.3
#41 0xb6784df3 in kdemain () from /usr/lib/libkdeinit_kate.so
#42 0xb7f64524 in kdeinitmain () from /usr/lib/kde3/kate.so
#43 0x0804e4df in ?? ()

can reproduce:
1. mark line 7-20
2. ctrl+c
3. ctrl+shift+d
4. ctrl+d
5. crash

if step 2 is omitted, it does not crash.

Valgrind trace:

**3864** new/new[] failed and should throw an exception, but Valgrind
   cannot throw exceptions and so is aborting instead.  Sorry.
==3864==    at 0x4A20AE7: VALGRIND_PRINTF_BACKTRACE (valgrind.h:2366)
==3864==    by 0x4A20D0C: operator new[](unsigned long) (vg_replace_malloc.c:199)
==3864==    by 0x7C1BDAD: QString::setLength(unsigned) (in /usr/lib/libqt-mt.so.3.3.6)
==3864==    by 0x7C1BF18: QString::grow(unsigned) (in /usr/lib/libqt-mt.so.3.3.6)
==3864==    by 0x7C20876: QString::insert(unsigned, QChar const*, unsigned) (in /usr/lib/libqt-mt.so.3.3.6)
==3864==    by 0xB2AAA50: KateTextLine::insertText(unsigned, unsigned, QChar const*, unsigned char*) (in /usr/lib/kde3/libkatepart.so)
==3864==    by 0xB327DBF: KateDocument::editInsertText(unsigned, unsigned, QString const&) (in /usr/lib/kde3/libkatepart.so)
==3864==    by 0xB333542: KateDocument::insertText(unsigned, unsigned, QString const&, bool) (in /usr/lib/kde3/libkatepart.so)
==3864==    by 0xB333A5B: KateDocument::insertText(unsigned, unsigned, QString const&) (in /usr/lib/kde3/libkatepart.so)
==3864==    by 0xB3047AB: KateDocument::addStartStopCommentToSelection(KateView*, int) (in /usr/lib/kde3/libkatepart.so)
==3864==    by 0xB35278F: KateDocument::comment(KateView*, unsigned, unsigned, int) (in /usr/lib/kde3/libkatepart.so)
==3864==    by 0xB3518AC: KateView::qt_invoke(int, QUObject*) (in /usr/lib/kde3/libkatepart.so)

SVN commit 607385 by dhaumann:

The view's selection selectStart and selectEnd are KateSuperCursors meaning
that they have super-cow power. They are updated *automatically* when text
is inserted or removed. A KateSuperCursor inserts itself into the document's
super cursor list. In KateDocument::editRemove/InsertText there is a loop
over all super cursors to keep them in sync.

The 'un/comment' functions tried to adapt the super cursors themselves,
although they were already updated. This resulted in negative columns, which
somewhere wrapped around in a uint-conversion, which then set an insane
selection, moo. This fixes the crash reported in bug #137555.

Review and testing welcome! ;)

CCMAIL:kwrite-devel@kde.org
BUG:137555


 M  +2 -14     katedocument.cpp  


--- branches/KDE/3.5/kdelibs/kate/part/katedocument.cpp #607384:607385
@@ -3671,10 +3671,7 @@
     removeText (sl, sc, sl, sc + startCommentLen);
 
     editEnd ();
-
-    // Set the new selection
-    ec -= endCommentLen + ( (el == sl) ? startCommentLen : 0 );
-    view->setSelection(sl, sc, el, ec + 1);
+    // set new selection not necessary, as the selection cursors are KateSuperCursors
   }
 
   return remove;
@@ -3737,16 +3734,7 @@
   }
 
   editEnd();
-
-  if (removed)
-  {
-    // Set the new selection
-    KateDocCursor end (view->selEnd());
-    end.setCol(view->selEndCol() - ((el == view->selEndLine()) ? removeLength : 0) );
-
-    setSelection(view->selStartLine(), view->selStartCol(), end.line(), end.col());
-  }
-
+  // updating selection already done by the KateSuperCursors
   return removed;
 }
 

SVN commit 607529 by dhaumann:

KateView:
  * change selection KateSmartRange insert-behaviour to KTextEditor::SmartRange::ExpandRight

KateDocument: forward port SVN commit 607385:
  * The view's selection range is a KateSmartRange. Due to its insert behaviour
    being ExpandRight the selection range is automatically changed.
  * code cleanup + review

CCBUG:137555


 M  +73 -111   katedocument.cpp  
 M  +2 -2      katedocument.h  
 M  +1 -1      kateview.cpp  


--- trunk/KDE/kdelibs/kate/part/katedocument.cpp #607528:607529
@@ -3361,21 +3361,17 @@
   Remove a given string at the beginning
   of the current line.
 */
-bool KateDocument::removeStringFromBegining(int line, QString &str)
+bool KateDocument::removeStringFromBeginning(int line, const QString &str)
 {
   KateTextLine::Ptr textline = m_buffer->plainLine(line);
 
   KTextEditor::Cursor cursor (line, 0);
-  bool there = false;
+  bool there = textline->startsWith(str);
 
-  if (textline->startsWith(str))
-    there = true;
-  else
+  if (!there)
   {
     cursor.setColumn(textline->firstChar());
-
-    if ((cursor.column() >= 0) && ((int)textline->length() >= (cursor.column() + str.length())) && (textline->string(cursor.column(), str.length()) == str))
-      there = true;
+    there = textline->matchesAt(cursor.column(), str);
   }
 
   if (there)
@@ -3391,24 +3387,21 @@
   Remove a given string at the end
   of the current line.
 */
-bool KateDocument::removeStringFromEnd(int line, QString &str)
+bool KateDocument::removeStringFromEnd(int line, const QString &str)
 {
   KateTextLine::Ptr textline = m_buffer->plainLine(line);
 
   KTextEditor::Cursor cursor (line, 0);
-  bool there = false;
+  bool there = textline->endsWith(str);
 
-  if(textline->endsWith(str))
+  if (there)
   {
     cursor.setColumn(textline->length() - str.length());
-    there = true;
   }
   else
   {
     cursor.setColumn(textline->lastChar() - str.length() + 1);
-
-    if ((cursor.column() >= 0) && ((int)textline->length() >= (cursor.column() + str.length())) && (textline->string(cursor.column(), str.length()) == str))
-      there = true;
+    there = textline->matchesAt(cursor.column(), str);
   }
 
   if (there)
@@ -3421,24 +3414,24 @@
 }
 
 /*
-  Add to the current line a comment line mark at
-  the beginning.
+  Add to the current line a comment line mark at the beginning.
 */
 void KateDocument::addStartLineCommentToSingleLine( int line, int attrib )
 {
-  if (highlight()->getCommentSingleLinePosition(attrib)==KateHighlighting::CSLPosColumn0)
+  QString commentLineMark = highlight()->getCommentSingleLineStart(attrib);
+  int pos = -1;
+
+  if (highlight()->getCommentSingleLinePosition(attrib) == KateHighlighting::CSLPosColumn0)
   {
-    QString commentLineMark = highlight()->getCommentSingleLineStart( attrib ) + ' ';
-    insertText (KTextEditor::Cursor(line, 0), commentLineMark);
+    pos = 0;
+    commentLineMark += ' ';
+  } else {
+    const KateTextLine::Ptr l = m_buffer->line(line);
+    pos = l->firstChar();
   }
-  else
-  {
-    QString commentLineMark=highlight()->getCommentSingleLineStart(attrib);
-    KateTextLine::Ptr l = m_buffer->line(line);
-    int pos=l->firstChar();
-    if (pos >=0)
-      insertText(KTextEditor::Cursor(line, pos), commentLineMark);
-  }
+
+  if (pos >= 0)
+    insertText (KTextEditor::Cursor(line, pos), commentLineMark);
 }
 
 /*
@@ -3447,14 +3440,14 @@
 */
 bool KateDocument::removeStartLineCommentFromSingleLine( int line, int attrib )
 {
-  QString shortCommentMark = highlight()->getCommentSingleLineStart( attrib );
-  QString longCommentMark = shortCommentMark + ' ';
+  const QString shortCommentMark = highlight()->getCommentSingleLineStart( attrib );
+  const QString longCommentMark = shortCommentMark + ' ';
 
   editStart();
 
   // Try to remove the long comment mark first
-  bool removed = (removeStringFromBegining(line, longCommentMark)
-                  || removeStringFromBegining(line, shortCommentMark));
+  bool removed = (removeStringFromBeginning(line, longCommentMark)
+               || removeStringFromBeginning(line, shortCommentMark));
 
   editEnd();
 
@@ -3463,12 +3456,12 @@
 
 /*
   Add to the current line a start comment mark at the
- beginning and a stop comment mark at the end.
+  beginning and a stop comment mark at the end.
 */
 void KateDocument::addStartStopCommentToSingleLine( int line, int attrib )
 {
-  QString startCommentMark = highlight()->getCommentStart( attrib ) + ' ';
-  QString stopCommentMark = ' ' + highlight()->getCommentEnd( attrib );
+  const QString startCommentMark = highlight()->getCommentStart( attrib ) + ' ';
+  const QString stopCommentMark = ' ' + highlight()->getCommentEnd( attrib );
 
   editStart();
 
@@ -3476,7 +3469,7 @@
   insertText (KTextEditor::Cursor(line, 0), startCommentMark);
 
   // Go to the end of the line
-  int col = m_buffer->plainLine(line)->length();
+  const int col = m_buffer->plainLine(line)->length();
 
   // Add the stop comment mark
   insertText (KTextEditor::Cursor(line, col), stopCommentMark);
@@ -3501,15 +3494,15 @@
 #warning "that's a bad idea, can lead to stray endings, FIXME"
 #endif
   // Try to remove the long start comment mark first
-  bool removedStart = (removeStringFromBegining(line, longStartCommentMark)
-                       || removeStringFromBegining(line, shortStartCommentMark));
+  bool removedStart = (removeStringFromBeginning(line, longStartCommentMark)
+                    || removeStringFromBeginning(line, shortStartCommentMark));
 
   bool removedStop = false;
   if (removedStart)
   {
     // Try to remove the long stop comment mark first
     removedStop = (removeStringFromEnd(line, longStopCommentMark)
-                      || removeStringFromEnd(line, shortStopCommentMark));
+                || removeStringFromEnd(line, shortStopCommentMark));
   }
 
   editEnd();
@@ -3518,18 +3511,17 @@
 }
 
 /*
-  Add to the current selection a start comment
- mark at the beginning and a stop comment mark
- at the end.
+  Add to the current selection a start comment mark at the beginning
+  and a stop comment mark at the end.
 */
 void KateDocument::addStartStopCommentToSelection( KateView *view, int attrib )
 {
-  QString startComment = highlight()->getCommentStart( attrib );
-  QString endComment = highlight()->getCommentEnd( attrib );
+  const QString startComment = highlight()->getCommentStart( attrib );
+  const QString endComment = highlight()->getCommentEnd( attrib );
 
   KTextEditor::Range range = view->selectionRange();
 
-  if ((range.end().column() == 0) && ((range.end().line() - 1) >= 0))
+  if ((range.end().column() == 0) && (range.end().line() > 0))
     range.end().setPosition(range.end().line() - 1, lineLength(range.end().line() - 1));
 
   editStart();
@@ -3538,24 +3530,21 @@
   insertText (range.start(), startComment);
 
   editEnd ();
-
-  // Set the new selection
-  range.end().setColumn(range.end().column() + endComment.length() + ( range.onSingleLine() ? startComment.length() : 0 ));
-  view->setSelection(range);
+  // selection automatically updated (KateSmartRange)
 }
 
 /*
-  Add to the current selection a comment line
- mark at the beginning of each line.
+  Add to the current selection a comment line mark at the beginning of each line.
 */
 void KateDocument::addStartLineCommentToSelection( KateView *view, int attrib )
 {
-  QString commentLineMark = highlight()->getCommentSingleLineStart( attrib ) + ' ';
+  const QString commentLineMark = highlight()->getCommentSingleLineStart( attrib ) + ' ';
 
   int sl = view->selectionRange().start().line();
   int el = view->selectionRange().end().line();
 
-  if ((view->selectionRange().end().column() == 0) && ((el-1) >= 0))
+  // if end of selection is in column 0 in last line, omit the last line
+  if ((view->selectionRange().end().column() == 0) && (el > 0))
   {
     el--;
   }
@@ -3569,13 +3558,7 @@
   }
 
   editEnd ();
-
-  // Set the new selection
-
-  KateDocCursor end (view->selectionRange().end(), this);
-  end.setColumn(view->selectionRange().end().column() + ((el == view->selectionRange().end().line()) ? commentLineMark.length() : 0) );
-
-  view->setSelection(KTextEditor::Range(view->selectionRange().start().line(), 0, end.line(), end.column()));
+  // selection automatically updated (KateSmartRange)
 }
 
 bool KateDocument::nextNonSpaceCharPos(int &line, int &col)
@@ -3611,7 +3594,7 @@
     if(line == 0) return false;
     --line;
     col = textLine->length();
-}
+  }
   // No non-space char found
   line = -1;
   col = -1;
@@ -3624,8 +3607,8 @@
 */
 bool KateDocument::removeStartStopCommentFromSelection( KateView *view, int attrib )
 {
-  QString startComment = highlight()->getCommentStart( attrib );
-  QString endComment = highlight()->getCommentEnd( attrib );
+  const QString startComment = highlight()->getCommentStart( attrib );
+  const QString endComment = highlight()->getCommentEnd( attrib );
 
   int sl = qMax<int> (0, view->selectionRange().start().line());
   int el = qMin<int>  (view->selectionRange().end().line(), lastLine());
@@ -3634,16 +3617,14 @@
 
   // The selection ends on the char before selectEnd
   if (ec != 0) {
-    ec--;
-  } else {
-    if (el > 0) {
-      el--;
-      ec = m_buffer->plainLine(el)->length() - 1;
-    }
+    --ec;
+  } else if (el > 0) {
+    --el;
+    ec = m_buffer->plainLine(el)->length() - 1;
   }
 
-  int startCommentLen = startComment.length();
-  int endCommentLen = endComment.length();
+  const int startCommentLen = startComment.length();
+  const int endCommentLen = endComment.length();
 
   // had this been perl or sed: s/^\s*$startComment(.+?)$endComment\s*/$2/
 
@@ -3660,10 +3641,7 @@
     removeText (KTextEditor::Range(sl, sc, sl, sc + startCommentLen));
 
     editEnd ();
-
-    // Set the new selection
-    ec -= endCommentLen + ( (el == sl) ? startCommentLen : 0 );
-    view->setSelection(KTextEditor::Range(sl, sc, el, ec + 1));
+    // selection automatically updated (KateSmartRange)
   }
 
   return remove;
@@ -3671,21 +3649,20 @@
 
 bool KateDocument::removeStartStopCommentFromRegion(const KTextEditor::Cursor &start,const KTextEditor::Cursor &end,int attrib)
 {
-  QString startComment = highlight()->getCommentStart( attrib );
-  QString endComment = highlight()->getCommentEnd( attrib );
-  int startCommentLen = startComment.length();
-  int endCommentLen = endComment.length();
+  const QString startComment = highlight()->getCommentStart( attrib );
+  const QString endComment = highlight()->getCommentEnd( attrib );
+  const int startCommentLen = startComment.length();
+  const int endCommentLen = endComment.length();
 
-    bool remove = m_buffer->plainLine(start.line())->matchesAt(start.column(), startComment)
-      && ( (end.column() - endCommentLen ) >= 0 )
-      && m_buffer->plainLine(end.line())->matchesAt(end.column() - endCommentLen , endComment);
-      if (remove)  {
-        editStart();
-          removeText(KTextEditor::Range(end.line(), end.column() - endCommentLen, end.line(), end.column()));
-          removeText(KTextEditor::Range(start, startCommentLen));
-        editEnd();
-      }
-      return remove;
+  const bool remove = m_buffer->plainLine(start.line())->matchesAt(start.column(), startComment)
+                   && m_buffer->plainLine(end.line())->matchesAt(end.column() - endCommentLen , endComment);
+  if (remove) {
+    editStart();
+      removeText(KTextEditor::Range(end.line(), end.column() - endCommentLen, end.line(), end.column()));
+      removeText(KTextEditor::Range(start, startCommentLen));
+    editEnd();
+  }
+  return remove;
 }
 
 /*
@@ -3694,24 +3671,17 @@
 */
 bool KateDocument::removeStartLineCommentFromSelection( KateView *view, int attrib )
 {
-  QString shortCommentMark = highlight()->getCommentSingleLineStart( attrib );
-  QString longCommentMark = shortCommentMark + ' ';
+  const QString shortCommentMark = highlight()->getCommentSingleLineStart( attrib );
+  const QString longCommentMark = shortCommentMark + ' ';
 
   int sl = view->selectionRange().start().line();
   int el = view->selectionRange().end().line();
 
-  if ((view->selectionRange().end().column() == 0) && ((el-1) >= 0))
+  if ((view->selectionRange().end().column() == 0) && (el > 0))
   {
     el--;
   }
 
-  // Find out how many char will be removed from the last line
-  int removeLength = 0;
-  if (m_buffer->plainLine(el)->startsWith(longCommentMark))
-    removeLength = longCommentMark.length();
-  else if (m_buffer->plainLine(el)->startsWith(shortCommentMark))
-    removeLength = shortCommentMark.length();
-
   bool removed = false;
 
   editStart();
@@ -3720,22 +3690,14 @@
   for (int z = el; z >= sl; z--)
   {
     // Try to remove the long comment mark first
-    removed = (removeStringFromBegining(z, longCommentMark)
-                 || removeStringFromBegining(z, shortCommentMark)
-                 || removed);
+    removed = (removeStringFromBeginning(z, longCommentMark)
+            || removeStringFromBeginning(z, shortCommentMark)
+            || removed);
   }
 
   editEnd();
+  // selection automatically updated (KateSmartRange)
 
-  if (removed)
-  {
-    // Set the new selection
-    KateDocCursor end (view->selectionRange().end(), this);
-    end.setColumn(view->selectionRange().end().column() - ((el == view->selectionRange().end().line()) ? removeLength : 0) );
-
-    view->setSelection(KTextEditor::Range(view->selectionRange().start(), end));
-  }
-
   return removed;
 }
 
--- trunk/KDE/kdelibs/kate/part/katedocument.h #607528:607529
@@ -674,8 +674,8 @@
     void joinLines( uint first, uint last );
 
   private:
-    bool removeStringFromBegining(int line, QString &str);
-    bool removeStringFromEnd(int line, QString &str);
+    bool removeStringFromBeginning(int line, const QString &str);
+    bool removeStringFromEnd(int line, const QString &str);
 
     /**
       Find the position (line and col) of the next char
--- trunk/KDE/kdelibs/kate/part/kateview.cpp #607528:607529
@@ -115,7 +115,7 @@
     , m_hasWrap( false )
     , m_startingUp (true)
     , m_updatingDocumentConfig (false)
-    , m_selection(m_doc->smartManager()->newSmartRange(KTextEditor::Range::invalid()))
+    , m_selection(m_doc->smartManager()->newSmartRange(KTextEditor::Range::invalid(), 0L, KTextEditor::SmartRange::ExpandRight))
     , blockSelect (false)
     , m_imComposeEvent( false )
 {