lua-users home
lua-l archive

[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index] [Thread Index]

>  Andrew> This code thinks it can create an anonymous group capture with
>  Andrew> no valid .s pointer (to contain the results of a match-time
>  Andrew> capture). I _think_ the failure is normally hidden by the fact
>  Andrew> that base[0] is _usually_ a reuse of the same Capture that was
>  Andrew> used for the match-time capture itself - except that the array
>  Andrew> of Capture structures might get reallocated between the two
>  Andrew> uses, without preserving the old value of the (now unused)
>  Andrew> Capture.
> Further analysis shows that my original idea was wrong; correct
> substitution _requires_ that the value of base[0].s actually be
> preserved rather than initialized in adddyncaptures. So the fix seems to
> be to ensure that it is preserved across expansion of the captures
> array:
> --- lpvm.c.orig 2018-11-10 09:37:29 UTC
> +++ lpvm.c
> @@ -311,7 +311,7 @@ const char *match (lua_State *L, const c
>            if (fr + n >= SHRT_MAX)
>              luaL_error(L, "too many results in match-time capture");
>            if ((captop += n + 2) >= capsize) {
> -            capture = doublecap(L, capture, captop, n + 2, ptop);
> +            capture = doublecap(L, capture, captop, n + 1, ptop);
>              capsize = 2 * captop;
>            }
>            /* add new captures to 'capture' list */
> By telling doublecap that one less Capture structure is "new", we make
> it preserve one more of the existing values, which is the one we need.
> This patch fixes both my testcase and the original report, as far as I
> can tell.

Many thanks for the detailed analysis.

Although your analysis and the fix seem logically correct, I think it
would be cleaner to preserve the initial open-group capture in the
whole process.  As it is, we are removing a capture and creating a new
one, and kind of "by chance" the original value of base[0].s is being
preserved. The same way you got it wrong in your first message, we
will get it wrong again in the future.

-- Roberto