dibyendumajumdar/ravi

Wrong Type Deduction for Values from Upvalue Tables

XmiliaH opened this issue · 5 comments

If an integer or number array is accessed through an upvalue, the type of the result will be the same as the array, since the handling case in luaK_dischargevars does not set ravi_type as the other case does. The upvalue case it the following:

ravi/src/lcode.c

Lines 623 to 627 in 170fd79

lua_assert(e->u.ind.vt == VUPVAL);
if (e->u.ind.key_ravi_type == RAVI_TSTRING && isshortstr(fs, e->u.ind.idx))
op = OP_RAVI_GETTABUP_SK;
else
op = OP_GETTABUP; /* 't' is in an upvalue */

The following lua code will show the issue and crash with ravi/src/lvm.c:2278: luaV_execute: Assertion `(((((rb))->tt_) == (((((5) | ((1) << 4))) | (1 << 15)))) || ((((rb))->tt_) == (((((5) | ((2) << 4))) | (1 << 15)))))' failed.

local function f(x:integer[])
    return function ()
        return x[1][1] + 1
    end
end

local x = f(table.intarray(3))

x()

The bytecode for the inner function contains the unexpected IARRAY_GET, since this object is an integer and not an array.

1       [3]     GETTABUP        0 0 -1  ; x 1
2       [3]     IARRAY_GET      0 0 -1  ; 1
3       [3]     ADDII           0 0 -1  ; - 1
4       [3]     RETURN          0 2
5       [4]     RETURN          0 1

Thank you for the report

Unfortunately the best we can do here is set ANY type as we don't have more specialized opcodes for upvalues.

I don't see a problem with moving

ravi/src/lcode.c

Lines 616 to 620 in b0a5b01

if (e->ravi_type == RAVI_TARRAYFLT || e->ravi_type == RAVI_TARRAYINT)
/* set the type of resulting expression */
e->ravi_type = e->ravi_type == RAVI_TARRAYFLT ? RAVI_TNUMFLT : RAVI_TNUMINT;
else
e->ravi_type = RAVI_TANY;

out of the if to
e->u.info = luaK_codeABC(fs, op, 0, e->u.ind.t, e->u.ind.idx);

But I don' know your code better then you.

Stepping in the debugger I see that e has the type of the variable upvalue points to - so you are right that we could infer that the result will be integer in this case. I will need to check this a bit as I wrote this code a while back and I need to refresh my memory.

btw appreciate these reports very much, thank you for taking the time.