Jason Franklin wrote:
> My environment is Ubuntu 18.04, GNOME Terminal.
>
> While tinkering around, I discovered that the following snippet of
> Vimscript will result in a segfault:
>
> " This produces a segfault.
> while 1
> silent! let v:errmsg = []
> let v:errmsg = ''
> endwhile
>
> The loop is there because you may have to execute the sequence
> a couple of times before a segfault happens.
>
> Also notice that running:
>
> :let v:errmsg = []
> :echo v:errmsg
>
> does not show the last generated error message as one would expect,
> it shows the empty string.
>
> I wrote a test to prove that my patch sets the error message in
> this case:
>
> func Test_let_errmsg()
> silent! let v:errmsg = []
> call assert_false(empty(v:errmsg))
> endfunc
>
> I'm not exactly sure where to put the test... there were several
> test scripts that seemed relevant here.
>
> I dug into the code and came up with this:
>
> diff --git a/src/eval.c b/src/eval.c
> index d1a7fd37d..c485ecabd 100644
> --- a/src/eval.c
> +++ b/src/eval.c
> @@ -7883,20 +7883,23 @@ set_var(
> || tv_check_lock(v->di_tv.v_lock, name, FALSE))
> return;
>
> - /*
> - * Handle setting internal v: variables separately where needed to
> - * prevent changing the type.
> - */
> + // Handle setting internal v: variables separately where needed to
> + // prevent changing the type.
> if (ht == &vimvarht)
> {
> if (v->di_tv.v_type == VAR_STRING)
> {
> - vim_free(v->di_tv.vval.v_string);
> + VIM_CLEAR(v->di_tv.vval.v_string);
> if (copy || tv->v_type != VAR_STRING)
> - v->di_tv.vval.v_string = vim_strsave(tv_get_string(tv));
> + {
> + if (STRCMP("v:errmsg", name) == 0)
> + tv_get_string(tv); // calls emsg(), which sets v:errmsg
> + else
> + v->di_tv.vval.v_string = vim_strsave(tv_get_string(tv));
> + }
> else
> {
> - /* Take over the string to avoid an extra alloc/free. */
> + // Take over the string to avoid an extra alloc/free.
> v->di_tv.vval.v_string = tv->vval.v_string;
> tv->vval.v_string = NULL;
> }
>
> This patch stops the segfault from happening and lets the check of the "tv"
> variable set v:errmsg (see comment).
>
> I would appreciate some careful review here. I don't precisely understand
> why VIM_CLEAR works instead of vim_free(). I suspect it is due to some check
> for NULL later. Any feedback to help my understanding is always appreciated.
> It may be that a much better solution exists, but I'm not really familiar
> enough with the code to arrive at the perfect fix... though I try.
Good catch! I don't have time to debug right now, but I suspect
tv_get_string() produces an error message and when v_string was not set
to NULL, it gets freed twice.
Solution would be to call vim_strsave() and storing the result in a
local variable, instead of assigning it to v_string. And probably use
tv_get_string_chk() to skip storing the value when there is an error.
--
There is a fine line between courage and foolishness.
Unfortunately, it's not a fence.
/// Bram Moolenaar -- [email protected] -- 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 ///
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
