Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resizing window crashes while editing file containing NUL and using showbreak and breakindentopt=shift:4 #8817

Closed
idbrii opened this issue Aug 29, 2021 · 8 comments

Comments

@idbrii
Copy link

idbrii commented Aug 29, 2021

Describe the bug
Vim crashes when resizing while editing a file containing a long line with null bytes and while using specific 'linebreak' settings.

I encountered this bug while vertically splitting my window while editing Steam's vdf files. I've narrowed it down to a minimal repro using application window resizing.

To Reproduce

minimal.vim:

set linebreak
set showbreak=>>
set breakindent
set breakindentopt=shift:4

" Setup test scenario
set lines=60
set columns=53
silent edit c:/scratch/broken.txt
" File must contain null bytes the string I've given uses 0 as
" placeholders.
nnoremap \\ :silent %s/0/<C-v>000/g
silent normal \\

" Trigger crash
set columns=52

c:/scratch/broken.txt:

0aaaaaaaaa00a0aaaaa0¯¸Ðùaaaaaaa0aaaaaaaaaaaa0aaa0"a:\aaaaa\aaaa\aaaaaa\aaaa\aaaaaa\aaaaaaaaaaaaa - aaaaaaaa\"0aaaaaaaa0"a

(see minimal for replacing 0 with NUL.)

Detailed steps to reproduce the behavior:

  1. Run gvim --clean -Nu minimal.vim

Actual behavior
Crash

Expected behavior
Window shrinks

Debugging

I attached a debugger and it's crashing on this line (might be the wrong version of the file)):

vim_memset(p, ' ', n_extra);

with this callstack:

 	gvim.exe!memset() Line 80	Unknown
>	gvim.exe!win_line(window_S * wp, long lnum, int startrow, int endrow, int nochange, int number_only) Line 2295	C
 	gvim.exe!win_update(window_S * wp) Line 2468	C
 	gvim.exe!update_screen(int type_arg) Line 321	C
 	gvim.exe!set_shellsize(int width, int height, int mustset) Line 3526	C
 	[Inline Frame] gvim.exe!shell_resized() Line 3390	C
 	gvim.exe!gui_resize_shell(int pixel_width, int pixel_height) Line 1556	C
 	gvim.exe!_OnSize(HWND__ * hwnd, unsigned int state, int cx, int cy) Line 2867	C
 	gvim.exe!_WndProc(HWND__ * hwnd, unsigned int uMsg, unsigned __int64 wParam, __int64 lParam) Line 4596	C
 	[External Code]	
 	gvim.exe!_WndProc(HWND__ * hwnd, unsigned int uMsg, unsigned __int64 wParam, __int64 lParam) Line 4603	C
 	[External Code]	
 	[Inline Frame] gvim.exe!MyWindowProc(HWND__ *) Line 1265	C
 	gvim.exe!_WndProc(HWND__ * hwnd, unsigned int uMsg, unsigned __int64 wParam, __int64 lParam) Line 4929	C
 	[External Code]	
 	[Inline Frame] gvim.exe!MyWindowProc(HWND__ *) Line 1265	C
 	gvim.exe!_WndProc(HWND__ * hwnd, unsigned int uMsg, unsigned __int64 wParam, __int64 lParam) Line 4929	C
 	[External Code]	
 	gvim.exe!process_message() Line 2033	C
 	gvim.exe!gui_mch_wait_for_chars(int wtime) Line 2152	C
 	[Inline Frame] gvim.exe!gui_wait_for_chars_3(long) Line 2920	C
 	gvim.exe!ui_wait_for_chars_or_timer(long wtime, int(*)(long, int *, int) interrupted, int *) Line 488	C
 	[Inline Frame] gvim.exe!gui_wait_for_chars_or_timer(long) Line 2935	C
 	gvim.exe!inchar_loop(unsigned char * buf, int maxlen, long wtime, int tb_change_cnt, int(*)(long, int *, int)) Line 384	C
 	gvim.exe!gui_wait_for_chars_buf(unsigned char * buf, int maxlen, long wtime, int tb_change_cnt) Line 2989	C
 	[Inline Frame] gvim.exe!gui_inchar(unsigned char *) Line 3019	C
 	gvim.exe!ui_inchar(unsigned char * buf, int maxlen, long wtime, int tb_change_cnt) Line 226	C
 	gvim.exe!inchar(unsigned char * buf, int maxlen, long wait_time) Line 3563	C
 	gvim.exe!vgetorpeek(int advance) Line 3342	C
 	gvim.exe!vgetc() Line 1690	C
 	[Inline Frame] gvim.exe!safe_vgetc() Line 1919	C
 	gvim.exe!normal_cmd(oparg_S * oap, int toplevel) Line 570	C
 	gvim.exe!main_loop(int cmdwin, int noexmode) Line 1504	C
 	gvim.exe!vim_main2() Line 882	C
 	[Inline Frame] gvim.exe!mzscheme_main() Line 955	C
 	gvim.exe!VimMain(int) Line 427	C
 	gvim.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInst, wchar_t * lpszCmdLine, int nCmdShow) Line 49	C
 	[External Code]	

