Repository navigation
fix(editor): prefix search and suggestion keys on non-PIE builds - #179
Merged
Merged
Conversation
On the 42 machines Up stayed plain history and Right / End / ^F / Alt-F / Alt-N never touched the ghost: none of #136's or #140's keys were installed. rl_slot_arm() decides whether a slot still holds readline's stock function by comparing pointers, and took the stock ones by address (&rl_get_previous_history, ...). cc there is clang 12, which links non-PIE, where such an address is the executable's PLT stub; Ubuntu's libreadline is linked -Bsymbolic-functions and keeps its own internal address in the keymaps. The two never compared equal, every slot read as a user binding, and slot_user kept it that way for the session. gcc (CI) and the static release build link PIE, so neither showed it. rl_slot_funcs() now asks readline for its functions by name (rl_named_function), the pointer its keymaps and funmap hold, for the stock side and for the history-search pair that replaces Up / Down. The latter also matters for readline's own rl_last_func check: a search bound by stub address would restart on every Up instead of walking back. Verified on a non-PIE debug build (file: "LSB executable"), both readers: history_prefix_search_test 57 failed -> 148 ok / 1 failed, the one being bind -p against the oracle's bundled readline 8.3 (fails on develop too); autosuggest_test 86 failed -> 346 ok / 0 failed. A PIE build passes before and after, so CI cannot see this without a non-PIE job.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On the 42 machines none of #136's / #140's keys were installed: Up stayed plain history; Right / End / ^F / Alt-F / Alt-N ignored the ghost.
Root cause:
rl_slot_arm()compares the function in a slot with readline's stock one, taken by address.ccthere is clang 12 (non-PIE), so&rl_get_previous_historyis the PLT stub, while Ubuntu's-Bsymbolic-functionslibreadline keeps its internal address in the keymaps. Never equal → every slot looked user-bound. CI (gcc, PIE) and the static release can't show it.Fix:
rl_slot_funcs()gets readline's functions withrl_named_function(), for the stock side and for the history-search pair (whoserl_last_funccheck also needs the real pointer to walk back on repeated Up).Non-PIE debug build, both readers:
*
bind -pvs the oracle's bundled readline 8.3; fails on develop too.Follow-up worth considering: a non-PIE CI job running these two tests.
🤖 Generated with Claude Code