forked from LuaJIT/LuaJIT
-
Notifications
You must be signed in to change notification settings - Fork 14
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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)
- Loading branch information
1 parent
553d906
commit e07c2fd
Showing
2 changed files
with
56 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
local tap = require('tap') | ||
|
||
-- Test file to demonstrate the incorrect LuaJIT's optimization | ||
-- `bufput_append()` for BUFPUT IR. | ||
-- See also https://github.com/LuaJIT/LuaJIT/issues/791. | ||
|
||
local test = tap.test('lj-791-fold-bufhdr-append'):skipcond({ | ||
['Test requires JIT enabled'] = not jit.status(), | ||
}) | ||
|
||
test:plan(1) | ||
|
||
local EMPTY_STR = '' | ||
local prefix = 'Lu' | ||
local result | ||
|
||
jit.opt.start('hotloop=1') | ||
|
||
-- The interesting part of IRs is the following (non-GC64 mode): | ||
-- 0006 str BUFSTR 0005 0003 | ||
-- 0007 > str SLOAD #2 T | ||
-- 0008 p32 BUFHDR [0x400004a0] RESET | ||
-- 0009 p32 BUFPUT 0008 "Lu" | ||
-- 0010 p32 BUFPUT 0009 0007 | ||
-- 0011 + str BUFSTR 0010 0008 | ||
-- 0012 + int ADD 0001 +1 | ||
-- 0013 > int LE 0012 +5 | ||
-- 0014 > --- LOOP ------------ | ||
-- 0015 p32 BUFHDR [0x400004a0] RESET | ||
|
||
-- The instruction to be folded is the following: | ||
-- 0016 p32 BUFPUT 0015 0011 | ||
-- | ||
-- The 0011 operand is PHI, which is not the last IR in the BUFSTR | ||
-- chain (`ir->prev = REF_BIAS + 0006`). Folding this IR leads to | ||
-- this resulting IR: | ||
-- p32 BUFHDR 0010 APPEND | ||
-- Which appends to buffer instead of resetting, so the resulting | ||
-- string contains one more symbol. | ||
|
||
-- XXX: Use 5 iterations to run the variant part of the loop. | ||
for _ = 1, 5 do | ||
result = prefix .. 'a' | ||
-- We need a non-constant string to be appended to prevent more | ||
-- aggressive optimizations. Use an empty string for | ||
-- convenience. Also, use a constant string in the first operand | ||
-- in the concatenation operator for more readable `jit.dump` | ||
-- output. | ||
prefix = 'Lu' .. EMPTY_STR | ||
end | ||
|
||
test:is(result, 'Lua', 'BUFPUT APPEND optimization is not applied for PHIs') | ||
|
||
test:done(true) |