Bad lookup generated by lj_record_idx in GC64

[XmiliaH]

In the case where ix->oldv is the global nilnode since the key is not in the table the following check might actually succeed in GC64 since MSize is only 32 bit.

LuaJIT/src/lj_record.c

Lines 1451 to 1452 in 3ee3c9c

	MSize hslot = (MSize)((char *)ix->oldv - (char *)&noderef(t->node)[0].val);
	if (t->hmask > 0 && hslot <= t->hmask*(MSize)sizeof(Node) &&

This will result in res being TREF_NIL and rbref > TREF_NIL which will trigger the rollback removing the check for the nilnode.

LuaJIT/src/lj_record.c

Lines 1556 to 1565 in 3ee3c9c

	if (oldv == niltvg(J2G(J))) {
	emitir(IRTG(IR_EQ, IRT_PGC), xref, lj_ir_kkptr(J, niltvg(J2G(J))));
	res = TREF_NIL;
	} else {
	res = emitir(IRTG(loadop, t), xref, 0);
	}
	if (tref_ref(res) < rbref) { /* HREFK + load forwarded? */
	lj_ir_rollback(J, rbref); /* Rollback to eliminate hmask guard. */
	J->guardemit = rbguard;
	}

The following code demonstrates the issue:

local ffi = require"ffi"
local table_new = require"table.new"

ffi.cdef[[
    unsigned long long strtoull(const char *restrict str, char **restrict endptr, int base);
    typedef struct GCtab {
        uint64_t nextgc; uint8_t marked; uint8_t gct;
        uint8_t nomm;
        int8_t colo;
        uint64_t array;
        uint64_t gclist;
        uint64_t metatable;
        uint64_t node;
        uint32_t asize;
        uint32_t hmask;
        uint64_t freetop;
    } GCtab;
]]

local function get_node_pointer(obj)
    local str_ptr = string.format("%p", obj)
    local uint64_ptr = ffi.C.strtoull(str_ptr, nil, 0)
    return ffi.cast("GCtab*", uint64_ptr).node
end

local nil_node = get_node_pointer{}

local function test_tab(tab)
    local ptr = get_node_pointer(tab)
    local diff = tonumber(ffi.cast("uint32_t", nil_node - ptr))
    return diff < 65535 * 24
end

local tab = table_new(0, 65535)
local root

while not test_tab(tab) do
    tab.next = root
    root = tab
    tab = table_new(0, 65535)
end

print("Found tab")

local function get_n(tab)
    local x
    for i = 1, 100 do
        x = tab.n
    end
    return x
end

get_n(tab)

print(get_n({n=1}))

The ffi is used there to find a table for which the check assumes that nilnode is in the hash array.

The result of the print is nil but 1 is expected.

This can be fixed by using GCSize instead of MSize.

--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -1448,9 +1448,9 @@ static TRef rec_idx_key(jit_State *J, RecordIndex *ix, IRRef *rbref,
     key = emitir(IRTN(IR_CONV), key, IRCONV_NUM_INT);
   if (tref_isk(key)) {
     /* Optimize lookup of constant hash keys. */
-    MSize hslot = (MSize)((char *)ix->oldv - (char *)&noderef(t->node)[0].val);
-    if (t->hmask > 0 && hslot <= t->hmask*(MSize)sizeof(Node) &&
-       hslot <= 65535*(MSize)sizeof(Node)) {
+    GCSize hslot = (GCSize)((char *)ix->oldv - (char *)&noderef(t->node)[0].val);
+    if (hslot <= t->hmask*(GCSize)sizeof(Node) &&
+       hslot <= 65535*(GCSize)sizeof(Node)) {
       TRef node, kslot, hm;
       *rbref = J->cur.nins;  /* Mark possible rollback point. */
       *rbguard = J->guardemit;

Note: t->hmask > 0 is not required as it is checked for the empty hash part shortcut a view lines above.

[MikePall]

Fixed. Thanks!