* Re: [Tarantool-patches] Fwd: [PATCH luajit] Invalidate SCEV entry when returning to lower frame.
[not found] <aMJzTO1sL5oc0E56@root>
@ 2025-09-11 7:35 ` Sergey Bronnikov via Tarantool-patches
0 siblings, 0 replies; only message in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-09-11 7:35 UTC (permalink / raw)
To: Sergey Kaplun, tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 4510 bytes --]
Hi, Sergey,
thanks for the patch! LGTM
Sergey
On 9/11/25 09:59, Sergey Kaplun wrote:
>>
>> From: Mike Pall <mike>
>>
>> Thanks to Zhongwei Yao.
>>
>> (cherry picked from commit 65c849390702b1150d52e64db86cbc6b3c98413e)
>>
>> When returning to the lower frame, LuaJIT does not clear the Scalar
>> Evolution analysis entry. Hence, this may lead to its invalid usage in
>> the next function called if the IR references match. The further
>> analysis is invalid and may lead to the assertion failure.
>>
>> This patch invalidates the ScEv entry IR reference index when returning
>> to the lower frame.
>>
>> Sergey Kaplun:
>> * added the description and the test for the problem
>>
>> Part of tarantool/tarantool#11691
>> ---
>>
>> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1115-invalid-scev-entry-lower-frame
>> Related issues:
>> *https://github.com/LuaJIT/LuaJIT/pull/1115
>> *https://github.com/tarantool/tarantool/issues/11691
>>
>> src/lj_record.c | 1 +
>> ...15-invalid-scev-entry-lower-frame.test.lua | 69 +++++++++++++++++++
>> 2 files changed, 70 insertions(+)
>> create mode 100644 test/tarantool-tests/lj-1115-invalid-scev-entry-lower-frame.test.lua
>>
>> diff --git a/src/lj_record.c b/src/lj_record.c
>> index 1dd22dac..ba409a61 100644
>> --- a/src/lj_record.c
>> +++ b/src/lj_record.c
>> @@ -898,6 +898,7 @@ void lj_record_ret(jit_State *J, BCReg rbase, ptrdiff_t gotresults)
>> emitir(IRTG(IR_RETF, IRT_PGC), trpt, trpc);
>> J->retdepth++;
>> J->needsnap = 1;
>> + J->scev.idx = REF_NIL;
>> lj_assertJ(J->baseslot == 1+LJ_FR2, "bad baseslot for return");
>> /* Shift result slots up and clear the slots of the new frame below. */
>> memmove(J->base + cbase, J->base-1-LJ_FR2, sizeof(TRef)*nresults);
>> diff --git a/test/tarantool-tests/lj-1115-invalid-scev-entry-lower-frame.test.lua b/test/tarantool-tests/lj-1115-invalid-scev-entry-lower-frame.test.lua
>> new file mode 100644
>> index 00000000..47bc87ae
>> --- /dev/null
>> +++ b/test/tarantool-tests/lj-1115-invalid-scev-entry-lower-frame.test.lua
>> @@ -0,0 +1,69 @@
>> +local tap = require('tap')
>> +
>> +-- Test file to demonstrate LuaJIT's incorrect Scalar Evolution
>> +-- analysis for recording of return to a lower frame.
>> +-- See also:https://github.com/LuaJIT/LuaJIT/pull/1115.
>> +local test = tap.test('lj-1115-invalid-scev-entry-lower-frame'):skipcond({
>> + ['Test requires JIT enabled'] = not jit.status(),
>> +})
>> +
>> +test:plan(1)
>> +
>> +local HOTLOOP = 1
>> +local HOTEXIT = 1
>> +local RECORD_IDX = HOTLOOP + 1
>> +-- Number of iterations to start recording side trace with two
>> +-- iterations in the cycle.
>> +local NITER = RECORD_IDX + HOTEXIT + 2
>> +
>> +local function test_function(tab)
>> + -- XXX: For reproducing the issue, it is necessary to avoid
>> + -- UGET. Local functions use MOV and take the same IR slots.
>> + local function trace_root(data)
>> + -- Start of the trace, setup ScEv entry.
>> + for i = 1, #data - 1 do
>> + -- Start of the side trace by the hmask check.
>> + if data[i].t == 'a' then
>> + return i + 1
>> + end
>> + end
>> + -- Unreachable in this test.
>> + return nil
>> + end
>> +
>> + local function other_scev(data, start)
>> + for i = start, #data - 1 do
>> + -- The ScEv entry matches the recorded IR from the parent
>> + -- trace before the patch. It leads to the assertion
>> + -- failure.
>> + if data[i].t == 'a' then
>> + return
>> + end
>> + end
>> + end
>> +
>> + -- Record the root trace first. Then record the side trace
>> + -- returning to the lower frame (this function).
>> + local start = trace_root(tab)
>> + -- The ScEv entry is invalid after the return to the lower
>> + -- frame. Record the trace with another range in the ScEv entry
>> + -- to obtain the error.
>> + return start, other_scev(tab, start)
>> +end
>> +
>> +local data = {}
>> +for i = 1, NITER do
>> + data[#data + 1] = {t = 'a' .. i}
>> +end
>> +
>> +-- Change the hmask value to start the side trace recording.
>> +data[RECORD_IDX] = {}
>> +-- Setup for the trace's return to the lower frame.
>> +data[NITER - 2] = {t = 'a'}
>> +
>> +jit.opt.start('hotloop=' .. HOTLOOP, 'hotexit=' .. HOTEXIT)
>> +
>> +test_function(data)
>> +
>> +test:ok(true, 'correct ScEv entry invalidation for return to a lower frame')
>> +test:done(true)
>> --
>> 2.51.0
>>
> ----- End forwarded message -----
>
[-- Attachment #2: Type: text/html, Size: 5121 bytes --]
^ permalink raw reply [flat|nested] only message in thread