Skip to content

Commit ac1cb36

Browse files
Mike PallBuristan
authored andcommitted
FFI: Fix 64 bit shift fold rules.
Thanks to Peter Cawley. (cherry picked from commit 9e04372) For `IR_BSHR`, `IR_BROL`, `IR_BROR` during `kfold_int64arith()` the left argument is truncated down to 32 bits, which leads to incorrect results if the right argument is >= 2^32. Also, `IR_BSAR` does an unsigned shift rather than a signed shift, but since this case branch is unreachable, it is harmless for now. This patch fixes all misbehaviours (including possible for `IR_BSAR`) to preserve IR semantics. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#10199 Reviewed-by: Sergey Bronnikov <[email protected]> Reviewed-by: Maxim Kokryashkin <[email protected]> Signed-off-by: Sergey Kaplun <[email protected]> (cherry picked from commit f0bc089)
1 parent 85ffbbf commit ac1cb36

File tree

2 files changed

+81
-4
lines changed

2 files changed

+81
-4
lines changed

src/lj_opt_fold.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,10 +382,10 @@ static uint64_t kfold_int64arith(jit_State *J, uint64_t k1, uint64_t k2,
382382
case IR_BOR: k1 |= k2; break;
383383
case IR_BXOR: k1 ^= k2; break;
384384
case IR_BSHL: k1 <<= (k2 & 63); break;
385-
case IR_BSHR: k1 = (int32_t)((uint32_t)k1 >> (k2 & 63)); break;
386-
case IR_BSAR: k1 >>= (k2 & 63); break;
387-
case IR_BROL: k1 = (int32_t)lj_rol((uint32_t)k1, (k2 & 63)); break;
388-
case IR_BROR: k1 = (int32_t)lj_ror((uint32_t)k1, (k2 & 63)); break;
385+
case IR_BSHR: k1 >>= (k2 & 63); break;
386+
case IR_BSAR: k1 = (uint64_t)((int64_t)k1 >> (k2 & 63)); break;
387+
case IR_BROL: k1 = lj_rol(k1, (k2 & 63)); break;
388+
case IR_BROR: k1 = lj_ror(k1, (k2 & 63)); break;
389389
default: lj_assertJ(0, "bad IR op %d", op); break;
390390
}
391391
#else
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
local tap = require('tap')
2+
3+
-- Test file to demonstrate LuaJIT misbehaviour on folding
4+
-- for bitshift operations.
5+
-- See also, https://github.com/LuaJIT/LuaJIT/issues/1079.
6+
7+
local test = tap.test('lj-1079-fix-64-bitshift-folds'):skipcond({
8+
['Test requires JIT enabled'] = not jit.status(),
9+
})
10+
11+
local bit = require('bit')
12+
13+
test:plan(4)
14+
15+
-- Generic function for `bit.ror()`, `bit.rol()`.
16+
local function bitop_rotation(bitop_func)
17+
local r = {}
18+
for i = 1, 4 do
19+
-- (i & k1) o k2 ==> (i o k2) & (k1 o k2)
20+
-- XXX: Don't use named constants here to match folding rules.
21+
-- `7LL` is just some mask, that doesn't change the `i` value.
22+
-- `32` is used for the half bit-width rotation.
23+
local int64 = bit.band(i, 7LL)
24+
r[i] = tonumber(bitop_func(int64, 32))
25+
end
26+
return r
27+
end
28+
29+
-- Similar function for `bit.rshift()`.
30+
local function bitop_rshift_signed()
31+
local r = {}
32+
for i = 1, 4 do
33+
-- (i & k1) o k2 ==> (i o k2) & (k1 o k2)
34+
-- XXX: Use `-i` instead of `i` to prevent other folding due
35+
-- to IR difference so the IRs don't match fold rule mask.
36+
-- (-i & 7LL) < 1 << 32 => result == 0.
37+
local int64 = bit.band(-i, 7LL)
38+
r[i] = tonumber(bit.rshift(int64, 32))
39+
end
40+
return r
41+
end
42+
43+
-- A little bit different example, which leads to the assertion
44+
-- failure due to the incorrect recording.
45+
local function bitop_rshift_huge()
46+
local r = {}
47+
for i = 1, 4 do
48+
-- (i & k1) o k2 ==> (i o k2) & (k1 o k2)
49+
-- XXX: Need to use cast to the int64_t via `+ 0LL`, see the
50+
-- documentation [1] for the details.
51+
-- [1]: https://bitop.luajit.org/semantics.html
52+
local int64 = bit.band(2 ^ 33 + i, 2 ^ 33 + 0LL)
53+
r[i] = tonumber(bit.rshift(int64, 32))
54+
end
55+
return r
56+
end
57+
58+
local function test_64bitness(subtest, payload_func, bitop_func)
59+
subtest:plan(1)
60+
61+
jit.off()
62+
jit.flush()
63+
local results_joff = payload_func(bitop_func)
64+
jit.on()
65+
-- Reset hotcounters.
66+
jit.opt.start('hotloop=1')
67+
local results_jon = payload_func(bitop_func)
68+
subtest:is_deeply(results_jon, results_joff,
69+
'same results for VM and JIT for ' .. subtest.name)
70+
end
71+
72+
test:test('rshift signed', test_64bitness, bitop_rshift_signed)
73+
test:test('rshift huge', test_64bitness, bitop_rshift_huge)
74+
test:test('rol', test_64bitness, bitop_rotation, bit.rol)
75+
test:test('ror', test_64bitness, bitop_rotation, bit.ror)
76+
77+
test:done(true)

0 commit comments

Comments
 (0)