Bug report
Bug description:
In readline.c, setup_readline probes libedit's indexing behaviour by calling add_history twice (with the strings "1" and "2"), replacing one entry via replace_history_entry, and then calling clear_history to clean up.
|
/* Detect if libedit's readline emulation uses 0-based |
|
* indexing or 1-based indexing. |
|
*/ |
|
add_history("1"); |
|
if (history_get(1) == NULL) { |
|
libedit_history_start = 0; |
|
} else { |
|
libedit_history_start = 1; |
|
} |
|
/* Some libedit implementations use 1 based indexing on |
|
* replace_history_entry where libreadline uses 0 based. |
|
* The API our module presents is supposed to be 0 based. |
|
* It's a mad mad mad mad world. |
|
*/ |
|
{ |
|
add_history("2"); |
|
HIST_ENTRY *old_entry = replace_history_entry(1, "X", NULL); |
|
_py_free_history_entry_lock_held(old_entry); |
|
HIST_ENTRY *item = history_get(libedit_history_start); |
|
if (item && item->line && strcmp(item->line, "X")) { |
|
libedit_append_replace_history_offset = 0; |
|
} else { |
|
libedit_append_replace_history_offset = 1; |
|
} |
|
} |
|
clear_history(); |
|
|
|
using_history(); |
|
|
|
/* Force rebind of TAB to insert-tab */ |
|
rl_bind_key('\t', rl_insert); |
|
/* Bind both ESC-TAB and ESC-ESC to the completion function */ |
|
rl_bind_key_in_map ('\t', rl_complete, emacs_meta_keymap); |
|
rl_bind_key_in_map ('\033', rl_complete, emacs_meta_keymap); |
|
#ifdef HAVE_RL_RESIZE_TERMINAL |
|
/* Set up signal handler for window resize */ |
|
sigwinch_ohandler = PyOS_setsig(SIGWINCH, readline_sigwinch_handler); |
|
#endif |
On macOS, Apple's libedit clear_history does not free the line field (the internally strdup'd string) for all history entries, so one 2-byte allocation escapes. The replaced entry's old value is correctly freed by _py_free_history_entry_lock_held, but clear_history leaks one of the remaining strdup'd probe strings.
This leak (discovered while running unit tests for a Python package with LSan) is reported by Leak Sanitizer as:
Direct leak of 2 byte(s) in 1 object(s) allocated from:
#0 0x000101f3177c in strdup+0xfc (libclang_rt.asan_osx_dynamic.dylib:arm64+0x4d77c)
#1 0x0001dbc62ab0 (libedit.3.dylib:arm64e+0x14ab0)
#2 0x0001dbc63410 in history+0x6e4 (libedit.3.dylib:arm64e+0x15410)
#3 0x0001dbc5853c in add_history+0x50 (libedit.3.dylib:arm64e+0xa53c)
#4 0x00010b7b0ad4 in setup_readline readline.c:1373
#5 0x00010b7b0ad4 in PyInit_readline readline.c:1679
#6 0x000100ee935c in _PyImport_RunModInitFunc importdl.c:436
#7 0x000100ee36ac in import_run_extension import.c:2164
#8 0x000100ee6b90 in _imp_create_dynamic_impl import.c:5503
#9 0x000100ee6b90 in _imp_create_dynamic import.c.h:488
#10 0x000100aa5ffc in _PyVectorcall_Call call.c:273
#11 0x000100e01f68 in _PyEval_EvalFrameDefault generated_cases.c.h:2724
#12 0x000100dfc064 in _PyEval_EvalFrame pycore_ceval.h:122
#13 0x000100dfc064 in _PyEval_Vector ceval.c:2156
#14 0x000100aaa7c0 in _PyObject_VectorcallTstate pycore_call.h:144
#15 0x000100aaa7c0 in object_vacall call.c:823
#16 0x000100aaa29c in PyObject_CallMethodObjArgs call.c:960
#17 0x000100edf110 in import_find_and_load import.c:4125
#18 0x000100ede4a8 in PyImport_ImportModuleLevelObject import.c
#19 0x000100e37658 in _PyEval_ImportNameWithImport ceval.c:3011
#20 0x000100e37658 in _PyEval_ImportName ceval.c:2990
#21 0x000100e104ec in _PyEval_EvalFrameDefault generated_cases.c.h:6768
#22 0x000100af1004 in _PyEval_EvalFrame pycore_ceval.h:122
#23 0x000100af1004 in gen_send_ex2 genobject.c:280
#24 0x000100af1004 in gen_send_ex genobject.c:373
#25 0x000100aed464 in gen_iternext genobject.c:764
#26 0x000100df48cc in builtin_next bltinmodule.c:1764
#27 0x000100e14854 in _Py_BuiltinCallFast_StackRef ceval.c:824
#28 0x000100e14854 in _PyEval_EvalFrameDefault generated_cases.c.h:2420
#29 0x000100dfc064 in _PyEval_EvalFrame pycore_ceval.h:122
#30 0x000100dfc064 in _PyEval_Vector ceval.c:2156
#31 0x000100aa489c in _PyObject_VectorcallDictTstate call.c:146
#32 0x000100aa7130 in _PyObject_Call_Prepend call.c:504
#33 0x000100c292ec in call_method typeobject.c:3095
#34 0x000100c292ec in slot_tp_call typeobject.c:10913
#35 0x000100aa4d90 in _PyObject_MakeTpCall call.c:242
#36 0x000100dfce0c in _Py_VectorCallInstrumentation_StackRefSteal ceval.c:775
#37 0x000100e1336c in _PyEval_EvalFrameDefault generated_cases.c.h:3325
#38 0x000100dfc064 in _PyEval_EvalFrame pycore_ceval.h:122
#39 0x000100dfc064 in _PyEval_Vector ceval.c:2156
#40 0x000100aa9208 in _PyObject_VectorcallTstate pycore_call.h:144
#41 0x000100aa9208 in _PyObject_VectorcallPrepend call.c:877
SUMMARY: AddressSanitizer: 2 byte(s) leaked in 1 allocation(s).
That leak can easily by running the following under an LSan-enabled CPython on macOS.
ASAN_OPTIONS=detect_leaks=1 ./python.exe -c "import readline"
LLM-reviewing this issue surfaces two other issues around that code:
-
The probing block runs unconditionally even on GNU readline builds, where libedit_history_start and libedit_append_replace_history_offset are never read (every use of those variables is guarded by if (using_libedit_emulation)). This means add_history/clear_history are called unnecessarily on every readline import on Linux.
-
There is no null check before calling _py_free_history_entry_lock_held(old_entry) on the return value of replace_history_entry, which can return NULL if the index is out of range.
Proposed fix
- Guard the entire probing block with
if (using_libedit_emulation).
- Replace
clear_history inside that block with an explicit remove_history(libedit_history_start) loop that calls _py_free_history_entry_lock_held on each returned entry, bypassing the libedit bug.
- Keep the unconditional
clear_history call outside the block for readline state reset.
CPython versions tested on:
CPython main branch
Operating systems tested on:
macOS
Linked PRs
Bug report
Bug description:
In
readline.c,setup_readlineprobes libedit's indexing behaviour by callingadd_historytwice (with the strings"1"and"2"), replacing one entry viareplace_history_entry, and then callingclear_historyto clean up.cpython/Modules/readline.c
Lines 1360 to 1397 in 629da5c
On macOS, Apple's libedit
clear_historydoes not free thelinefield (the internallystrdup'd string) for all history entries, so one 2-byte allocation escapes. The replaced entry's old value is correctly freed by_py_free_history_entry_lock_held, butclear_historyleaks one of the remainingstrdup'd probe strings.This leak (discovered while running unit tests for a Python package with LSan) is reported by Leak Sanitizer as:
That leak can easily by running the following under an LSan-enabled CPython on macOS.
ASAN_OPTIONS=detect_leaks=1 ./python.exe -c "import readline"LLM-reviewing this issue surfaces two other issues around that code:
The probing block runs unconditionally even on GNU readline builds, where
libedit_history_startandlibedit_append_replace_history_offsetare never read (every use of those variables is guarded byif (using_libedit_emulation)). This meansadd_history/clear_historyare called unnecessarily on every readline import on Linux.There is no null check before calling
_py_free_history_entry_lock_held(old_entry)on the return value ofreplace_history_entry, which can returnNULLif the index is out of range.Proposed fix
if (using_libedit_emulation).clear_historyinside that block with an explicitremove_history(libedit_history_start)loop that calls_py_free_history_entry_lock_heldon each returned entry, bypassing the libedit bug.clear_historycall outside the block for readline state reset.CPython versions tested on:
CPython main branch
Operating systems tested on:
macOS
Linked PRs
readline#150537