On 1/10/19 6:13 PM, Martijn van Duren wrote:
> On 1/10/19 1:00 PM, Otto Moerbeek wrote:
>> On Thu, Jan 10, 2019 at 02:18:50AM -0700, Anthony J. Bentley wrote:
>>
>>> Hi,
>>>
>>> I have a vi test case that reliably crashes.
>>>
>>> It consists of two files. To crash in a 80x24 terminal, the files should
>>> be as follows:
>>>
>>> foo.txt should be 27 lines long. The fifth line should consist of 81
>>> characters, and the others 80 or below.
>>>
>>> bar.txt should be 5 lines or more, each line of less than 80 characters.
>>>
>>> Example foo.txt:
>>> a
>>> b
>>> c
>>> d
>>> 123456789012345678901234567890123456789012345678901234567890123456789012345678901
>>> f
>>> g
>>> h
>>> i
>>> j
>>> k
>>> l
>>> m
>>> n
>>> o
>>> p
>>> q
>>> r
>>> s
>>> t
>>> u
>>> v
>>> w
>>> x
>>> y
>>> z
>>> A
>>>
>>> Example bar.txt:
>>> 1
>>> 2
>>> 3
>>> 4
>>> 5
>>> 6
>>>
>>> Start with "vi foo.txt bar.txt".
>>> In foo, hit ^F once, then :n to switch files.
>>> In bar, line 6 is not visible (bug). Hitting ^F here will crash vi.
>>>
>>> This might be related to the fact that after the first ^F, foo's long
>>> line is split, with only the second half shown on screen. The crash
>>> occurs when trying to return the logical column of the cursor:
>>>
>>> #0 0x00000733ac5cbda6 in vs_column (sp=0x73623e317b0, colp=0x73623e31850)
>>> at /usr/src/usr.bin/vi/build/../vi/vs_relative.c:40
>>> #1 0x00000733ac5c5c15 in vi (spp=0x7f7ffffbfdc0)
>>> at /usr/src/usr.bin/vi/build/../vi/vi.c:110
>>> #2 0x00000733ac5ae0b4 in editor (gp=0x7363966e5f0, argc=<optimized out>,
>>> argv=<optimized out>) at /usr/src/usr.bin/vi/build/../common/main.c:427
>>> #3 0x00000733ac58e6ce in main (argc=3, argv=0x7f7ffffc0008)
>>> at /usr/src/usr.bin/vi/build/../cl/cl_main.c:93
>>>
>>> --
>>> Anthony J. Bentley
>>>
>>
>> If you enable DEBUG, the abort in vs_refresh.c:695 gets triggered.
>> Note that not only line 6 is not drawn, also the ~ lines are missing.
>> Not hitting ^F but walking to the end or going to the last line with G
>> in bar works...
>>
>> No further clue yet.
>>
>> -Otto
>>
> I don't have a solution just yet, but I found the culprit.
>
> When doing the ^F it places the top of the top of the screen at the
> overflow of the long line. This position causes HMAP->soff to be set to
> 2, which isn't reset when loading the new file and causes the soff on
> all the lines to increment by 1 compared to the previous line, so line
> 2 is placed at 2 + 3 = 5, line 3 at 3 + 4 = 7, ...
> This isn't a full explanation yet, since I haven't figured out yet why
> line 1-5 are printed correctly and I also don't know why scrolling down
> causes the file to be reset correctly or even exactly what happens when
> we ^F on the incorrectly printed file, but it's a start.
>
> The following (not to be committed) diff proves that the soff is indeed
> the culprit, but the math is still to obscure for me to say where the
> fix really should be, which could be somewhere entirely different.
>
> martijn@
>
So this is how far I'm willing to go for today:
When entering vi/vs_refresh.c:326 HMAP->lno says that there's 5 lines,
which get returned. I haven't been able to trace where this value is
set so far, but it's obviously related to how the HMAP->soff is set to
2.
Considering the different ways vi may switch files I found that that
vi only test for this on vi/vi.c:396. So the diff below resets the
HMAP state there. Only HMAP should be reset, because the other SMAP
fields are derived from that one (see vs_sm_{next,prev}. This is
also backed by vi/vi.c:965, which only initializes HMAP, and not all
the other maps.
I also tested the diff with a small number of args and opening
files in split screen and I didn't notice any issues.
I'm somewhat confident that this is the correct solution, but I'm
not 100% certain, considering Rapunzel probably has an easier time
untangling her hair.
thoughts?
martijn@
Index: vi/vi.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/vi/vi.c,v
retrieving revision 1.21
diff -u -p -r1.21 vi.c
--- vi/vi.c 18 Apr 2017 01:45:35 -0000 1.21
+++ vi/vi.c 11 Jan 2019 07:18:39 -0000
@@ -396,6 +396,10 @@ intr: CLR_INTERRUPT(sp);
if (F_ISSET(sp, SC_FSWITCH)) {
F_CLR(sp, SC_FSWITCH);
(void)sp->gp->scr_rename(sp, sp->frp->name, 1);
+ /* If we switch files, we must draw a fresh screen. */
+ HMAP->lno = sp->lno;
+ HMAP->coff = 0;
+ HMAP->soff = 1;
}
/* If leaving vi, return to the main editor loop. */
