Skip to content

Commit 303dbad

Browse files
Mike Palligormunkin
Mike Pall
authored andcommitted
Fix FOLD rule for BUFHDR append.
Reported by XmiliaH. (cherry-picked from commit bc1bdbf) `bufput_append()` may fold `BUFHDR RESET` + `BUFPUT` IRs to `BUFHDR APPEND` even if the right operand (`BUFSTR`) is the PHI. If it's not the last IR in the `BUFSTR` chain, this may lead to an incorrect resulting value in the buffer, which contains a longer string since `APPEND` is used instead of `RESET`. This patch adds the corresponding check inside the fold rule. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#9145 Reviewed-by: Maxim Kokryashkin <[email protected]> Reviewed-by: Sergey Bronnikov <[email protected]> Signed-off-by: Igor Munkin <[email protected]> (cherry picked from commit e895818)
1 parent 8ef4901 commit 303dbad

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

src/lj_opt_fold.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,8 @@ LJFOLDF(bufput_append)
584584
if ((J->flags & JIT_F_OPT_FWD) &&
585585
!(fleft->op2 & IRBUFHDR_APPEND) &&
586586
fleft->prev == fright->op2 &&
587-
fleft->op1 == IR(fright->op2)->op1) {
587+
fleft->op1 == IR(fright->op2)->op1 &&
588+
!(irt_isphi(fright->t) && IR(fright->op2)->prev)) {
588589
IRRef ref = fins->op1;
589590
IR(ref)->op2 = (fleft->op2 | IRBUFHDR_APPEND); /* Modify BUFHDR. */
590591
IR(ref)->op1 = fright->op1;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
local tap = require('tap')
2+
3+
-- Test file to demonstrate the incorrect LuaJIT's optimization
4+
-- `bufput_append()` for BUFPUT IR.
5+
-- See also https://github.com/LuaJIT/LuaJIT/issues/791.
6+
7+
local test = tap.test('lj-791-fold-bufhdr-append'):skipcond({
8+
['Test requires JIT enabled'] = not jit.status(),
9+
})
10+
11+
test:plan(1)
12+
13+
local EMPTY_STR = ''
14+
local prefix = 'Lu'
15+
local result
16+
17+
jit.opt.start('hotloop=1')
18+
19+
-- The interesting part of IRs is the following (non-GC64 mode):
20+
-- 0006 str BUFSTR 0005 0003
21+
-- 0007 > str SLOAD #2 T
22+
-- 0008 p32 BUFHDR [0x400004a0] RESET
23+
-- 0009 p32 BUFPUT 0008 "Lu"
24+
-- 0010 p32 BUFPUT 0009 0007
25+
-- 0011 + str BUFSTR 0010 0008
26+
-- 0012 + int ADD 0001 +1
27+
-- 0013 > int LE 0012 +5
28+
-- 0014 > --- LOOP ------------
29+
-- 0015 p32 BUFHDR [0x400004a0] RESET
30+
31+
-- The instruction to be folded is the following:
32+
-- 0016 p32 BUFPUT 0015 0011
33+
--
34+
-- The 0011 operand is PHI, which is not the last IR in the BUFSTR
35+
-- chain (`ir->prev = REF_BIAS + 0006`). Folding this IR leads to
36+
-- this resulting IR:
37+
-- p32 BUFHDR 0010 APPEND
38+
-- Which appends to buffer instead of resetting, so the resulting
39+
-- string contains one more symbol.
40+
41+
-- XXX: Use 5 iterations to run the variant part of the loop.
42+
for _ = 1, 5 do
43+
result = prefix .. 'a'
44+
-- We need a non-constant string to be appended to prevent more
45+
-- aggressive optimizations. Use an empty string for
46+
-- convenience. Also, use a constant string in the first operand
47+
-- in the concatenation operator for more readable `jit.dump`
48+
-- output.
49+
prefix = 'Lu' .. EMPTY_STR
50+
end
51+
52+
test:is(result, 'Lua', 'BUFPUT APPEND optimization is not applied for PHIs')
53+
54+
test:done(true)

0 commit comments

Comments
 (0)