From 79fa12d6f07f2566fa12dbff73a3369e45211779 Mon Sep 17 00:00:00 2001 From: LESdylan Date: Sun, 18 Oct 2026 03:36:19 +0200 Subject: [PATCH] fix(editor): free the two undo lists readline 8.1 drops on the floor ASan reported a leak at exit after an ordinary editing session: 32 bytes plus the typed text, allocated by readline's rl_add_undo. Two paths in readline 8.1 (Ubuntu 22.04's, what the 42 machines link) lose an undo list once per line, and the in-process reader made them the shell's: - Saved line. Walking the history from a typed line with previous-history (Ctrl-P, or Up when prefix search is off) saves that line, with its undo list as data, so Down can bring it back. Ended on the recalled entry (Enter, ^C, ^G), the saved copy outlives the read and the next readline() frees it without its data. hist_rl_reclaim_saved() now runs before each read and takes the list back with public calls only: rl_maybe_save_line() so there is a saved line (unsave rings the bell otherwise), rl_maybe_unsave_line() to move its data into rl_undo_list, rl_free_undo_list() to free it, which also clears any history entry still pointing at it. - Aborted search. Alt-N / Alt-P (stock non-incremental search) reads its string with an undo list of its own; aborting restores the saved line over rl_undo_list. rl_nsearch_reclaim() frees that list in the reader when the byte readline is about to get ends the search: ^G, ^C, Rubout or ^H on an empty string, a failed read. A readline that frees it itself finds NULL. Marked Caveat: the abort keys mirror 8.1's search.c. Both predate #136/#140 (same report on 134ac5af); prefix search made the first one the everyday path on a non-PIE build, where Up was plain previous-history. tests/rl_undo_leak_test.py runs 10 leaking key sequences, each in a fresh shell with detect_leaks=1, plus 3 checks that the keys still work (Up then Down restores the typed line, Alt-P finds, ^G restores). Before: 11 failed; after: 0. SKIP on a binary built without ASan. --- incs/history.h | 1 + src/infrastructure/history_rl.c | 25 ++++++++ src/infrastructure/rl_private.h | 1 + src/platform/posix/rl_getc.c | 4 +- src/platform/posix/rl_inproc.c | 1 + src/platform/posix/rl_nsearch.c | 39 ++++++++++++ tests/rl_undo_leak_test.py | 103 ++++++++++++++++++++++++++++++++ 7 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 src/platform/posix/rl_nsearch.c create mode 100644 tests/rl_undo_leak_test.py diff --git a/incs/history.h b/incs/history.h index 9b342821..790b8a92 100644 --- a/incs/history.h +++ b/incs/history.h @@ -54,6 +54,7 @@ void hist_rehome(t_shell *state); keeps on an edited entry (history_rl.c). Never call the raw ones. */ void hist_rl_drop(int idx); void hist_rl_clear(void); +void hist_rl_reclaim_saved(void); void init_history(t_shell *state); void free_hist(t_shell *state); void parse_history_file(t_shell *state); diff --git a/src/infrastructure/history_rl.c b/src/infrastructure/history_rl.c index d95afc94..32cdcb54 100644 --- a/src/infrastructure/history_rl.c +++ b/src/infrastructure/history_rl.c @@ -61,6 +61,31 @@ void hist_rl_drop(int idx) undo_list_free((UNDO_LIST *)free_history_entry(e)); } +/* Before each read: free the undo list readline parked with its saved +** copy of the typed line. +** +** Walking the history from a typed line (previous-history: Ctrl-P, or Up +** with prefix search off) makes readline save that line, with its undo +** list as the entry's data, so Down can bring it back. When the line ends +** elsewhere -- Enter or ^C on a recalled entry -- the saved copy outlives +** the read, and the next readline() frees it without the undo list: +** readline 8.1 (Ubuntu 22.04's) leaked the typed text's undo records once +** per such line, which ASan reported at exit. +** rl_maybe_unsave_line() is the public way to take that list back into +** rl_undo_list, and rl_free_undo_list() frees it as readline does, also +** dropping any history entry still pointing at it. Unsave rings the bell +** when nothing is saved, so the save before it makes sure something is. +** Both rewrite rl_line_buffer, which is harmless here: the read that +** follows starts from an empty line. */ +void hist_rl_reclaim_saved(void) +{ + if (!rl_line_buffer) + return ; + rl_maybe_save_line(); + rl_maybe_unsave_line(); + rl_free_undo_list(); +} + /* clear_history(), freeing the undo lists it cannot. */ void hist_rl_clear(void) { diff --git a/src/infrastructure/rl_private.h b/src/infrastructure/rl_private.h index b3f31bbd..ccece657 100644 --- a/src/infrastructure/rl_private.h +++ b/src/infrastructure/rl_private.h @@ -165,6 +165,7 @@ int rl_shell_leave(t_shell *state, t_rl_bracket *b); void rl_prerow_arm(t_string *rows); FILE *rl_out_open(void); int rl_getc_hook(FILE *stream); +int rl_nsearch_reclaim(int c); int rl_idle_timeout(void); int rl_idle_fd(void); void rl_idle_event(int r); diff --git a/src/platform/posix/rl_getc.c b/src/platform/posix/rl_getc.c index 2271cba4..18e4e886 100644 --- a/src/platform/posix/rl_getc.c +++ b/src/platform/posix/rl_getc.c @@ -128,7 +128,7 @@ int rl_getc_hook(FILE *stream) { rl_take_signals(); if (rl_should_abort()) - return (rl_abort_value()); + return (rl_nsearch_reclaim(rl_abort_value())); rl_block(&old); r = -1; if (!rl_pending_signal() && !rl_should_abort()) @@ -141,5 +141,5 @@ int rl_getc_hook(FILE *stream) } if (r == '\r' || r == '\n') rl_suggest_unpaint(); - return (r); + return (rl_nsearch_reclaim(r)); } diff --git a/src/platform/posix/rl_inproc.c b/src/platform/posix/rl_inproc.c index 40e0487e..3d181303 100644 --- a/src/platform/posix/rl_inproc.c +++ b/src/platform/posix/rl_inproc.c @@ -58,6 +58,7 @@ int rl_read_inproc(t_shell *state, char *prompt) char *row; char *line; + hist_rl_reclaim_saved(); row = rl_editor_enter(state, prompt); line = readline(row); rl_editor_exit(state); diff --git a/src/platform/posix/rl_nsearch.c b/src/platform/posix/rl_nsearch.c new file mode 100644 index 00000000..8969f610 --- /dev/null +++ b/src/platform/posix/rl_nsearch.c @@ -0,0 +1,39 @@ +/* ************************************************************************** */ +/* */ +/* ::: :::::::: */ +/* rl_nsearch.c :+: :+: :+: */ +/* +:+ +:+ +:+ */ +/* By: dlesieur +#+ +:+ +#+ */ +/* +#+#+#+#+#+ +#+ */ +/* Created: 2026/10/18 03:40:00 by dlesieur #+# #+# */ +/* Updated: 2026/10/18 03:40:00 by dlesieur ### ########.fr */ +/* */ +/* ************************************************************************** */ + +#include "rl_private.h" + +/* The byte (or EOF / READERR) the reader is about to hand readline, after +** freeing the search string's undo list when that byte ends a +** non-incremental search (Alt-N / Alt-P, stock) without searching. +** +** The search reads its string into the line buffer with an undo list of +** its own: it saves the line, sets rl_undo_list to NULL and inserts. On +** Enter it frees that list before searching; on abort it puts the saved +** line back, overwriting rl_undo_list, and readline 8.1 (Ubuntu 22.04's) +** loses the list -- a leak per aborted search, reported by ASan at exit. +** Freeing it here, before readline sees the key, leaves nothing to lose, +** and a readline that frees it itself finds NULL. +** +** Caveat: the abort keys are readline 8.1's (search.c, +** _rl_nsearch_dispatch): ^G, ^C, Rubout or ^H on an empty string, and a +** failed read. A key a later readline adds to that list leaks again; a +** key it drops loses only the undo history of the search string. */ +int rl_nsearch_reclaim(int c) +{ + if (!RL_ISSTATE(RL_STATE_NSEARCH)) + return (c); + if (c == '\a' || c == 3 || c < 0 + || ((c == '\b' || c == 127) && rl_point == 0)) + rl_free_undo_list(); + return (c); +} diff --git a/tests/rl_undo_leak_test.py b/tests/rl_undo_leak_test.py new file mode 100644 index 00000000..e86eac28 --- /dev/null +++ b/tests/rl_undo_leak_test.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +"""No undo list leaks out of readline's history walk or its aborted search. + +Two places where readline 8.1 (Ubuntu 22.04's) drops an undo list it owns, +each once per line, reported by LeakSanitizer when the shell exits: + + saved line type, then walk the history with previous-history (Ctrl-P, or + Up with prefix search off) and end the line on the recalled + entry: Enter, ^C or ^G. The next read freed readline's saved + copy of the typed line, but not its undo list. + Fixed by hist_rl_reclaim_saved() (history_rl.c). + search Alt-P / Alt-N (stock non-incremental search), type a string, + abort with ^G or Rubout on an empty string: readline restored + the line over the search string's undo list. + Fixed by rl_nsearch_reclaim() (rl_nsearch.c). + +Each case is a fresh shell (in-process reader: the forked one discards its +heap with the child) run with detect_leaks=1; it must exit without a +report. The `works/` cases check the keys still do their job. On a binary +built without ASan nothing can be measured: SKIP, not pass. +""" +import os +import sys +import time + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), + "helpers")) +from ptyexpect import Tty # noqa: E402 + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +SHELL = os.path.abspath(sys.argv[1] if len(sys.argv) > 1 else + os.path.join(ROOT, "build", "bin", "hellish")) +FAILS = [] +UP, DOWN = "\x1b[A", "\x1b[B" +ALT_P, ALT_N = "\x1bp", "\x1bn" +HISTORY = ["echo alpha", "echo beta"] + + +def check(name, ok, detail=""): + print(("ok " if ok else "FAIL ") + name) + if not ok: + FAILS.append(name) + print(" " + detail) + + +def session(name, steps, want=None): + """A shell with HISTORY run, then each key group of `steps` sent and + given time to land, then exit. No leak report allowed; `want`, when + given, must be printed by the line the keys ran.""" + t = Tty(SHELL, rc="PS1='P$ '\nHISTFILE=/dev/null\n", + env={"ASAN_OPTIONS": "detect_leaks=1", + "HELLISH_HISTORY_PREFIX": "0"}) + ok = t.expect(b"P$ ") + for c in HISTORY: + t.send(c + "\r") + ok = ok and t.expect(b"P$ ") + mark = len(t.out) + for keys in steps: + t.send(keys) + t._read(0.25) + if want is not None: + ok = ok and t.expect(b"\n" + want.encode() + b"\r\n") + t.send("\x03") + ok = ok and t.expect(b"P$ ") + t.send("exit\r") + # LeakSanitizer reports as the shell exits: read until the pty closes, + # so the report is in t.out before anything is decided. + end = time.time() + 10 + while t._read(0.2) and time.time() < end: + pass + st = t.close() + leak = b"LeakSanitizer" in t.out + check(name, ok and not leak and os.WIFEXITED(st), + repr(t.since(mark)[-400:])) + + +def main(): + with open(SHELL, "rb") as f: + if b"AddressSanitizer" not in f.read(): + print("SKIP %s has no ASan: leaks cannot be seen" % SHELL) + return 0 + session("saved/up-then-enter", ["echo", UP, "\r"]) + session("saved/up-then-ctrl-c", ["echo", UP, "\x03"]) + session("saved/up-then-ctrl-g", ["ec", UP, "\x07"]) + session("saved/ctrl-p-then-enter", ["echo", "\x10", "\r"]) + session("saved/ctrl-p-walk-back-and-forth", + ["echo", "\x10", "\x10", "\x0e", "\x10", "\r"]) + session("search/alt-p-abort-ctrl-g", [ALT_P, "ech", "\x07"]) + session("search/alt-n-abort-ctrl-g", [ALT_N, "ech", "\x07"]) + session("search/alt-p-rubout-past-empty", [ALT_P, "e", "\x7f", "\x7f"]) + session("search/typed-line-then-abort", ["ls", ALT_P, "ech", "\x07"]) + session("search/typed-then-alt-p-enter", ["ech", ALT_P, "\r"]) + session("works/up-down-brings-the-typed-line-back", + ["echo typed", UP, DOWN, "\r"], want="typed") + session("works/alt-p-search-runs-the-match", + [ALT_P, "alph", "\r", "\r"], want="alpha") + session("works/aborted-search-restores-the-line", + ["echo kept", ALT_P, "x", "\x07", "\r"], want="kept") + print("%d failed" % len(FAILS)) + return 1 if FAILS else 0 + + +sys.exit(main())