From b91613f42c33b2dd39013c8dca8c1efed7ac0986 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Sun, 20 Jul 2025 21:11:43 +0800 Subject: [PATCH] vim-patch:9.1.1567: crash when using inline diff mode (#35005) Problem: Crash when using inline diff mode (Ilya Grigoriev) Solution: Set tp_diffbuf to NULL when skipping a diff block (Yee Cheng Chin). Fix an array out of bounds crash when using diffopt+=inline:char when 4 or more buffers are being diff'ed. This happens when one of the blocks is empty. The inline highlight logic skips using that buffer's block, but when another buffer is used later and calls diff_read() to merge the diff blocks together, it could erroneously consider the empty block's diff info which has not been initialized, leaving to diff numbers that are invalid. Later on the diff num is used without bounds checking which leads to the crash. Fix this by making sure to unset tp_diffbuf to NULL when we skip a block, so diff_read() will not consider this buffer to be used within inline diff. Also, add more bounds checking just to be safe. closes: vim/vim#17805 https://github.com/vim/vim/commit/c8b99e2d139cf72c567892e44939f2719f703fa8 Co-authored-by: Yee Cheng Chin --- src/nvim/diff.c | 14 ++++++++++++-- test/functional/ui/diff_spec.lua | 26 ++++++++++++++++++++++++++ test/old/testdir/test_diffmode.vim | 16 ++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/nvim/diff.c b/src/nvim/diff.c index a3b5998271..2882d52723 100644 --- a/src/nvim/diff.c +++ b/src/nvim/diff.c @@ -3092,6 +3092,11 @@ static void diff_find_change_inline_diff(diff_T *dp) diff_T *orig_diff = curtab->tp_first_diff; curtab->tp_first_diff = NULL; + // diff_read() also uses curtab->tp_diffbuf to determine what's an active + // buffer + buf_T *(orig_diffbuf[DB_COUNT]); + memcpy(orig_diffbuf, curtab->tp_diffbuf, sizeof(orig_diffbuf)); + garray_T linemap[DB_COUNT]; garray_T file1_str; garray_T file2_str; @@ -3118,8 +3123,12 @@ static void diff_find_change_inline_diff(diff_T *dp) continue; // skip buffer that isn't loaded } if (dp->df_count[i] == 0) { - continue; // skip buffer that don't have any texts in this block + // skip buffers that don't have any texts in this block so we don't + // end up marking the entire block as modified in multi-buffer diff + curtab->tp_diffbuf[i] = NULL; + continue; } + if (file1_idx == -1) { file1_idx = i; } @@ -3297,7 +3306,7 @@ static void diff_find_change_inline_diff(diff_T *dp) for (; new_diff != NULL; new_diff = new_diff->df_next) { diffline_change_T change = { 0 }; for (int i = 0; i < DB_COUNT; i++) { - if (new_diff->df_lnum[i] == 0) { + if (new_diff->df_lnum[i] <= 0) { // should never be < 0. Checking just for safety continue; } linenr_T diff_lnum = new_diff->df_lnum[i] - 1; // use zero-index @@ -3334,6 +3343,7 @@ done: diff_clear(curtab); curtab->tp_first_diff = orig_diff; + memcpy(curtab->tp_diffbuf, orig_diffbuf, sizeof(orig_diffbuf)); ga_clear(&file1_str); ga_clear(&file2_str); diff --git a/test/functional/ui/diff_spec.lua b/test/functional/ui/diff_spec.lua index 481ead8eed..a50b3d07d1 100644 --- a/test/functional/ui/diff_spec.lua +++ b/test/functional/ui/diff_spec.lua @@ -2801,6 +2801,32 @@ it('diff mode inline highlighting with 3 buffers', function() screen:expect(s6) end) +-- oldtest: Test_diff_inline_multibuffer_empty_block() +it('diff mode inline highlighting with multi-buffer and empty block', function() + write_file('Xdifile1', 'anchor1\n1234567890abcde\nanchor2') + write_file('Xdifile2', 'anchor1\n1234567--0abc-e\nanchor2') + write_file('Xdifile3', 'anchor1\nanchor2') + write_file('Xdifile4', 'anchor1\n1???????90abcd?\nanchor2') + finally(function() + os.remove('Xdifile1') + os.remove('Xdifile2') + os.remove('Xdifile3') + os.remove('Xdifile4') + end) + local screen = Screen.new(83, 20) + command('args Xdifile1 Xdifile2 Xdifile3 Xdifile4 | vert all | windo diffthis') + command('1wincmd w | set diffopt=filler,internal,inline:char') + screen:expect([[ + {7: }^anchor1 │{7: }anchor1 │{7: }anchor1 │{7: }anchor1 | + {7: }{4:1}{27:23456789}{4:0abc}{27:de}{4: }│{7: }{4:1}{27:234567--}{4:0abc}{27:-e}{4: }│{7: }{23:------------------}│{7: }{4:1}{27:???????9}{4:0abc}{27:d?}{4: }| + {7: }anchor2 │{7: }anchor2 │{7: }anchor2 │{7: }anchor2 | + {1:~ }│{1:~ }│{1:~ }│{1:~ }|*15 + {3:Xdifile1 }{2:Xdifile2 Xdifile3 Xdifile4 }| + | + ]]) + n.assert_alive() +end) + it('diff mode algorithm:histogram and inline:char with long lines #34329', function() local screen = Screen.new(55, 20) exec([[ diff --git a/test/old/testdir/test_diffmode.vim b/test/old/testdir/test_diffmode.vim index df9fb805ab..6a4c923900 100644 --- a/test/old/testdir/test_diffmode.vim +++ b/test/old/testdir/test_diffmode.vim @@ -2384,6 +2384,22 @@ func Test_diff_inline_multibuffer() call StopVimInTerminal(buf) endfunc +" Test that when using multi-buffer diff, an empty block would be correctly +" skipped in the result, without resulting in invalid states or crashes. +func Test_diff_inline_multibuffer_empty_block() + CheckScreendump + + call writefile(['anchor1', '1234567890abcde', 'anchor2'], 'Xdifile1') + call writefile(['anchor1', '1234567--0abc-e', 'anchor2'], 'Xdifile2') + call writefile(['anchor1', 'anchor2'], 'Xdifile3') + call writefile(['anchor1', '1???????90abcd?', 'anchor2'], 'Xdifile4') + + let buf = RunVimInTerminal('-d Xdifile1 Xdifile2 Xdifile3 Xdifile4', {}) + call VerifyInternal(buf, "Test_diff_inline_multibuffer_empty_block_01", " diffopt+=inline:char") + + call StopVimInTerminal(buf) +endfunc + func Test_diffget_diffput_linematch() CheckScreendump call delete('.Xdifile1.swp')