openresty / luajit2

OpenResty's Branch of LuaJIT 2
https://luajit.org/luajit.html
Other
1.2k stars 193 forks source link

Invalid HSTORE elimination over nkeys and clone #147

Closed XmiliaH closed 2 years ago

XmiliaH commented 2 years ago

It is possible that a HSTORE before a call to table.nkeys or table.clone is eliminated by a HSTORE after said functions resulting in wrong results.

This is demonstrated by the following example which should print 0 nil but the actual output is 1 99999.

local clone = require"table.clone"
local nkeys = require"table.nkeys"
local c, n
local t = {idx = 0}

-- jit.off()

for i = 1, 100000 do
    t.idx = nil
    n = nkeys(t)
    c = clone(t)
    t.idx = i
end

print(n, c.idx)
agentzh commented 2 years ago

@XmiliaH Thanks for the report! We'll look into this. It seems like the optimizer needs to be aware of the nkeys IR.

XmiliaH commented 2 years ago

I think the problem is in lj_opt_dse_ahstore and can be fixed with:

@@ -369,7 +369,9 @@ TRef LJ_FASTCALL lj_opt_dse_ahstore(jit_State *J)
        ** since they are followed by at least one guarded VLOAD.
        */
        for (ir = IR(J->cur.nins-1); ir > store; ir--)
-         if (irt_isguard(ir->t) || ir->o == IR_ALEN)
+         if (irt_isguard(ir->t) || ir->o == IR_ALEN || 
+        (ir->o == IR_CALLL && ir->op2 == IRCALL_lj_tab_nkeys) || 
+        (ir->o == IR_CALLS && ir->op2 == IRCALL_lj_tab_clone))
            goto doemit;  /* No elimination possible. */
        /* Remove redundant store from chain and replace with NOP. */
        *refp = store->prev;
zhuizhuhaomeng commented 2 years ago

@XmiliaH would you please create a PR for this issue?