diff --git a/Buffer.cc b/Buffer.cc index 0715673..fb60ba6 100644 --- a/Buffer.cc +++ b/Buffer.cc @@ -929,10 +929,14 @@ Buffer::insert_row(int row, const std::string_view text) rows_cache_dirty_ = true; edited_at_(row); if (swap_rec_) { - // Avoid allocation: emit the row text insertion (if any) and the newline insertion. - if (!text.empty()) - swap_rec_->OnInsert(row, 0, text); - swap_rec_->OnInsert(row, static_cast(text.size()), std::string_view("\n", 1)); + // One record: the first of two could trigger a journal checkpoint, + // whose snapshot already holds the newline the second then added + // again on replay. + std::string line; + line.reserve(text.size() + 1); + line.append(text); + line.push_back('\n'); + swap_rec_->OnInsert(row, 0, line); } } diff --git a/CMakeLists.txt b/CMakeLists.txt index bb78587..9010a8f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,6 +22,8 @@ option(ENABLE_ASAN "Enable AddressSanitizer for builds" OFF) if (ENABLE_ASAN) message(STATUS "ASan enabled") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -fno-omit-frame-pointer") + # The project is C++; without this only C sources were instrumented. + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -fno-omit-frame-pointer") # Ensure the sanitizer is linked too (especially important on some platforms) set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address") endif () diff --git a/Command.cc b/Command.cc index b876e6a..ad63d83 100644 --- a/Command.cc +++ b/Command.cc @@ -4103,8 +4103,12 @@ cmd_yank(CommandContext &ctx) } const std::size_t delta_y = nl_count; const std::size_t delta_x = (last_nl == std::string::npos) ? ins.size() : (ins.size() - last_nl - 1); - const std::size_t above = (y0 >= sy) ? (y0 - sy) : 0; - buf->SetCursor(delta_x, y0 + delta_y + above * nl_count); + // The cursor may have left the selection (C-k a, Enter, ...) without + // updating it; place it relative to the nearest selected line. + const std::size_t yc = std::clamp(y0, sy, ey); + const std::size_t above = yc - sy; + buf->SetCursor(delta_x, yc + delta_y + above * nl_count); + clamp_cursor_to_buffer(*buf); } else { if (u) u->Begin(UndoType::Paste); @@ -5765,6 +5769,22 @@ run_handler(Editor &ed, const Command &cmd, CommandContext &ctx) if (UndoSystem *u = b->Undo()) u->AbortGroups(); }; + // Whatever a command did to the cursor (remembered positions, visual-line + // arithmetic, row counts from before an edit), never leave it past a line + // or the buffer: the next edit would be applied at the clamped position + // but recorded for undo at the stale one, and could not be undone. + struct ClampOnExit { + Editor &ed; + + + ~ClampOnExit() + { + if (Buffer *b = ed.CurrentBuffer()) + clamp_cursor_to_buffer(*b); + } + } clamp_on_exit{ed}; + if (Buffer *b = ed.CurrentBuffer()) + clamp_cursor_to_buffer(*b); try { return cmd.handler(ctx); } catch (const std::exception &e) { diff --git a/docs/audits/claude-findings-20260923.md b/docs/audits/claude-findings-20260923.md index c21ce70..64ac7f7 100644 --- a/docs/audits/claude-findings-20260923.md +++ b/docs/audits/claude-findings-20260923.md @@ -430,6 +430,19 @@ before the fix (unless noted). indefinitely. The editor loop now retries it each second while idle (this was also why SwapReplay_LostRecord_ResyncsWithCheckpoint was flaky under load). +- Undoing C-k C-k (and redoing an inserted row) journaled the row and + its newline as two records; a checkpoint triggered by the first + already contained the newline, so recovery silently produced an + extra blank line. +- Visual-line yank with the cursor moved out of the selection (C-k a, + Enter) left it past the end of a line or the buffer, and text typed + there could not be undone. Every command now leaves the cursor + inside the buffer (checked in the command dispatcher). +- ENABLE_ASAN instrumented only C sources, i.e. none of the project; + it now applies to C++ as well. (Earlier "ASan clean" results in this + audit came from that uninstrumented build; the full suite has since + passed under a correctly instrumented ASan build with leak checks, + and the stress fuzzers ran with ASan and UBSan.) **Known limitations (not fixed)** - Catastrophic regex backtracking (e.g. `(a*)*b`) can still take very diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc index dd90711..2412ad3 100644 --- a/tests/test_audit_regressions.cc +++ b/tests/test_audit_regressions.cc @@ -1337,3 +1337,60 @@ TEST(Audit_ReplaceAll_TrimmedEditMatchesModel) } } } + + +// insert_row (undo of C-k C-k) sent its text and newline as two journal +// records; a checkpoint triggered by the first (here the 1 MiB threshold) +// already held the newline, which the second then added again on replay. +TEST(Audit_Swap_InsertRowIsOneRecord) +{ + TempDir d("insert_row_swap"); + const std::string path = (d.path / "big.txt").string(); + { + std::ofstream o(path, std::ios::binary | std::ios::trunc); + o << std::string(1100000, 'A') << "\nB\n"; + } + std::remove(kte::SwapManager::ComputeSwapPathForFilename(path).c_str()); + TestHarness h; + std::string err; + ASSERT_TRUE(h.EditorRef().OpenFile(path, err)); + Buffer &b = h.Buf(); + b.SetCursor(0, 0); + ASSERT_TRUE(h.Exec(CommandId::KillLine)); + ASSERT_TRUE(h.Exec(CommandId::Undo)); + const std::string text = b.BytesForTests(); + h.EditorRef().Swap()->Flush(&b); + const std::string copy = (d.path / "crash.swp").string(); + { + std::ofstream o(copy, std::ios::binary | std::ios::trunc); + o << slurp(kte::SwapManager::ComputeSwapPathForTests(b)); + } + Buffer b2; + ASSERT_TRUE(b2.OpenFromFile(path, err)); + ASSERT_TRUE(kte::SwapManager::ReplayFile(b2, copy, err)); + ASSERT_TRUE(b2.BytesForTests() == text); + b.SetDirty(false); +} + + +// Visual-line yank with the cursor moved outside the selection (C-k a) +// left it past the end of an empty row; text typed there could not be +// undone. Every command now leaves the cursor inside the buffer. +TEST(Audit_VisualYank_CursorStaysInBuffer) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "abc def\n"); + b.SetCursor(0, 0); + ASSERT_TRUE(h.Exec(CommandId::DeleteWordNext)); + ASSERT_TRUE(h.Exec(CommandId::VisualLineModeToggle)); + ASSERT_TRUE(h.Exec(CommandId::MarkAllAndJumpEnd)); + ASSERT_TRUE(h.Exec(CommandId::Yank)); + ASSERT_TRUE(b.Curx() <= b.GetLineString(b.Cury()).size()); + ASSERT_TRUE(b.Cury() < b.Nrows()); + ASSERT_TRUE(h.Exec(CommandId::InsertText, "Z")); + ASSERT_TRUE(h.Exec(CommandId::Backspace)); + for (int i = 0; i < 20; ++i) + (void) h.Exec(CommandId::Undo); + ASSERT_EQ(h.Text(), std::string("abc def\n")); +}