Skip to content

Commit 59faed0

Browse files
Mike PallBuristan
authored andcommitted
x86/x64: Don't use undefined MUL/IMUL zero flag.
Reported by VrIgHtEr. (cherry picked from commit c92d0cb) When emitting the arithmetic operations on registers via the `asm_intarith()`, the next `test` instruction may be dropped since the flag register is modified by the arithmetic instruction to be emitted. But the `imul` instruction [1] doesn't modify ZF, so its value is undefined. This patch prevents dropping the `test` instruction if the emitted instruction is `imul`. Sergey Kaplun: * added the description and the test for the problem [1]: https://www.felixcloutier.com/x86/imul Part of tarantool/tarantool#11691
1 parent a74e5be commit 59faed0

File tree

2 files changed

+40
-1
lines changed

2 files changed

+40
-1
lines changed

src/lj_asm_x86.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2061,7 +2061,8 @@ static void asm_intarith(ASMState *as, IRIns *ir, x86Arith xa)
20612061
RegSet allow = RSET_GPR;
20622062
Reg dest, right;
20632063
int32_t k = 0;
2064-
if (as->flagmcp == as->mcp) { /* Drop test r,r instruction. */
2064+
if (as->flagmcp == as->mcp && xa != XOg_X_IMUL) {
2065+
/* Drop test r,r instruction. */
20652066
MCode *p = as->mcp + ((LJ_64 && *as->mcp < XI_TESTb) ? 3 : 2);
20662067
MCode *q = p[0] == 0x0f ? p+1 : p;
20672068
if ((*q & 15) < 14) {
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
local tap = require('tap')
2+
3+
-- Test file to demonstrate incorrect assembling optimization
4+
-- for x86/x64 CPUs.
5+
-- See also: https://github.com/LuaJIT/LuaJIT/issues/1376.
6+
7+
local test = tap.test('lj-1376-undefined-mul-test-flag'):skipcond({
8+
['Test requires JIT enabled'] = not jit.status(),
9+
})
10+
11+
test:plan(1)
12+
13+
local a, b = 0ULL, 0ULL
14+
15+
jit.opt.start('hotloop=1')
16+
for _ = 1, 4 do
17+
-- Before the patch, the `test` instruction is dropped by
18+
-- assuming the `imul` instruction before it modifies the flags
19+
-- register. It results in the following mcode:
20+
-- | imul r15, rbp
21+
-- | jnz 0x559415b10060 ->5
22+
-- Instead of the following:
23+
-- | imul r15, rbp
24+
-- | test r15, r15
25+
-- | jnz 0x559415b10060 ->5
26+
-- This leads to the incorrect branch being taken.
27+
if a * b ~= 0ULL then
28+
test:fail('the impossible branch is taken')
29+
test:done(true)
30+
end
31+
-- XXX: Need to update multiplier to stay in the variant part of
32+
-- the loop, since invariant contains IR_NOP (former unused
33+
-- IR_CNEW) between IRs, and the optimization is not applied.
34+
b = b + 1
35+
end
36+
37+
test:ok(true, 'no dropping of test instruction')
38+
test:done(true)

0 commit comments

Comments
 (0)