Bram Moolenaar wrote:
> Yukihiro Nakadaira wrote:
>
>> Kana Natsuno wrote:
>> > On Mon, 12 Oct 2009 05:03:27 +0900, Bram Moolenaar <[email protected]>
>> > wrote:
>> >> Can you somehow simplify the way to reproduce this and send me the Vim
>> >> script for this?
>> >
>> > Sorry, I tried to simplify before reporting but failed. Because the
>> > problem I encountered seems to depend on the number/order of operations,
>> > and the problem doesn't occur even if I delete a key/value pair
>> > expression from dictionary literal which seems not to be related to
>> > executing scripts.
>> >
>> > Anyway, I'll try to simplify again.
>>
>> Perhaps your problem is ...
>>
>> In eval.c, all "s:" dictionary is stored in one array (ga_scripts).
>> When new script file is sourced, the array is re-allocated for new "s:"
>> dictionary. Then "s:" dictionary, previously assigned to other variable
>> (e.g. :let g:foo = s:), will become an invalid pointer.
>>
>>
>> Script to reproduce: (might not crash 100%)
>>
>> let script_vars = []
>> for src in range(10)
>> call writefile(['call add(script_vars, s:)'], src)
>> source `=src`
>> " allocate memory to ensure crash.
>> call repeat(' ', 1000)
>> endfor
>> echo script_vars
>
> Very good point. Thanks for finding this problem. I'll think about a
> solution. Unless someone beats me to it.
Hi
Crash in eval.c described above was reported a long time ago.
Attached patch fixes it by making ga_scripts in eval.c a growing
array of pointers "scriptvar_T *" rather than growing array of
structs "scriptvar_T" so that when ga_scripts is reallocated,
structures don't move in memory.
I verified that the sample script given by Yukihiro Nakadaira which
always caused a crash (and errors with Valgrind) no longer crashes
after patch (and Valgrind no complain either).
Regards
-- Dominique
--
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
diff -r 24100651daa9 runtime/doc/todo.txt
--- a/runtime/doc/todo.txt Tue Mar 23 18:22:46 2010 +0100
+++ b/runtime/doc/todo.txt Fri Apr 30 21:59:02 2010 +0200
@@ -62,9 +62,6 @@
Patch to support netbeans in Unix console Vim. (Xavier de Gaye, 2009 Apr 26)
Now with Mercurial repository (2010 Jan 2)
-Crash when assigning s: to variable, pointer becomes invalid later.
-(Yukihiro Nakadaira, 2009 Oct 12, confirmed by Dominique Pelle)
-
Test 69 breaks on MS-Windows, both 32 and 64 builds. (George Reilly, 2010 Feb
26)
@@ -175,7 +172,7 @@
because win_valid() always returns FALSE. Below win_new_tabpage() in
ex_docmd.c.
-Space before comma in function defenition not allowed: "function x(a , b)"
+Space before comma in function definition not allowed: "function x(a , b)"
Give a more appropriate error message. Add a remark to the docs.
string_convert() should be able to convert between utf-8 and utf-16le. Used
diff -r 24100651daa9 src/eval.c
--- a/src/eval.c Tue Mar 23 18:22:46 2010 +0100
+++ b/src/eval.c Fri Apr 30 21:59:02 2010 +0200
@@ -145,9 +145,9 @@
dict_T sv_dict;
} scriptvar_T;
-static garray_T ga_scripts = {0, 0, sizeof(scriptvar_T), 4, NULL};
-#define SCRIPT_SV(id) (((scriptvar_T *)ga_scripts.ga_data)[(id) - 1])
-#define SCRIPT_VARS(id) (SCRIPT_SV(id).sv_dict.dv_hashtab)
+static garray_T ga_scripts = {0, 0, sizeof(scriptvar_T *), 4, NULL};
+#define SCRIPT_SV(id) (((scriptvar_T **)ga_scripts.ga_data)[(id) - 1])
+#define SCRIPT_VARS(id) (SCRIPT_SV(id)->sv_dict.dv_hashtab)
static int echo_attr = 0; /* attributes used for ":echo" */
@@ -866,17 +866,21 @@
hash_init(&vimvarht); /* garbage_collect() will access it */
hash_clear(&compat_hashtab);
+ free_scriptnames();
+
+ /* global variables */
+ vars_clear(&globvarht);
+
+ /* autoloaded script names */
+ ga_clear_strings(&ga_loaded);
+
/* script-local variables */
for (i = 1; i <= ga_scripts.ga_len; ++i)
+ {
vars_clear(&SCRIPT_VARS(i));
+ vim_free(SCRIPT_SV(i));
+ }
ga_clear(&ga_scripts);
- free_scriptnames();
-
- /* global variables */
- vars_clear(&globvarht);
-
- /* autoloaded script names */
- ga_clear_strings(&ga_loaded);
/* unreferenced lists and dicts */
(void)garbage_collect();
@@ -18803,7 +18807,7 @@
/* Must be something like "s:", otherwise "ht" would be NULL. */
switch (varname[-2])
{
- case 's': return &SCRIPT_SV(current_SID).sv_var;
+ case 's': return &SCRIPT_SV(current_SID)->sv_var;
case 'g': return &globvars_var;
case 'v': return &vimvars_var;
case 'b': return &curbuf->b_bufvar;
@@ -18928,13 +18932,14 @@
ht = &SCRIPT_VARS(i);
if (ht->ht_mask == HT_INIT_SIZE - 1)
ht->ht_array = ht->ht_smallarray;
- sv = &SCRIPT_SV(i);
+ sv = SCRIPT_SV(i);
sv->sv_var.di_tv.vval.v_dict = &sv->sv_dict;
}
while (ga_scripts.ga_len < id)
{
- sv = &SCRIPT_SV(ga_scripts.ga_len + 1);
+ sv = SCRIPT_SV(ga_scripts.ga_len + 1) =
+ (scriptvar_T *)alloc_clear(sizeof(scriptvar_T));
init_var_dict(&sv->sv_dict, &sv->sv_var);
++ga_scripts.ga_len;
}
@@ -21931,7 +21936,7 @@
if (find_viminfo_parameter('!') == NULL)
return;
- fprintf(fp, _("\n# global variables:\n"));
+ fputs(_("\n# global variables:\n"), fp);
todo = (int)globvarht.ht_used;
for (hi = globvarht.ht_array; todo > 0; ++hi)
