Skip to content

Commit d86738f

Browse files
Mike PallBuristan
authored andcommitted
ARM64: Fix IR_SLOAD assembly.
Reported by Gate88. (cherry picked from commit 6c4826f) The issue is in the case when IR SLOAD is unused on a trace, it persists only for typecheck, and has the `num` type. In this case, the `dest` register is `RID_NONE`. Hence, the `fmov` instruction is emitted unconditionally, where the destination register is `d0` (`RID_NONE & 31`). So, the value of this register is spoiled. If it holds any value evaluated before and used after this SLOAD, it leads to incorrect behaviour. So the emitted assembly for the aforementioned SLOAD looks like the following: | ldr x27, [x7, LuaJIT#24] | cmp x0, x27, lsr LuaJIT#32 | bls 0x67d7f4f0 ->0 | fmov d0, x27 ; this spoils d0 Instead of the following: | ldr x27, [x7, LuaJIT#24] | cmp x0, x27, lsr LuaJIT#32 | bls 0x6a91f4e8 ->0 This patch adds the check that the register is in use before emitting the instruction. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11278 Reviewed-by: Sergey Bronnikov <[email protected]> Signed-off-by: Sergey Kaplun <[email protected]>
1 parent c8e49a2 commit d86738f

File tree

2 files changed

+46
-1
lines changed

2 files changed

+46
-1
lines changed

src/lj_asm_arm64.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1177,7 +1177,7 @@ static void asm_sload(ASMState *as, IRIns *ir)
11771177
tmp = ra_scratch(as, allow);
11781178
rset_clear(allow, tmp);
11791179
}
1180-
if (irt_isnum(t) && !(ir->op2 & IRSLOAD_CONVERT))
1180+
if (ra_hasreg(dest) && irt_isnum(t) && !(ir->op2 & IRSLOAD_CONVERT))
11811181
emit_dn(as, A64I_FMOV_D_R, (dest & 31), tmp);
11821182
/* Need type check, even if the load result is unused. */
11831183
asm_guardcc(as, irt_isnum(t) ? CC_LS : CC_NE);
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
local tap = require('tap')
2+
-- Test file to demonstrate the incorrect JIT assembling of unused
3+
-- `IR_SLOAD` with number type on arm64.
4+
-- See also https://github.com/LuaJIT/LuaJIT/issues/903.
5+
local test = tap.test('lj-903-arm64-unused-number-sload-typecheck'):skipcond({
6+
['Test requires JIT enabled'] = not jit.status(),
7+
})
8+
9+
test:plan(1)
10+
11+
-- Just use any different numbers (but not integers to avoid
12+
-- integer IR type).
13+
local SLOT = 0.1
14+
local MARKER_VALUE = 4.2
15+
-- XXX: Special mapping to avoid folding and removing always true
16+
-- comparison.
17+
local anchor = {marker = MARKER_VALUE}
18+
19+
-- Special function to inline on trace to generate SLOAD
20+
-- typecheck.
21+
local function sload_unused(x)
22+
return x
23+
end
24+
25+
-- The additional wrapper to use stackslots in the function.
26+
local function test_sload()
27+
local sload = SLOT
28+
for _ = 1, 4 do
29+
-- This line should use the `d0` register.
30+
local marker = anchor.marker - MARKER_VALUE
31+
-- This generates unused IR_SLOAD with typecheck (number).
32+
-- Before the patch, it occasionally overwrites the `d0`
33+
-- register and causes the execution of the branch.
34+
sload_unused(sload)
35+
if marker ~= 0 then
36+
return false
37+
end
38+
end
39+
return true
40+
end
41+
42+
jit.opt.start('hotloop=1')
43+
test:ok(test_sload(), 'correct SLOAD assembling')
44+
45+
test:done(true)

0 commit comments

Comments
 (0)