Skip to content

Commit 103a5af

Browse files
Fix crash on reverseIndex and cmdRestoreCursor (issue #256) (#431)
* Fix crash on reverseIndex when yBase exceeds lines.count (#256) The crash occurred because Buffer.clear() did not reset yBase to 0. When switching between normal and alternate buffers, the stale yBase value could cause buffer.y + buffer.yBase to exceed buffer.lines.count, triggering an out-of-bounds access in reverseIndex() and scroll(). Changes: - Buffer.swift: Add yBase = 0 to clear() to fix the root cause - Terminal.swift: Add defensive bounds checks in reverseIndex() and scroll() to prevent crashes if buffer state becomes invalid - BufferTests.swift: Add 9 unit tests covering the bug and edge cases Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix crash in cmdRestoreCursor when savedX/savedY are invalid The function was restoring buffer.x and buffer.y from saved values without bounds checking, causing abort() in Debug builds when savedX or savedY became invalid after resize/scroll operations. Fix: Clamp savedX and savedY to valid ranges before assignment: - buffer.x = min(max(0, buffer.savedX), cols - 1) - buffer.y = min(max(0, buffer.savedY), rows - 1) This is the same class of bug as issue #256. Added 3 tests for cursor restore clamping behavior. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 79af76d commit 103a5af

3 files changed

Lines changed: 416 additions & 6 deletions

File tree

Sources/SwiftTerm/Buffer.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,16 +300,17 @@ public final class Buffer {
300300
public func clear ()
301301
{
302302
yDisp = 0
303+
yBase = 0
303304
xBase = 0
304305
linesTop = 0
305306
x = 0
306307
y = 0
307-
308+
308309
_lines = CircularBufferLineList (maxLength: getCorrectBufferLength(rows))
309310
_lines.makeEmpty = { [unowned self] line in getBlankLine(attribute: CharData.defaultAttr, isWrapped: false) }
310311
scrollTop = 0
311312
scrollBottom = rows - 1
312-
313+
313314
// Figure out how to do this elegantly
314315
// SetupTabStops ()
315316
}

Sources/SwiftTerm/Terminal.swift

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2493,8 +2493,10 @@ open class Terminal {
24932493

24942494
func cmdRestoreCursor (_ pars: [Int], _ collect: cstring)
24952495
{
2496-
buffer.x = buffer.savedX
2497-
buffer.y = buffer.savedY
2496+
// Clamp savedX and savedY to valid ranges to prevent abort() in Debug builds.
2497+
// Saved values can become invalid after resize/scroll operations.
2498+
buffer.x = min(max(0, buffer.savedX), cols - 1)
2499+
buffer.y = min(max(0, buffer.savedY), rows - 1)
24982500
curAttr = buffer.savedAttr
24992501
charset = buffer.savedCharset
25002502
originMode = buffer.savedOriginMode
@@ -5021,6 +5023,14 @@ open class Terminal {
50215023
} else {
50225024
// scrollTop is non-zero which means no line will be going to the
50235025
// scrollback, instead we can just shift them in-place.
5026+
5027+
// Ensure the indices are within bounds to prevent crash (related to issue #256)
5028+
// This can happen when the buffer has been trimmed and yBase is stale
5029+
guard bottomRow < buffer.lines.count else {
5030+
print ("scroll: bottomRow \(bottomRow) >= lines.count \(buffer.lines.count), state: yBase=\(buffer.yBase) scrollTop=\(buffer.scrollTop) scrollBottom=\(buffer.scrollBottom) isAlternate=\(isCurrentBufferAlternate)")
5031+
return
5032+
}
5033+
50245034
let scrollRegionHeight = bottomRow - topRow + 1 /*as it's zero-based*/
50255035
if scrollRegionHeight > 1 {
50265036
if !buffer.lines.shiftElements (start: topRow + 1, count: scrollRegionHeight - 1, offset: -1) {
@@ -5435,11 +5445,20 @@ open class Terminal {
54355445
// possibly move the code below to term.reverseScroll()
54365446
// test: echo -ne '\e[1;1H\e[44m\eM\e[0m'
54375447
// blankLine(true) is xterm/linux behavior
5448+
let startIndex = buffer.y + buffer.yBase
54385449
let scrollRegionHeight = buffer.scrollBottom - buffer.scrollTop
5439-
if !buffer.lines.shiftElements (start: buffer.y + buffer.yBase, count: scrollRegionHeight, offset: 1) {
5450+
5451+
// Ensure the start index is within bounds to prevent crash (issue #256)
5452+
// This can happen when the buffer has been trimmed and yBase is stale
5453+
guard startIndex < buffer.lines.count else {
5454+
print ("reverseIndex: start index \(startIndex) >= lines.count \(buffer.lines.count), state: y=\(buffer.y) yBase=\(buffer.yBase) scrollTop=\(buffer.scrollTop) scrollBottom=\(buffer.scrollBottom) isAlternate=\(isCurrentBufferAlternate)")
5455+
return
5456+
}
5457+
5458+
if !buffer.lines.shiftElements (start: startIndex, count: scrollRegionHeight, offset: 1) {
54405459
print ("Assertion on reverseIndex, state was: y=\(buffer.y) scrollTop=\(buffer.scrollTop) yDisp=\(buffer.yDisp) linesTop=\(buffer.linesTop) isAlternate=\(isCurrentBufferAlternate)")
54415460
}
5442-
buffer.lines [buffer.y + buffer.yBase] = buffer.getBlankLine (attribute: eraseAttr ())
5461+
buffer.lines [startIndex] = buffer.getBlankLine (attribute: eraseAttr ())
54435462
updateRange (startLine: buffer.scrollTop, endLine: buffer.scrollBottom)
54445463
} else if buffer.y > 0 {
54455464
buffer.y -= 1

0 commit comments

Comments
 (0)