Re: Lua 5.3w2 Test Suite Issues

[Paige DePol <lual@...>]

On Mar 31, 2014, at 4:55 PM, Doug Currie <doug.currie@gmail.com> wrote:

> On Sun, Mar 30, 2014 at 9:46 PM, Paige DePol <lual@serfnet.org> wrote:
> Another way of fixing the issue ...
> 
> Reminder: this issue is present in 5.2 as well.
> 
> A portable solution for fixing unpack (without changing lua_checkstack) using unsigned arithmetic:
> 
> static int unpack (lua_State *L) {
>   int i, e;
>   unsigned int n;
>   luaL_checktype(L, 1, LUA_TTABLE);
>   i = luaL_optint(L, 2, 1);
>   e = luaL_opt(L, luaL_checkint, 3, luaL_len(L, 1));
>   if (i > e) return 0;  /* empty range */
>   n = (unsigned int )e - (unsigned int )i;  /* number of elements minus one */
>   if (n >= INT_MAX || !lua_checkstack(L, (int )(n+1)))  /* detect arith. overflow */
>     return luaL_error(L, "too many results to unpack");
>   lua_rawgeti(L, 1, i);  /* push arg[i] (avoiding overflow problems) */
>   while (i++ < e)  /* push arg[i + 1...e] */
>     lua_rawgeti(L, 1, i);
>   return n;
> }
> 
> The variable n is changed to unsigned, and the calculation of the number of elements uses unsigned arithmetic. This works because unsigned arithmetic cannot overflow; we know the sign of the result is positive since i <= e. Note that we defer the addition of 1 to n until after the test against INT_MAX to avoid a carry in the case of unpack ({}, -2^31, 2^31-1) leading to a result of zero.
> 
> e

Yes, this is also a valid solution, however, I favour adding the sanity check to lua_checkstack() as it potentially helps prevent future issues should a negative value be passed for 'size'. My fix simply adds one line to lua_checkstack() and changes one comparison in unpack() to fix the issue as well.

~Paige