Watch variables

Name Value Type
*p_extra 41 ')' unsigned char
c 94 int
n_extra -2 int
p_extra 0x00007ff71d4b21c8 ")" unsigned char *

Seems wrong that n_extra is negative, but I'm not sure that's causing the crash.

Environment :
VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Aug 22 2021 22:02:37)
MS-Windows 64-bit GUI version with OLE support
Included patches: 1-3367
(from vim/vim-win32-installer, installed via scoop)

Win10 x64

GUI

@chrisbra
Copy link
Member

yes, n_extra should not be negative. I wonder if this is caused by 8.2.2903. Can you check, if this patch fixes it:

diff --git a/src/drawline.c b/src/drawline.c
index deeaf5f16..fe56904db 100644
--- a/src/drawline.c
+++ b/src/drawline.c
@@ -1979,9 +1979,12 @@ win_line(

                    // We have just drawn the showbreak value, no need to add
                    // space for it again
-                   if (vcol == vcol_sbr)
+                   if (vcol == vcol_sbr && n_extra > 0)
                        n_extra -= MB_CHARLEN(get_showbreak_value(wp));

+                   if (n_extra < 0)
+                       n_extra = 0;
+
                    if (c == TAB && n_extra + col > wp->w_width)
 # ifdef FEAT_VARTABS
                        n_extra = tabstop_padding(vcol, wp->w_buffer->b_p_ts,

@idbrii
Copy link
Author

idbrii commented Aug 30, 2021

Unfortunately, I'm not building my own vim so I can't check your patch. I'm using the pdbs provided by vim-win32-installer to debug.

@brammool
Copy link
Contributor

That fix looks like it might solve the problem. I cannot reproduce the crash though. Can the example be turned into a test?

@dpelle
Copy link
Member

dpelle commented Aug 30, 2021

I tried to reproduce but I could not on Linux x86_64.
Is this maybe a Windows specific bug?

@brammool
Copy link
Contributor

I included the fix, since we don't want Vim to crash. Still, it would be good to have a test (which would fail without the fix).

@idbrii
Copy link
Author

idbrii commented Sep 4, 2021

I can confirm that the fix works. Updated to 8.2.3399 and can no longer reproduce. Thanks!

@idbrii
Copy link
Author

idbrii commented Sep 4, 2021

I tried writing a test, but couldn't figure out how to do the NUL insertion:

func Test_breakindent21_nullbytes()
  CheckFeature linebreak
  let s:input = '0aaaaaaaaa00a0aaaaa0¯¸Ðùaaaaaaa0aaaaaaaaaaaa0aaa0"a:\aaaaa\aaaa\aaaaaa\aaaa\aaaaaa\aaaaaaaaaaaaa - aaaaaaaa\"0aaaaaaaa0"a'
  call s:test_windows('setl linebreak showbreak=>> breakindent breakindentopt=shift:4')
  vertical resize 51

  " File must contain null bytes the string I've given uses 0 as
  " placeholders.
  " TODO: Neither \0 nor Ctrl-v 000 works from script. My mapping + normal command didn't work either.
  %s/0/\0/g
  %s/0/\^@/g

  " Trigger crash
  vertical resize 52
  call s:close_windows('set linebreak& showbreak& breakindent& breakindentopt&')
endfunc

@brammool
Copy link
Contributor

brammool commented Sep 5, 2021

Yegappan made a test for this, it was included with 8.2.3397

seandewar added a commit to seandewar/neovim that referenced this issue Sep 10, 2021
…ptions

Problem:    Crash with combination of 'linebreak' and other options.
Solution:   Avoid n_extra to become negative. (Christian Brabandt,
            closes vim/vim#8817)
vim/vim@20e0c3d
seandewar added a commit to seandewar/neovim that referenced this issue Sep 10, 2021
…ptions

Problem:    Crash with combination of 'linebreak' and other options.
Solution:   Avoid n_extra to become negative. (Christian Brabandt,
            closes vim/vim#8817)
vim/vim@20e0c3d
seandewar added a commit to seandewar/neovim that referenced this issue Sep 10, 2021
…ptions

Problem:    Crash with combination of 'linebreak' and other options.
Solution:   Avoid n_extra to become negative. (Christian Brabandt,
            closes vim/vim#8817)
vim/vim@20e0c3d
lewis6991 pushed a commit to lewis6991/neovim that referenced this issue Dec 12, 2021
…ptions

Problem:    Crash with combination of 'linebreak' and other options.
Solution:   Avoid n_extra to become negative. (Christian Brabandt,
            closes vim/vim#8817)
vim/vim@20e0c3d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants