Skip to content

Commit c8b99e2

Browse files
ychinchrisbra
ychin
authored and
chrisbra
committed
patch 9.1.1567: crash when using inline diff mode
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: #17805 Signed-off-by: Yee Cheng Chin <ychin.git@gmail.com> Signed-off-by: Christian Brabandt <cb@256bit.org>
1 parent 0e40501 commit c8b99e2

File tree

4 files changed

+51
-2
lines changed

4 files changed

+51
-2
lines changed

‎src/diff.c‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3391,6 +3391,11 @@ diff_find_change_inline_diff(
33913391
diff_T *orig_diff = curtab->tp_first_diff;
33923392
curtab->tp_first_diff = NULL;
33933393

3394+
// diff_read() also uses curtab->tp_diffbuf to determine what's an active
3395+
// buffer
3396+
buf_T *(orig_diffbuf[DB_COUNT]);
3397+
memcpy(orig_diffbuf, curtab->tp_diffbuf, sizeof(orig_diffbuf));
3398+
33943399
// Buffers to populate mmfile 1/2 that would be passed to xdiff as memory
33953400
// files. Use a grow array as it is not obvious how much exact space we
33963401
// need.
@@ -3412,7 +3417,12 @@ diff_find_change_inline_diff(
34123417
continue; // skip buffer that isn't loaded
34133418

34143419
if (dp->df_count[i] == 0)
3415-
continue; // skip buffer that don't have any texts in this block
3420+
{
3421+
// skip buffers that don't have any texts in this block so we don't
3422+
// end up marking the entire block as modified in multi-buffer diff
3423+
curtab->tp_diffbuf[i] = NULL;
3424+
continue;
3425+
}
34163426

34173427
if (file1_idx == -1)
34183428
file1_idx = i;
@@ -3626,7 +3636,7 @@ diff_find_change_inline_diff(
36263636
CLEAR_FIELD(change);
36273637
for (int i = 0; i < DB_COUNT; i++)
36283638
{
3629-
if (new_diff->df_lnum[i] == 0)
3639+
if (new_diff->df_lnum[i] <= 0) // should never be < 0. Checking just for safety.
36303640
continue;
36313641
linenr_T diff_lnum = new_diff->df_lnum[i] - 1; // use zero-index
36323642
linenr_T diff_lnum_end = diff_lnum + new_diff->df_count[i];
@@ -3675,6 +3685,7 @@ diff_find_change_inline_diff(
36753685

36763686
diff_clear(curtab);
36773687
curtab->tp_first_diff = orig_diff;
3688+
memcpy(curtab->tp_diffbuf, orig_diffbuf, sizeof(orig_diffbuf));
36783689

36793690
ga_clear(&file1_str);
36803691
ga_clear(&file2_str);

‎src/testdir/dumps/Test_diff_inline_multibuffer_empty_block_01.dump‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
| +0#0000e05#a8a8a8255@1|a+0#0000000#ffffff0|n|c|h|o|r|1| @10||+1&&| +0#0000e05#a8a8a8255@1|a+0#0000000#ffffff0|n|c|h|o|r|1| @7||+1&&| +0#0000e05#a8a8a8255@1|a+0#0000000#ffffff0|n|c|h|o|r|1| @8||+1&&| +0#0000e05#a8a8a8255@1|a+0#0000000#ffffff0|n|c|h|o|r|1| @7
2+
| +0#0000e05#a8a8a8255@1|1+0#0000000#ffd7ff255|2+2&#ff404010|3|4|5|6|7|8|9|0+0&#ffd7ff255|a|b|c|d+2&#ff404010|e| +0&#ffd7ff255@2||+1&#ffffff0| +0#0000e05#a8a8a8255@1|1+0#0000000#ffd7ff255|2+2&#ff404010|3|4|5|6|7|-@1|0+0&#ffd7ff255|a|b|c|-+2&#ff404010|e||+1&#ffffff0| +0#0000e05#a8a8a8255@1|-+0#4040ff13#afffff255@15||+1#0000000#ffffff0| +0#0000e05#a8a8a8255@1|1+0#0000000#ffd7ff255|?+2&#ff404010@6|9|0+0&#ffd7ff255|a|b|c|d+2&#ff404010|?
3+
| +0#0000e05#a8a8a8255@1|a+0#0000000#ffffff0|n|c|h|o|r|2| @10||+1&&| +0#0000e05#a8a8a8255@1|a+0#0000000#ffffff0|n|c|h|o|r|2| @7||+1&&| +0#0000e05#a8a8a8255@1|a+0#0000000#ffffff0|n|c|h|o|r|2| @8||+1&&| +0#0000e05#a8a8a8255@1|a+0#0000000#ffffff0|n|c|h|o|r|2| @7
4+
|~+0#4040ff13&| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
5+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
6+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
7+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
8+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
9+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
10+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
11+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
12+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
13+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
14+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
15+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
16+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
17+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
18+
|~| @18||+1#0000000&|~+0#4040ff13&| @15||+1#0000000&|~+0#4040ff13&| @16||+1#0000000&|~+0#4040ff13&| @15
19+
|X+3#0000000&|d|i|f|i|l|e|1| @1|1|,|1| @3|A|l@1| |<+1&&|d|i|f|i|l|e|2| |1|,|1| @1|A|l@1| |<|d|i|f|i|l|e|3| |1|,|1| @2|A|l@1| |<|d|i|f|i|l|e|4| |1|,|1| @1|A|l@1
20+
|:+0&&> @73

‎src/testdir/test_diffmode.vim‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2636,6 +2636,22 @@ func Test_diff_inline_multibuffer()
26362636
call StopVimInTerminal(buf)
26372637
endfunc
26382638

2639+
" Test that when using multi-buffer diff, an empty block would be correctly
2640+
" skipped in the result, without resulting in invalid states or crashes.
2641+
func Test_diff_inline_multibuffer_empty_block()
2642+
CheckScreendump
2643+
2644+
call writefile(['anchor1', '1234567890abcde', 'anchor2'], 'Xdifile1')
2645+
call writefile(['anchor1', '1234567--0abc-e', 'anchor2'], 'Xdifile2')
2646+
call writefile(['anchor1', 'anchor2'], 'Xdifile3')
2647+
call writefile(['anchor1', '1???????90abcd?', 'anchor2'], 'Xdifile4')
2648+
2649+
let buf = RunVimInTerminal('-d Xdifile1 Xdifile2 Xdifile3 Xdifile4', {})
2650+
call VerifyInternal(buf, "Test_diff_inline_multibuffer_empty_block_01", " diffopt+=inline:char")
2651+
2652+
call StopVimInTerminal(buf)
2653+
endfunc
2654+
26392655
func Test_diffget_diffput_linematch()
26402656
CheckScreendump
26412657
call delete('.Xdifile1.swp')

‎src/version.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,8 @@ static char *(features[]) =
719719

720720
static int included_patches[] =
721721
{ /* Add new patch number below this line */
722+
/**/
723+
1567,
722724
/**/
723725
1566,
724726
/**/

0 commit comments

Comments
 (0)