Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong Type Deduction for Values from Upvalue Tables #211

Open
XmiliaH opened this issue Feb 17, 2021 · 5 comments
Open

Wrong Type Deduction for Values from Upvalue Tables #211

XmiliaH opened this issue Feb 17, 2021 · 5 comments

Comments

@XmiliaH
Copy link
Contributor

XmiliaH commented Feb 17, 2021

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
@dibyendumajumdar
Copy link
Owner

Thank you for the report

@dibyendumajumdar
Copy link
Owner

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

@XmiliaH
Copy link
Contributor Author

XmiliaH commented Feb 17, 2021

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.

@dibyendumajumdar
Copy link
Owner

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.

@dibyendumajumdar
Copy link
Owner

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

dibyendumajumdar added a commit that referenced this issue Feb 18, 2021
… we can safely infer that the result of a get on array is a primitive type
dibyendumajumdar added a commit that referenced this issue Feb 18, 2021
… we can safely infer that the result of a get on array is a primitive type
dibyendumajumdar added a commit that referenced this issue Feb 18, 2021
… we can safely infer that the result of a get on array is a primitive type
dibyendumajumdar added a commit that referenced this issue Feb 18, 2021
… we can safely infer that the result of a get on array is a primitive type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants