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 c8bcf1e) `fold_abc_k()` doesn't patch the first ABC check when the right constant operand is negative. This leads to out-of-bounds access from the array on a trace. This patch casts the operands to uint32_t for comparison. If the right IR contains a negative integer, the second IR will always be patched. Also, because the ABC check on the trace is unordered, this guard will always fail. Also, this fold rule creates new instructions that reference operands across PHIs. This opens the room for other optimizations (like DCE), so some guards become eliminated, and we use out-of-bounds access from the array part of the table on trace. This patch adds the missing `PHIBARRIER()` check. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#9145
- Loading branch information
1 parent
5bb1901
commit 3a5de42
Showing
2 changed files
with
95 additions
and
2 deletions.
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,92 @@ | ||
local tap = require('tap') | ||
|
||
-- Test file to demonstrate LuaJIT's incorrect fold optimization | ||
-- for Array Bound Check for constants. | ||
-- ABC(asize, k1), ABC(asize k2) ==> ABC(asize, max(k1, k2)). | ||
-- See also https://github.com/LuaJIT/LuaJIT/issues/794. | ||
|
||
local test = tap.test('lj-794-abc-fold-constants'):skipcond({ | ||
['Test requires JIT enabled'] = not jit.status(), | ||
}) | ||
|
||
test:plan(2) | ||
|
||
local MAGIC_UNUSED = 42 | ||
|
||
local function abc_check_sign() | ||
local tab = {MAGIC_UNUSED} | ||
local return_value = 0 | ||
local abc_limit = 1 | ||
-- No need to run the loop on the first call. We will take | ||
-- the side exit anyway. | ||
for i = 1, 3 do | ||
-- Add an additional ABC check to be merged with. | ||
if i > 1 then | ||
-- luacheck: ignore | ||
return_value = tab[1] | ||
return_value = tab[abc_limit] | ||
-- XXX: Just use some negative number. | ||
abc_limit = -1000000 | ||
end | ||
end | ||
return return_value | ||
end | ||
|
||
jit.opt.start('hotloop=1') | ||
|
||
-- Compile the loop. | ||
abc_check_sign() | ||
-- Run the loop. | ||
test:is(abc_check_sign(), nil, 'correct ABC constant rule for negative values') | ||
|
||
-- Now test the second issue, when ABC optimization applies for | ||
-- operands across PHIs. | ||
|
||
-- XXX: Reset hotcounters to avoid collisions. | ||
jit.opt.start('hotloop=1') | ||
|
||
local tab_array = {} | ||
local small_tab = {MAGIC_UNUSED} | ||
local full_tab = {} | ||
|
||
-- First, create tables with different asizes, to be used in PHI. | ||
-- Create a large enough array part for the noticeable | ||
-- out-of-bounds access. | ||
for i = 1, 8 do | ||
full_tab[i] = MAGIC_UNUSED | ||
end | ||
|
||
-- Now, store these tables in the array. The PHI should be used in | ||
-- the trace to distinguish asizes from the variant and the | ||
-- invariant parts of the loop for the future ABC check. | ||
-- Nevertheless, before the patch, the ABC IR and the | ||
-- corresponding PHI are folded via optimization. This leads to | ||
-- incorrect behaviour. | ||
-- We need 5 iterations to execute both the variant and the | ||
-- invariant parts of the trace below. | ||
for i = 1, 5 do | ||
-- On the 3rd iteration, the recording is started. | ||
if i > 3 then | ||
tab_array[i] = small_tab | ||
else | ||
tab_array[i] = full_tab | ||
end | ||
end | ||
|
||
local result | ||
local alias_tab = tab_array[1] | ||
-- Compile a trace. | ||
-- Run 5 iterations to execute both the variant and the invariant | ||
-- parts. | ||
for i = 1, 5 do | ||
local previous_tab = alias_tab | ||
alias_tab = tab_array[i] | ||
-- Additional ABC check to fold. | ||
-- luacheck: ignore | ||
result = alias_tab[1] | ||
result = previous_tab[8] | ||
end | ||
|
||
test:is(result, nil, 'correct ABC constant rule across PHI') | ||
|
||
test:done(true) |