From 7664c99658c07b63bcdfc3827dd74d8da19ef1fd Mon Sep 17 00:00:00 2001 From: fvogel Date: Tue, 31 Jan 2012 22:24:11 +0000 Subject: [Bug-1630262]: segfault when deleting lines with peer text widgets --- ChangeLog | 7 ++++ generic/tkText.c | 43 +++++++++++++++++++++-- generic/tkTextBTree.c | 11 ++++-- generic/tkTextDisp.c | 32 +++++++++++++++-- tests/text.test | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 181 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3673abf..be731ca 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2012-01-?? Francois Vogel + + * generic/tkText.c: [Bug-1630262]: segfault when deleting lines + * generic/tkTextBTree.c with peer text widgets + * generic/tkTextDisp.c + * tests/text.test + 2012-01-29 Jan Nijtmans * win/tkImgPhoto.c: [Bug 3480634]: PNG Images missing in menus on Mac diff --git a/generic/tkText.c b/generic/tkText.c index d050170..18cbcf4 100644 --- a/generic/tkText.c +++ b/generic/tkText.c @@ -3097,6 +3097,11 @@ DeleteIndexRange( resetView = 1; line = line1; byteIndex = tPtr->topIndex.byteIndex; + } else { + /* + * Deletion range starts after the top line. This peers's view + * will not need to be reset. Nothing to do. + */ } } else if (index2.linePtr == tPtr->topIndex.linePtr) { /* @@ -3113,6 +3118,11 @@ DeleteIndexRange( } else { byteIndex -= (index2.byteIndex - index1.byteIndex); } + } else { + /* + * Deletion range ends before the top line. This peers's view + * will not need to be reset. Nothing to do. + */ } if (resetView) { lineAndByteIndex[resetViewCount] = line; @@ -3157,14 +3167,43 @@ DeleteIndexRange( TkTextIndex indexTmp; if (tPtr == textPtr) { - if (viewUpdate) { + if (viewUpdate) { + /* + * line cannot be before -startline of textPtr because + * this line corresponds to an index which is necessarily + * between "1.0" and "end" relative to textPtr. + * Therefore no need to clamp line to the -start/-end + * range. + */ + TkTextMakeByteIndex(sharedTextPtr->tree, textPtr, line, byteIndex, &indexTmp); TkTextSetYView(tPtr, &indexTmp, 0); } } else { - TkTextMakeByteIndex(sharedTextPtr->tree, NULL, line, + TkTextMakeByteIndex(sharedTextPtr->tree, tPtr, line, byteIndex, &indexTmp); + /* + * line may be before -startline of tPtr and must be + * clamped to -startline before providing it to + * TkTextSetYView otherwise lines before -startline + * would be displayed. + * There is no need to worry about -endline however, + * because the view will only be reset if the deletion + * involves the TOP line of the screen + */ + + if (tPtr->start != NULL) { + int start; + TkTextIndex indexStart; + + start = TkBTreeLinesTo(NULL, tPtr->start); + TkTextMakeByteIndex(sharedTextPtr->tree, NULL, start, + 0, &indexStart); + if (TkTextIndexCmp(&indexTmp, &indexStart) < 0) { + indexTmp = indexStart; + } + } TkTextSetYView(tPtr, &indexTmp, 0); } } diff --git a/generic/tkTextBTree.c b/generic/tkTextBTree.c index 0038e64..005ce46 100644 --- a/generic/tkTextBTree.c +++ b/generic/tkTextBTree.c @@ -662,12 +662,12 @@ AdjustStartEndRefs( if (textPtr->start != NULL) { count--; treePtr->startEnd[count] = textPtr->start; - treePtr->startEndRef[count] = treePtr->sharedTextPtr->peers; + treePtr->startEndRef[count] = textPtr; } if (textPtr->end != NULL) { count--; treePtr->startEnd[count] = textPtr->end; - treePtr->startEndRef[count] = treePtr->sharedTextPtr->peers; + treePtr->startEndRef[count] = textPtr; } } } @@ -1609,7 +1609,7 @@ TkBTreeFindLine( } /* - * Check for the any start/end offset for this text widget. + * Check for any start/end offset for this text widget. */ if (textPtr != NULL) { @@ -1993,6 +1993,11 @@ TkBTreeLinesTo( } if (textPtr != NULL && textPtr->start != NULL) { index -= TkBTreeLinesTo(NULL, textPtr->start); + if (index < 0) { + /* One should panic here! + Tcl_Panic("TkBTreeLinesTo: linePtr comes before -startline"); + */ + } } return index; } diff --git a/generic/tkTextDisp.c b/generic/tkTextDisp.c index 5f30b11..f674b75 100644 --- a/generic/tkTextDisp.c +++ b/generic/tkTextDisp.c @@ -3219,6 +3219,34 @@ TextInvalidateLineMetrics( int fromLine; TextDInfo *dInfoPtr = textPtr->dInfoPtr; + /* + * All lines to invalidate must be inside the -startline/-endline range. + */ + + if (linePtr != NULL) { + int start; + TkTextLine *toLinePtr; + if (textPtr->start != NULL) { + fromLine = TkBTreeLinesTo(NULL, linePtr); + start = TkBTreeLinesTo(NULL, textPtr->start); + if (fromLine < start) { + lineCount -= start - fromLine; + linePtr = textPtr->start; + } + } + if (textPtr->end != NULL) { + int count = 0; + toLinePtr = linePtr; + while (count < lineCount && toLinePtr != NULL) { + toLinePtr = TkBTreeNextLine(textPtr, toLinePtr); + count++; + } + if (toLinePtr == NULL) { + lineCount = count; + } + } + } + if (linePtr != NULL) { int counter = lineCount; @@ -3229,7 +3257,7 @@ TextInvalidateLineMetrics( */ TkBTreeLinePixelEpoch(textPtr, linePtr) = 0; - while (counter > 0 && linePtr != 0) { + while (counter > 0 && linePtr != NULL) { linePtr = TkBTreeNextLine(textPtr, linePtr); if (linePtr != NULL) { TkBTreeLinePixelEpoch(textPtr, linePtr) = 0; @@ -3244,7 +3272,7 @@ TextInvalidateLineMetrics( * more lines than is strictly necessary (but the examination of the * extra lines should be quick, since their pixelCalculationEpoch will * be up to date). However, to keep track of that would require more - * complex record-keeping that what we have. + * complex record-keeping than what we have. */ if (dInfoPtr->lineUpdateTimer == NULL) { diff --git a/tests/text.test b/tests/text.test index 44a3065..52689ba 100644 --- a/tests/text.test +++ b/tests/text.test @@ -11,7 +11,7 @@ eval tcltest::configure $argv tcltest::loadTestedCommands namespace import -force tcltest::test -# Create entries in the option database to be sure that geometry options +# Create entries in the odeption database to be sure that geometry options # like border width have predictable values. option add *Text.borderWidth 2 @@ -3622,6 +3622,100 @@ test text-32.1 {peer widget -start, -end and selection} { set res } {{10.0 20.0} {6.0 16.0} {6.0 11.0} {1.0 6.0} {1.0 2.0} {} {10.0 20.0}} +test text-32.2 {peer widget -start, -end and deletion (bug 1630262)} -setup { + destroy .t .pt + set res {} +} -body { + text .t + .t peer create .pt + for {set i 1} {$i < 100} {incr i} { + .t insert end "Line $i\n" + } + .t configure -startline 5 + # none of the following delete shall crash + # (all did before fixing bug 1630262) + # 1. delete on the same line: line1 == line2 in DeleteIndexRange, + # and resetView is true neither for .t not for .pt + .pt delete 2.0 2.2 + # 2. delete just one line: line1 < line2 in DeleteIndexRange, + # and resetView is true only for .t, not for .pt + .pt delete 2.0 3.0 + # 3. delete several lines: line1 < line2 in DeleteIndexRange, + # and resetView is true only for .t, not for .pt + .pt delete 2.0 5.0 + # 4. delete to the end line: line1 < line2 in DeleteIndexRange, + # and resetView is true only for .t, not for .pt + .pt delete 2.0 end + # this test succeeds provided there is no crash + set res 1 +} -cleanup { + destroy .pt +} -result {1} + +test text-32.3 {peer widget -start, -end and deletion (bug 1630262)} -setup { + destroy .t .pt + set res {} +} -body { + text .t + .t peer create .pt + for {set i 1} {$i < 100} {incr i} { + .t insert end "Line $i\n" + } + .t configure -startline 5 + .pt configure -startline 3 + # the following delete shall not crash + # (it did before fixing bug 1630262) + .pt delete 2.0 3.0 + # moreover -startline shall be correct + # (was wrong before fixing bug 1630262) + lappend res [.t cget -start] [.pt cget -start] +} -cleanup { + destroy .pt +} -result {4 3} + +test text-32.4 {peer widget -start, -end and deletion (bug 1630262)} -setup { + destroy .t .pt + set res {} +} -body { + text .t + .t peer create .pt + for {set i 1} {$i < 100} {incr i} { + .t insert end "Line $i\n" + } + .t configure -startline 5 -endline 15 + .pt configure -startline 8 -endline 12 + # .pt now shows a range entirely inside the range of .pt + # from .t, delete lines located after [.pt cget -end] + .t delete 9.0 10.0 + # from .t, delete lines straddling [.pt cget -end] + .t delete 6.0 9.0 + lappend res [.t cget -start] [.t cget -end] [.pt cget -start] [.pt cget -end] + .t configure -startline 5 -endline 12 + .pt configure -startline 8 -endline 12 + # .pt now shows again a range entirely inside the range of .pt + # from .t, delete lines located before [.pt cget -start] + .t delete 2.0 3.0 + # from .t, delete lines straddling [.pt cget -start] + .t delete 2.0 5.0 + lappend res [.t cget -start] [.t cget -end] [.pt cget -start] [.pt cget -end] + .t configure -startline 22 -endline 31 + .pt configure -startline 42 -endline 51 + # .t now shows a range entirely before the range of .pt + # from .t, delete some lines, then do it from .pt + .t delete 2.0 3.0 + .t delete 2.0 5.0 + .pt delete 2.0 5.0 + lappend res [.t cget -start] [.t cget -end] [.pt cget -start] [.pt cget -end] + .t configure -startline 55 -endline 75 + .pt configure -startline 60 -endline 70 + # .pt now shows a range entirely inside the range of .t + # from .t, delete a range straddling the entire range of .pt + .t delete 3.0 18.0 + lappend res [.t cget -start] [.t cget -end] [.pt cget -start] [.pt cget -end] +} -cleanup { + destroy .pt +} -result {5 11 8 10 5 8 6 8 22 27 38 44 55 60 57 57} + test text-33.1 {widget dump -command alters tags} { .t delete 1.0 end .t insert end "abc\n" a "---" {} "def" b " \n" {} "ghi\n" c -- cgit v0.12 From f376d1c99841e1edea5440aa66932d03e9b0e038 Mon Sep 17 00:00:00 2001 From: fvogel Date: Thu, 2 Feb 2012 10:40:12 +0000 Subject: Better fix for bug-1630262, also fixing bug-1615425 --- generic/tkTextBTree.c | 34 +++++++++++++++++++++++++++------- generic/tkTextDisp.c | 28 ---------------------------- 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/generic/tkTextBTree.c b/generic/tkTextBTree.c index 005ce46..67ff79d 100644 --- a/generic/tkTextBTree.c +++ b/generic/tkTextBTree.c @@ -1991,17 +1991,37 @@ TkBTreeLinesTo( index += nodePtr2->numLines; } } - if (textPtr != NULL && textPtr->start != NULL) { - index -= TkBTreeLinesTo(NULL, textPtr->start); - if (index < 0) { - /* One should panic here! - Tcl_Panic("TkBTreeLinesTo: linePtr comes before -startline"); - */ + if (textPtr != NULL) { + /* + * The index to return must be relative to textPtr, not to the entire + * tree. Take care to never return a negative index when linePtr + * denotes a line before -startline, or an index larger than the + * number of lines in textPtr when linePtr is a line past -endline. + */ + + int indexStart, indexEnd; + + if (textPtr->start != NULL) { + indexStart = TkBTreeLinesTo(NULL, textPtr->start); + } else { + indexStart = 0; + } + if (textPtr->end != NULL) { + indexEnd = TkBTreeLinesTo(NULL, textPtr->end); + } else { + indexEnd = TkBTreeNumLines(textPtr->sharedTextPtr->tree, NULL); + } + if (index < indexStart) { + index = 0; + } else if (index > indexEnd) { + index = TkBTreeNumLines(textPtr->sharedTextPtr->tree, textPtr); + } else { + index -= indexStart; } } return index; } - + /* *---------------------------------------------------------------------- * diff --git a/generic/tkTextDisp.c b/generic/tkTextDisp.c index f674b75..61fb76e 100644 --- a/generic/tkTextDisp.c +++ b/generic/tkTextDisp.c @@ -3219,34 +3219,6 @@ TextInvalidateLineMetrics( int fromLine; TextDInfo *dInfoPtr = textPtr->dInfoPtr; - /* - * All lines to invalidate must be inside the -startline/-endline range. - */ - - if (linePtr != NULL) { - int start; - TkTextLine *toLinePtr; - if (textPtr->start != NULL) { - fromLine = TkBTreeLinesTo(NULL, linePtr); - start = TkBTreeLinesTo(NULL, textPtr->start); - if (fromLine < start) { - lineCount -= start - fromLine; - linePtr = textPtr->start; - } - } - if (textPtr->end != NULL) { - int count = 0; - toLinePtr = linePtr; - while (count < lineCount && toLinePtr != NULL) { - toLinePtr = TkBTreeNextLine(textPtr, toLinePtr); - count++; - } - if (toLinePtr == NULL) { - lineCount = count; - } - } - } - if (linePtr != NULL) { int counter = lineCount; -- cgit v0.12 From bff83d76663bad240f5642260d60d34077bd97b4 Mon Sep 17 00:00:00 2001 From: fvogel Date: Fri, 3 Feb 2012 21:04:01 +0000 Subject: Added test for bug 1615425 --- tests/textTag.test | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/textTag.test b/tests/textTag.test index b112fc2..dcec25d 100644 --- a/tests/textTag.test +++ b/tests/textTag.test @@ -147,6 +147,19 @@ test textTag-2.13 {TkTextTagCmd - "add" option} haveCourier12 { .t tag add sel 1.1 1.5 2.4 .t tag ranges sel } {1.1 1.5 2.4 2.5} +test textTag-2.14 {tag add before -startline - Bug 1615425} haveCourier12 { + text .tt + for {set i 1} {$i <10} {incr i} { + .tt insert end "Line $i\n" + } + .tt tag configure mytag -font {Courier 12 bold} + .tt peer create .ptt + .ptt configure -startline 3 -endline 7 + # the test succeeds if next line does not crash + .tt tag add mytag 1.0 1.end + destroy .ptt .tt + set res 1 +} {1} catch {.t tag delete x} test textTag-3.1 {TkTextTagCmd - "bind" option} haveCourier12 { -- cgit v0.12 From 3ea0c8057eebd5bf0c51ec97477ef0715fb8d33b Mon Sep 17 00:00:00 2001 From: fvogel Date: Fri, 3 Feb 2012 21:45:31 +0000 Subject: Updated changelog to take bug-1615425 into account --- ChangeLog | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index be731ca..3d444cb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,9 +1,9 @@ 2012-01-?? Francois Vogel - * generic/tkText.c: [Bug-1630262]: segfault when deleting lines - * generic/tkTextBTree.c with peer text widgets - * generic/tkTextDisp.c - * tests/text.test + * generic/tkText.c: [Bug-1630262] and [Bug-1615425]: segfault + * generic/tkTextBTree.c when deleting lines or tagging outside of + * generic/tkTextDisp.c the -startline/-endline range with peer + * tests/text.test text widgets. 2012-01-29 Jan Nijtmans -- cgit v0.12 From 1487608db176d526e0c03938073340f5f2a068eb Mon Sep 17 00:00:00 2001 From: fvogel Date: Sat, 18 Feb 2012 21:55:28 +0000 Subject: Bug-3487407: Weird text indices --- ChangeLog | 9 ++++++++- generic/tkText.c | 4 ++++ generic/tkTextDisp.c | 2 +- generic/tkTextMark.c | 26 +++++++++++++++++++++----- tests/textMark.test | 7 +++++++ 5 files changed, 41 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3d444cb..994ba62 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,11 @@ -2012-01-?? Francois Vogel +2012-02-?? Francois Vogel + + * generic/tkText.c: [Bug-3487407]: Weird text indices. + * generic/tkTextDisp.c + * generic/tkTextMark.c + * tests/textMark.test + +2012-02-?? Francois Vogel * generic/tkText.c: [Bug-1630262] and [Bug-1615425]: segfault * generic/tkTextBTree.c when deleting lines or tagging outside of diff --git a/generic/tkText.c b/generic/tkText.c index 18cbcf4..6652f3d 100644 --- a/generic/tkText.c +++ b/generic/tkText.c @@ -2107,6 +2107,10 @@ ConfigureText( * Also, clamp the insert and current (unshared) marks to the new * -startline/-endline range limits of the widget. All other (shared) * marks are unchanged. + * The return value of TkTextMarkNameToIndex does not need to be + * checked: "insert" and "current" marks always exist, and the + * purpose of the code below precisely is to move them inside the + * -startline/-endline range. */ textPtr->sharedTextPtr->stateEpoch++; diff --git a/generic/tkTextDisp.c b/generic/tkTextDisp.c index 61fb76e..d0cd4d2 100644 --- a/generic/tkTextDisp.c +++ b/generic/tkTextDisp.c @@ -1971,7 +1971,7 @@ UpdateDisplayInfo( if (spaceLeft <= dInfoPtr->newTopPixelOffset) { /* - * We can full up all the needed space just by showing more of the + * We can fill up all the needed space just by showing more of the * current top line. */ diff --git a/generic/tkTextMark.c b/generic/tkTextMark.c index a9d709c..71a7949 100644 --- a/generic/tkTextMark.c +++ b/generic/tkTextMark.c @@ -285,6 +285,7 @@ TkTextSetMark( if (markPtr == textPtr->insertMarkPtr) { TkTextIndex index, index2; + int nblines; TkTextMarkSegToIndex(textPtr, textPtr->insertMarkPtr, &index); TkTextIndexForwChars(NULL,&index, 1, &index2, COUNT_INDICES); @@ -295,8 +296,17 @@ TkTextSetMark( */ TkTextChanged(NULL, textPtr, &index, &index2); - if (TkBTreeLinesTo(textPtr, indexPtr->linePtr) == - TkBTreeNumLines(textPtr->sharedTextPtr->tree, textPtr)) { + + /* + * The number of lines in the widget is zero if and only if it is + * a partial peer with -startline == -endline, i.e. an empty + * peer. In this case the mark shall be set exactly at the given + * index, and not one character backwards (bug 3487407). + */ + + nblines = TkBTreeNumLines(textPtr->sharedTextPtr->tree, textPtr); + if ((TkBTreeLinesTo(textPtr, indexPtr->linePtr) == nblines) + && (nblines > 0)) { TkTextIndexBackChars(NULL,indexPtr, 1, &insertIndex, COUNT_INDICES); indexPtr = &insertIndex; @@ -385,9 +395,15 @@ TkTextMarkSegToIndex( * * Results: * The return value is TCL_OK if "name" exists as a mark in the text - * widget. In this case *indexPtr is filled in with the next segment - * whose after the mark whose size is non-zero. TCL_ERROR is returned if - * the mark doesn't exist in the text widget. + * widget and is located within its -starline/-endline range. In this + * case *indexPtr is filled in with the next segment who is after the + * mark whose size is non-zero. TCL_ERROR is returned if the mark + * doesn't exist in the text widget, or if it is out of its -starline/ + * -endline range. In this latter case *indexPtr still contains valid + * information, in particular TkTextMarkNameToIndex called with the + * "insert" or "current" mark name may return TCL_ERROR, but *indexPtr + * contains the correct index of this mark before -startline or after + * -endline. * * Side effects: * None. diff --git a/tests/textMark.test b/tests/textMark.test index c1138ac..67b9ae5 100644 --- a/tests/textMark.test +++ b/tests/textMark.test @@ -170,6 +170,13 @@ test textMark-6.4 {TkTextMarkNameToIndex, with mark outside -startline/-endline .pt configure -startline {} -endline {} .t mark unset mymark } -result {1 {bad text index "mymark"} 1.0 1.0 1 {bad text index "mymark"} L 1 {bad text index "mymark"}} +test textMark-6.5 {insert and current marks in an empty peer - bug 3487407} -body { + .t mark set insert 1.0 + .t configure -start 5 -end 5 + set res [.t index insert] +} -cleanup { + .t configure -startline {} -endline {} +} -result {1.0} catch {eval {.t mark unset} [.t mark names]} test textMark-7.1 {MarkFindNext - invalid mark name} haveCourier12 { -- cgit v0.12