Journal insert_row as one record; keep the cursor in the buffer; fix ENABLE_ASAN
- Buffer::insert_row recorded its text and newline as two swap records after inserting both. A checkpoint triggered by the first (1 MiB of edits, or the checkpoint timer) snapshotted the newline already, and the second added it again on replay: recovery after undoing C-k C-k silently gained a blank line. It is now one record. - Visual-line yank placed the cursor relative to its row assuming that row was inside the selection; after C-k a or Enter it was not, and the cursor ended past the end of a line or the buffer, where typed text was recorded for undo at a position it was not inserted at and could not be undone. The formula uses the nearest selected row, and run_handler now clamps the cursor to the buffer before and after every command. - ENABLE_ASAN added -fsanitize=address to CMAKE_C_FLAGS only; the project is C++, so nothing was instrumented. Add it to CMAKE_CXX_FLAGS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
This commit is contained in:
@@ -929,10 +929,14 @@ Buffer::insert_row(int row, const std::string_view text)
|
|||||||
rows_cache_dirty_ = true;
|
rows_cache_dirty_ = true;
|
||||||
edited_at_(row);
|
edited_at_(row);
|
||||||
if (swap_rec_) {
|
if (swap_rec_) {
|
||||||
// Avoid allocation: emit the row text insertion (if any) and the newline insertion.
|
// One record: the first of two could trigger a journal checkpoint,
|
||||||
if (!text.empty())
|
// whose snapshot already holds the newline the second then added
|
||||||
swap_rec_->OnInsert(row, 0, text);
|
// again on replay.
|
||||||
swap_rec_->OnInsert(row, static_cast<int>(text.size()), std::string_view("\n", 1));
|
std::string line;
|
||||||
|
line.reserve(text.size() + 1);
|
||||||
|
line.append(text);
|
||||||
|
line.push_back('\n');
|
||||||
|
swap_rec_->OnInsert(row, 0, line);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -22,6 +22,8 @@ option(ENABLE_ASAN "Enable AddressSanitizer for builds" OFF)
|
|||||||
if (ENABLE_ASAN)
|
if (ENABLE_ASAN)
|
||||||
message(STATUS "ASan enabled")
|
message(STATUS "ASan enabled")
|
||||||
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -fno-omit-frame-pointer")
|
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)
|
# Ensure the sanitizer is linked too (especially important on some platforms)
|
||||||
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address")
|
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address")
|
||||||
endif ()
|
endif ()
|
||||||
|
|||||||
+22
-2
@@ -4103,8 +4103,12 @@ cmd_yank(CommandContext &ctx)
|
|||||||
}
|
}
|
||||||
const std::size_t delta_y = nl_count;
|
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 delta_x = (last_nl == std::string::npos) ? ins.size() : (ins.size() - last_nl - 1);
|
||||||
const std::size_t above = (y0 >= sy) ? (y0 - sy) : 0;
|
// The cursor may have left the selection (C-k a, Enter, ...) without
|
||||||
buf->SetCursor(delta_x, y0 + delta_y + above * nl_count);
|
// 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 {
|
} else {
|
||||||
if (u)
|
if (u)
|
||||||
u->Begin(UndoType::Paste);
|
u->Begin(UndoType::Paste);
|
||||||
@@ -5765,6 +5769,22 @@ run_handler(Editor &ed, const Command &cmd, CommandContext &ctx)
|
|||||||
if (UndoSystem *u = b->Undo())
|
if (UndoSystem *u = b->Undo())
|
||||||
u->AbortGroups();
|
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 {
|
try {
|
||||||
return cmd.handler(ctx);
|
return cmd.handler(ctx);
|
||||||
} catch (const std::exception &e) {
|
} catch (const std::exception &e) {
|
||||||
|
|||||||
@@ -430,6 +430,19 @@ before the fix (unless noted).
|
|||||||
indefinitely. The editor loop now retries it each second while idle
|
indefinitely. The editor loop now retries it each second while idle
|
||||||
(this was also why SwapReplay_LostRecord_ResyncsWithCheckpoint was
|
(this was also why SwapReplay_LostRecord_ResyncsWithCheckpoint was
|
||||||
flaky under load).
|
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)**
|
**Known limitations (not fixed)**
|
||||||
- Catastrophic regex backtracking (e.g. `(a*)*b`) can still take very
|
- Catastrophic regex backtracking (e.g. `(a*)*b`) can still take very
|
||||||
|
|||||||
@@ -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"));
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user