Patch 8.2.1206

[Bram Moolenaar]

Patch 8.2.1206
Problem: Vim9: crash in expr test when run in the GUI.
Solution: Temporarily comment out two test lines.
Files: src/testdir/test_vim9_expr.vim


*** ../vim-8.2.1205/src/testdir/test_vim9_expr.vim 2020-07-13 22:28:41.290857367 +0200
--- src/testdir/test_vim9_expr.vim 2020-07-13 23:21:41.200681211 +0200
***************
*** 1563,1570 ****
enddef

func Test_expr7_trailing_fails()
! call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)}'], 'E107')
! call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)} ()'], 'E274')
endfunc

func Test_expr_fails()
--- 1563,1572 ----
enddef

func Test_expr7_trailing_fails()
! " Temporarily commented out - somehow crash occurs with too many
! " CheckDefFailure calls in the GUI only.
! " call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)}'], 'E107')
! " call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)} ()'], 'E274')
endfunc

func Test_expr_fails()
*** ../vim-8.2.1205/src/version.c 2020-07-13 22:28:41.290857367 +0200
--- src/version.c 2020-07-13 23:22:34.232561256 +0200
***************
*** 756,757 ****
--- 756,759 ----
{ /* Add new patch number below this line */
+ /**/
+ 1206,
/**/

--
Don't Panic!
-- The Hitchhiker's Guide to the Galaxy

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Bram Moolenaar]

I wrote:

> Patch 8.2.1206
> Problem: Vim9: crash in expr test when run in the GUI.
> Solution: Temporarily comment out two test lines.
> Files: src/testdir/test_vim9_expr.vim
>
>
> *** ../vim-8.2.1205/src/testdir/test_vim9_expr.vim 2020-07-13 22:28:41.290857367 +0200
> --- src/testdir/test_vim9_expr.vim 2020-07-13 23:21:41.200681211 +0200
> ***************
> *** 1563,1570 ****
> enddef
>
> func Test_expr7_trailing_fails()
> ! call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)}'], 'E107')
> ! call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)} ()'], 'E274')
> endfunc
>
> func Test_expr_fails()
> --- 1563,1572 ----
> enddef
>
> func Test_expr7_trailing_fails()
> ! " Temporarily commented out - somehow crash occurs with too many
> ! " CheckDefFailure calls in the GUI only.
> ! " call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)}'], 'E107')
> ! " call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)} ()'], 'E274')
> endfunc
>
> func Test_expr_fails()

This is a hack to avoid a weird test failure. It appears to only happen
in the GUI. I have managed to repro locally, it causes a SEGV to
happen. But the moment I use ASAN or Valgrind the problem goes away.
It's also not related to specific lines in the test, it looks like it
depends on the number of calls. Perhaps the number of lambda functions.

I did manage to see the crash locaiton using a core file, but it looks
like the actual problem happened earlier and this is just the first
place where corrupted data gets used. It's in ex_defcompile().

I hope someone can reproduce and use some tool to figure out the root
cause.

--
Far back in the mists of ancient time, in the great and glorious days of the
former Galactic Empire, life was wild, rich and largely tax free.
Mighty starships plied their way between exotic suns, seeking adventure and
reward among the furthest reaches of Galactic space. In those days, spirits
were brave, the stakes were high, men were real men, women were real women
and small furry creatures from Alpha Centauri were real small furry creatures
from Alpha Centauri. And all dared to brave unknown terrors, to do mighty
deeds, to boldly split infinitives that no man had split before -- and thus
was the Empire forged.
-- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"

[Bram Moolenaar]

I wrote:

> > Patch 8.2.1206
> > Problem: Vim9: crash in expr test when run in the GUI.
> > Solution: Temporarily comment out two test lines.
> > Files: src/testdir/test_vim9_expr.vim
> >
> >
> > *** ../vim-8.2.1205/src/testdir/test_vim9_expr.vim 2020-07-13 22:28:41.290857367 +0200
> > --- src/testdir/test_vim9_expr.vim 2020-07-13 23:21:41.200681211 +0200
> > ***************
> > *** 1563,1570 ****
> > enddef
> >
> > func Test_expr7_trailing_fails()
> > ! call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)}'], 'E107')
> > ! call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)} ()'], 'E274')
> > endfunc
> >
> > func Test_expr_fails()
> > --- 1563,1572 ----
> > enddef
> >
> > func Test_expr7_trailing_fails()
> > ! " Temporarily commented out - somehow crash occurs with too many
> > ! " CheckDefFailure calls in the GUI only.
> > ! " call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)}'], 'E107')
> > ! " call CheckDefFailure(['let l = [2]', 'l->{l -> add(l, 8)} ()'], 'E274')
> > endfunc
> >
> > func Test_expr_fails()
>

> This is a hack to avoid a weird test failure. It appears to only happen
> in the GUI. I have managed to repro locally, it causes a SEGV to
> happen. But the moment I use ASAN or Valgrind the problem goes away.
> It's also not related to specific lines in the test, it looks like it
> depends on the number of calls. Perhaps the number of lambda functions.
>
> I did manage to see the crash locaiton using a core file, but it looks
> like the actual problem happened earlier and this is just the first
> place where corrupted data gets used. It's in ex_defcompile().
>
> I hope someone can reproduce and use some tool to figure out the root
> cause.

I found it: Apparently one function is removed and one function is added
when invoking compile_def_function(). Then the "ht_used" count doesn't
change, even the hashtable array doesn't change. But the removed item
may be past the current position in the array, while the added item is
before it, resulting in running over the end of the array.

This won't trigger any Asan or Valgrind message, since all the
manipulations up to the crash are correct.

In patch 8.2.1207 I added a field "ht_changed" to know the hashtable
changed, even though none of the other entries changed. We should
probably use that in a few more places that loop over a hashtable.

--
For a moment, nothing happened.
Then, after a second or so, nothing continued to happen.
-- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"