Problem: nvim_win_set_config may crash when attempting to move curwin to a
different tabpage if there is no other non-float available to switch to.
Solution: fix the crash. Fix ONE_WINDOW checks in winframe_find_altwin and
win_altframe to consider floating windows by instead using one_window. Allow
one_window to consider non-current tabpages. We can use one_window in
win_close_othertab now to also better reflect its use in win_close.
Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Problem: unlike win_close, win_close_othertab could be used to close the
autocommand window from a different tabpage. This causes aucmd_restbuf to close
the wrong window, potentially causing a crash.
Solution: disallow closing it. Also replace a deprecated use of exc_exec in the
test file.
Fixes#21409.
Problem: no check for nvim_open_win opening a new floating window into a closing
buffer, which can lead to crashes.
Solution: call check_split_disallowed; opening a new float semantically splits
from a window, so the same problems as regular splits apply. Also restore the
error if switch_win_noblock in win_set_buf fails (may not be possible to hit
this, but win_set_buf can silently succeed there since #31595).
As the lock check applies to curbuf (not the target buffer) this may feel a bit
restrictive, though this isn't different to how things like ":split" or
nvim_open_win with "split = true" works when targeting a different buffer. Only
checking the target buffer's lock will cause issues if win_set_buf doesn't end
up in the target buffer for whatever reason.
Maybe we could consider checking the lock of the buffer after win_set_buf and
close the window if it's locked (maybe a bit fiddly, especially as closing a
window can fail...), or make the open + switch operation more atomic, like how
Vim does for its popup windows..?
It also used to be the case that win_set_buf would set an error if autocommands
sent us to a different buffer than what was requested, but #31595 appears to
have also changed that... I haven't touched that here.
Problem: can't accurately know if close_buffer directly (e.g: not via autocmds)
decremented b_nwindows. This can cause crashes if win_close_othertab decides to
keep the window after calling close_buffer (if it did not free the buffer), as
b_nwindows may remain out-of-sync.
Solution: change the return value of close_buffer to accurately depict whether
it decremented b_nwindows. Check it in win_close_othertab to avoid a crash.
Similar issues may exist in other places that call close_buffer, but I've not
addressed those here (not to mention only one other place even checks its return
value...)
Problem: TabClosed is fired after close_buffer is called (after b_nwindows is
decremented) and after the tab page is removed from the list, but before it's
freed. This causes inconsistencies such as the removed tabpage having a valid
handle and functions like nvim_tabpage_get_number returning nonsense.
Solution: fire it after free_tabpage. Try to maintain the Nvim-specific
behaviour of setting `<amatch>` to the old tab page number, and the
(undocumented) behaviour of setting `<abuf>` to the buffer it was showing
(close_buffer sets w_buffer to NULL if it was freed, so it should be OK pass it
to apply_autocmds_group, similar to before).
Problem: No check for closing the only non-floating window in a non-current
tabpage that contains floats. This can lead to a tabpage that contains only
floats, causing crashes.
Solution: Copy the relevant check from win_close to win_close_othertab. Fix some
uncovered issues.
Closes#34943Fixes#31236
Co-authored-by: glepnir <glephunter@gmail.com>
Problem: Crash when a terminal receives OSC 2 just after closing its
buffer but before terminal job exits.
Solution: Remove FUNC_ATTR_NONNULL_ALL from buf_set_term_title() and
check for NULL.
(cherry picked from commit b67ac8cc6b)
Problem: null pointer member access when closing the only non-float in the
current tab page if autocommands after closing all floats also close all other
tab pages. (making it the last window)
Solution: check last_window again after closing the floats.
Also reduce the scope of "wp"; it would be bugprone to use it before it's later
reassigned to the rv of win_free_mem if freed by Buf/WinLeave.
(cherry picked from commit c14de47f1a)
Problem: Destroying a terminal with pending TermRequest leads to
heap-use-after-free when processing TermRequest afterwards.
Solution: Store the buffer handle instead of the Terminal pointer in the
pending TermRequest event, and don't emit TermRequest if the
terminal is already closed.
Problem: When switching to another terminal buffer in Terminal mode,
usually Nvim stays in Terminal mode, but leaves Terminal mode
if the old terminal buffer was deleted.
Solution: Don't always leave Terminal mode when active terminal buffer
is deleted. Instead let terminal_check_focus() decide that.
(cherry picked from commit 3621af9b97)
With `overlap=true`, more extmarks than the requested limit may be
collected in `extmark_get`. This then leads to an out of bounds write of
`rv` in `nvim_buf_get_extmarks`.
(cherry picked from commit 612cd99a00)
Problem: Buffer overflow in buf_write() when converting incomplete
multi-byte characters (Kevin Goodsell)
Solution: Make the buffer slightly larger
closes: vim/vim#19007f99de42a9f
Co-authored-by: Christian Brabandt <cb@256bit.org>
(cherry picked from commit 444e1ffe3e)
Problem: empty comma-separated patterns in an aupat aren't skipped correctly.
Solution: skip consecutive commas in an aupat.
Also simplify the logic.
(cherry picked from commit 1cde71233f)
Problem: patterns after a buffer-local pattern in a comma-separated aupat are
ignored.
Solution: ensure pat refers to the original buffer after each pattern, not the
buflocal_pat buffer, and when printing make sure it's normalized for
each pattern, not just the first.
Also simplify the logic when printing all autocommands for an event, and ensure
headings aren't repeated for each event when printing autocommands.
(cherry picked from commit 6d2330f50d)
Problem: use-after-free in win_move_after if win_enter autocommands free win1/2.
Solution: set w_pos_changed before calling win_enter.
(cherry picked from commit d1189ea508, also adding
an import of "exec" in the test)
Background:
Suppose a window has concealed lines, and sets conceallevel>2,
concealcursor="". The concealed lines are displayed if the window is
curwin and the cursor is on the those lines.
Problem:
line('w$', win) switches curwin to win, and then does validate_botline
for curwin. It computes botline assuming the concealed lines displayed,
resulting in a smaller value than the actual botline that the user sees.
Solution:
Evaluate line('w$', win) without switching curwin.
Apply similar changes to other functions that switches curwin.
Co-authored-by: zeertzjq <zeertzjq@outlook.com>
(cherry picked from commit 033f1123cd)
Problem: Wrong cursor position after formatting with long 'formatprg'.
Solution: Don't show hit-enter prompt when there are stuffed characters.
Previously a stuffed character at the hit-enter prompt will dismiss the
prompt immediately and be put in the typeahead buffer, which leads to
incorrect behavior as the typeahead buffer is processed after the stuff
buffers. Using vungetc() when KeyStuffed is TRUE can fix this problem,
but since the hit-enter prompt isn't visible anyway (and is likely not
desired here), just skip the prompt instead, which also avoids a wait
when using "wait" instead of "hit-enter" in 'messagesopt'.
fixes: vim/vim#18905closes: vim/vim#1890650325c3d59
(cherry picked from commit 18642a63be)
fix(lua): relax `vim.wait()` timeout validation #36900
Problem:
After bc0635a9fc `vim.wait()` rejects floats
and NaN values.
Solution:
Restore the prior behavior, while still supporting `math.huge`. Update
tests to cover float case.
(cherry picked from commit b87bdef2a8)
Problem:
`nlua_wait()` uses `luaL_checkinteger()` which doesn't support
`math.huge` since it's double type. On PUC Lua this fails with
'number has no integer representation' error and on LuaJIT this
overflows int.
Solution:
Use `luaL_checknumber()` and handle `math.huge`.
(cherry picked from commit bc0635a9fc)
Problem: sort() does not handle large numbers correctly
(Igbanam Ogbuluijah)
Solution: Don't truncate the return value of tv_get_number_chk()
(Yegappan Lakshmanan)
closes: vim/vim#1886804794efe12
Use a Lua test for now, as the Vimscript test uses tuples.
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
(cherry picked from commit 5370b7a2e0)
backport https://github.com/neovim/neovim/pull/36746
Problem:
Currently we fetch a prebuilt xxd.exe from neovim/deps for Windows,
which is not ideal in terms of obviousness, misses updates, and is
clumsy to update.
Solution:
Similar to tee.c (#36363), vendor xxd.c from Vim and build it as part
of the Neovim build process. This makes the source obvious, enables
analyzer checks, and simplifies updates.
Fixes#36664
Co-authored-by: benarcher2691 <ben.archer2691@gmail.com>
Problem: apply_autocmds function can free both buf_T and win_T pointers
Solution: instead retain winids for WinResized and WinScrolled
autocmds and use curbuf pointer, which is consistent with other uses
of apply_autocmds function
(cherry picked from commit 0a0c349b6f)
Problem: `extmark_splice()` was being called before `ml_replace()`,
which caused the on_bytes callback to be invoked with the old buffer
text instead of the new text.
Solution: store metadata for each match in a growing array, call
`ml_replace()` once to update the buffer, then call `extmark_splice()`
once per match.
Closes https://github.com/neovim/neovim/issues/36370.
(cherry picked from commit 7da4d6abe2)
Limit the default truncation item to the current recursion range so
nested `nvim_eval_statusline()` calls don't reuse stale `stl_items`
pointers. Add a functional regression that evaluates a Lua statusline
helper which forces truncation to ensure the nested scenario stays
stable.
AI-Assist: OpenAI ChatGPT
Fixes#36616
(cherry picked from commit e9b6474ae7)
Problem:
In socket_connect(), if connecting to the given TCP address times out,
libuv is still trying to connect to the address, and connect_cb may be
called when running the libuv event loop after the `status` variable
referenced by `req.data` goes out of scope.
Solution:
Close the uv_tcp_t handle and wait for connect_cb to be called before
retrying or failing in socket_connect(). This also avoid leaking libuv
handles.
The tests added here only check that the non-timeout case still works,
as checking the timeout case is very hard without modifications to the
code. Removing the first LOOP_PROCESS_EVENT_UNTIL() in socket_connect()
(the one with the timeout) is a way to check that manually.
Also add a comment about the cause of the ASAN error in #34586.
Problem: Closing socket with pending writes leaks memory.
Analysis: When calling rstream_may_close() on an RStream with pending
writes, it waits for write_cb() to call stream_close_handle(),
which closes it as if it's a WStream and doesn't free the
RStream's buffer.
Solution: Ensure that an RStream's buffer is freed when it's closed.
Problem: TextChangedT fires depending on whether Nvim needs to update_screen
while in terminal mode. This makes little sense as redraws can be completely
unrelated to the terminal. Also, TextChanged could be fired from changes in
terminal mode after returning to normal mode.
Solution: trigger it when b:changedtick changes, like other such events. Happens
when invalid cells are refreshed, though is no longer affected by cursor
changes. Don't fire TextChanged from changes in terminal mode after leaving.
Unlike the other TextChanged* events, I've elected to not have it be influenced
by typeahead. Plus, unlike when leaving insert mode when no TextChangedI events
are defined, I don't trigger TextChanged when returning to normal mode from
changes in terminal mode (is that a Vim bug?)
Curiously, Vim's TextChangedT is different; it's tied to its terminal cursor
redraws, which triggers pretty eagerly (but is unaffected by unrelated redraws)
- usually *twice* when data is sent to the terminal (regardless of whether it
causes any visible changes, like incomplete escape codes; wasn't true for Nvim).
Not clear to me how this event was actually intended to work, but this seems to
make the most sense to me.
Problem: w_wrow/col calculation in terminal_check_cursor is wrong when the
terminal is smaller than the window. Common when there's a larger window open
with the same terminal, or just after a resize (as refresh_size is deferred).
Solution: don't calculate it; validate_cursor will correct it later if it's
out-of-date.
Note that the toplines set for the terminal (also before this PR) assume
'nowrap' (which is set by default for terminal windows), and that no weird stuff
like filler lines are around. That means, for example, it's possible for the
cursor to be moved off-screen if there's wrapped lines. If this happens, it's
likely update_topline will move the cursor back on screen via validate_cursor or
similar, but maybe this should be handled more elegantly in the future?
Problem: autocommands can cause various problems in terminal mode, which can
lead to crashes, for example.
Solution: fix found issues. Move some checks to terminal_check and guard against
autocommands messing with things. Trigger TermEnter/Leave after terminal mode
has changed/restored most state. Wipeout the correct buffer if TermLeave
switches buffers and fix a UAF if it or WinScrolled/Resized frees the terminal
prematurely.
These changes also allow us to remove the buffer restrictions on TextChangedT;
they were inadequate in stopping some issues, and WinScrolled/Resized was
lacking them anyway.
Problem: when creating a new tabpage with a terminal window, terminal size is
not updated if there is no statusline.
Solution: do not rely on last_status to implicitly call terminal_check_size as a
side effect of making room for a statusline; call it directly.
Problem: topline of a focused terminal window may not tail to terminal output if
events scroll the window.
Solution: set the topline in terminal_check_cursor.
Problem: missing redraws when restoring saved cursorline/column, plus missing
statusline and mode redraws when not updating the screen in terminal mode.
Solution: schedule the redraws in a similar manner to other modes and remove
some now unnecessary redrawing logic. Redraw if cursorline-related options
change from entering terminal mode. This fixes test failures in later commits.
WTF: TextChangedT triggers based on must_redraw, which is... fun...? Try to
preserve its behaviour as much as we can for now.
Problem: in terminal mode, adjust_topline moves curwin's cursor to the last row
so set_topline tails the non-scrollback area. This may result in the observed
cursor position remaining tailed in events within the focused terminal, rather
than reflecting the actual cursor position.
Solution: use the focused terminal's actual cursor position immediately, rather
than relying on the next terminal_check_cursor call in terminal_check to set it.
Note: Maybe also possible for terminal mode cursor position to be stale
(reporting the normal mode position) when switching buffers in events to another
terminal until the next terminal_check call? (or until refresh_terminal is
called for it) Maybe not worth fixing that, though.
