Tony Mechelynck wrote:
> On 17/12/08 13:34, Bram Moolenaar wrote:
> >
> > Dominique Pelle wrote:
> >
> >> 2008/12/16 Bram Moolenaar<b...@moolenaar.net>:
> >>>
> >>> Matt Wozniski wrote:
> >>>
> >>>> function! ReturnArgs(...)
> >>>> return a:000
> >>>> endfunction
> >>>>
> >>>> " Seems to work fine?
> >>>> echo ReturnArgs(1, 2, 3)
> >>>>
> >>>> " SEGV
> >>>> echo string(ReturnArgs(1, 2, 3))
> >>>>
> >>>> function! MakeArgsDict(...)
> >>>> return { 'args': a:000 }
> >>>> endfunction
> >>>>
> >>>> " E685 Internal Error
> >>>> echo MakeArgsDict(1, 2, 3)
> >>>>
> >>>> " SEGV
> >>>> echo string(MakeArgsDict(1, 2, 3))
> >>> For it crashes a while after trying these things. Most likely the
> >>> reference count for a:000 is wrong. Never thought of someone returning
> >>> it...
> >> When I debugged, I found that v_list was pointing to an invalid address,
> >> which had been set in call_user_func() to&fc.l_varlist; This variable is
> >> in the stack and was only valid while in call_user_func() and the
> >> functions it
> >> may calls. Somehow, a list still refers to this address after returning
> >> from
> >> call_user_func() so v_list points then to an invalid address.
> >>
> >> Making variable fc static (in function call_user_func()) avoids using an
> >> invalid address and thus avoids a crash, but it's still not the right way
> >> to fix it.
> >>
> >>> " Seems to work fine?
> >>> echo ReturnArgs(1, 2, 3)
> >> Actually, even though this appears to work, valgrind memory checker
> >> already sees a problem there:
> >>
> >> ==23275== Invalid read of size 4
> >> ==23275== at 0x809C577: echo_string (eval.c:7232)
> >> ==23275== by 0x80AD48C: ex_echo (eval.c:19481)
> >> ==23275== by 0x80C71C6: do_one_cmd (ex_docmd.c:2622)
> >> ==23275== by 0x80C4A46: do_cmdline (ex_docmd.c:1096)
> >> ==23275== by 0x8149D7A: nv_colon (normal.c:5233)
> >> ==23275== by 0x81433FE: normal_cmd (normal.c:1200)
> >> ==23275== by 0x810678D: main_loop (main.c:1180)
> >> ==23275== by 0x81062DA: main (main.c:939)
> >> ==23275== Address 0xbef5e280 is not stack'd, malloc'd or (recently) free'd
> >> ==23275==
> >> ==23275== Invalid write of size 4
> >> ==23275== at 0x809C59D: echo_string (eval.c:7239)
> >> ==23275== by 0x80AD48C: ex_echo (eval.c:19481)
> >> ==23275== by 0x80C71C6: do_one_cmd (ex_docmd.c:2622)
> >> ==23275== by 0x80C4A46: do_cmdline (ex_docmd.c:1096)
> >> ==23275== by 0x8149D7A: nv_colon (normal.c:5233)
> >> ==23275== by 0x81433FE: normal_cmd (normal.c:1200)
> >> ==23275== by 0x810678D: main_loop (main.c:1180)
> >> ==23275== by 0x81062DA: main (main.c:939)
> >> ==23275== Address 0xbef5e280 is not stack'd, malloc'd or (recently) free'd
> >>
> >> (etc, more errors to follow)
> >>
> >> Line eval.c:7232 is:
> >>
> >> 7232 else if (copyID != 0&& tv->vval.v_list->lv_copyID ==
> >> copyID)
> >> 7233 {
> >> 7234 *tofree = NULL;
> >> 7235 r = (char_u *)"[...]";
> >> 7236 }
> >>
> >> 'tv->vval.v_list' points to the invalid address which set as there
> >> at line 21193:
> >>
> >> 21191 v->di_tv.v_type = VAR_LIST;
> >> 21192 v->di_tv.v_lock = VAR_FIXED;
> >> 21193 v->di_tv.vval.v_list =&fc.l_varlist;
> >>
> >> (fc being a local var in the stack)
> >
> > What is happening here is that a few things are put on the stack to
> > avoid malloc()/free() calls. These are quite expensive and adds
> > overhead to every function call.
> >
> > The reference count of these are not used. When the function returns,
> > the items automatically disappear. That's a bit of a problem if you
> > return the value or assigned it to a global variable.
> >
> > I think the only proper solution is to do that malloc()/free().
> > Not only for a:000, but also for l: and a:. And all elements
> > contained in them, that's going to be time consuming.
> >
> > Another method would be to disallow passing these dictionaries to
> > outside the function scope. One would have to make a copy instead.
> > Checking for this may be complicated though. And this also doesn't
> > solve the problem for variables such as a:firstline that are also on the
> > stack.
> >
> > It looks like the best solution is to put the whole funccall_T in
> > allocated memory. And only free it when all the reference counts are
> > back to zero. Need to make a list of them and add some code to the
> > garbage collector.
>
> When returning a Number (e.g. ":return a:lastline") or a Float, I
> suppose the return method guarantees that there'll be no problem. I'm
> less certain about returning a String variable local to the function,
> but I suppose existing code already provides for that. A Funcref to a
> function defined inside the function should remain valid -- or does it?
> But what we're talking about here is when the returned object is a List
> or Dictionary containing (at any level of depth) something local to the
> function, which would disappear at return-time. (Returning a pointer to
> a _global_ List or Dictionary ought to be no problem if no local
> elements or sub-elements have been added by the function.)
This only matters for a:000, l: and a:. These are allocated on the
stack. All other things are in allocated memory and can be passed
around without problems.
> Proposed solution: returning a List or Dictionary local to the function
> is forbidden (and generates an error). But what about returning a List
> or Dictionary containing _elements_ local to the function? Maybe
> nondestructively check (recursively, but breaking out on first error).
>
> The user can of course catch the error by wrapping the return statement
> in a try block (if he wants to), triggering a deepcopy() to a global
> variable if caught.
>
> Alternative solution: returning a List or Dictionary actually returns a
> global deepcopy(). But this would impair performance, and it might have
> other unobvious consequences.
We could do something special for the return statement. But assigning
the things to a global variable is the most difficult issue. Checking
each assignment is a bit of a problem. I can't be 100% sure that we
will find all places in the code that need to have this "function local
thing that needs to be copied" check. And it will make all assignments
a bit slower.
Allocating the whole funccall_T once and freeing it later is not that
much overhead. It's the number of allocations that matter, not the
size. The only tricky thing is checking that there is a reference to
the items inside. That's something not too difficult to figure out and
easy to check if it works.
--
VOICE OVER: As the horrendous Black Beast lunged forward, escape for Arthur
and his knights seemed hopeless, when, suddenly ... the animator
suffered a fatal heart attack.
ANIMATOR: Aaaaagh!
VOICE OVER: The cartoon peril was no more ... The Quest for Holy Grail could
continue.
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD
/// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ download, build and distribute -- http://www.A-A-P.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
