Skip to content

Commit 3bda0a4

Browse files
Mike PallBuristan
Mike Pall
authored andcommitted
x86/x64: Don't fuse loads across table.clear.
Reported by Peter Cawley. (cherry picked from commit 45c88b7) Load fusing optimization takes into account only the presence of the corresponding stores, but not any calls that may affect the table content. This may lead to the incorrect stores if the fusing optimization occurs across the `table.clear()` call, leading to inconsistent behaviour between the JIT and the VM. This patch adds the corresponding check. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#10709 Reviewed-by: Sergey Bronnikov <[email protected]> Signed-off-by: Sergey Kaplun <[email protected]> (cherry picked from commit e6b59ca)
1 parent 6a2a933 commit 3bda0a4

File tree

2 files changed

+37
-0
lines changed

2 files changed

+37
-0
lines changed

src/lj_asm_x86.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,7 @@ static Reg asm_fuseload(ASMState *as, IRRef ref, RegSet allow)
465465
}
466466
} else if (ir->o == IR_ALOAD || ir->o == IR_HLOAD || ir->o == IR_ULOAD) {
467467
if (noconflict(as, ref, ir->o + IRDELTA_L2S, 0) &&
468+
noconflict(as, ref, IR_CALLS, 0) && /* Don't cross table.clear. */
468469
!(LJ_GC64 && irt_isaddr(ir->t))) {
469470
asm_fuseahuref(as, ir->op1, xallow);
470471
return RID_MRM;
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
local tap = require('tap')
2+
-- Test file to demonstrate LuaJIT's incorrect fusion across
3+
-- `table.clear()`.
4+
-- See also: https://github.com/LuaJIT/LuaJIT/issues/1117.
5+
local test = tap.test('lj-1117-fuse-across-table-clear'):skipcond({
6+
['Test requires JIT enabled'] = not jit.status(),
7+
})
8+
9+
local ffi = require('ffi')
10+
local table_clear = require('table.clear')
11+
12+
test:plan(1)
13+
14+
local tab = {0}
15+
local alias_tab = tab
16+
local result_tab = {}
17+
18+
jit.opt.start('hotloop=1')
19+
20+
for i = 1, 4 do
21+
-- Load to be fused.
22+
local value = tab[1]
23+
-- Clear the alias table to trick the code flow analysis.
24+
table_clear(alias_tab)
25+
-- Need this cast to trigger load fusion. See `asm_comp()` for
26+
-- the details. Before the patch, this fusion takes the
27+
-- incorrect address of the already cleared table, which leads
28+
-- to the failure of the check below.
29+
result_tab[i] = ffi.cast('int64_t', value)
30+
-- Revive the value.
31+
tab[1] = 0
32+
end
33+
34+
test:samevalues(result_tab, 'no fusion across table.clear')
35+
36+
test:done(true)

0 commit comments

Comments
 (0)