Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>,
	tarantool-patches <tarantool-patches@dev.tarantool.org>
Subject: Re: [Tarantool-patches] Fwd: [PATCH luajit] Invalidate SCEV entry when returning to lower frame.
Date: Thu, 11 Sep 2025 10:35:07 +0300	[thread overview]
Message-ID: <2add5454-ffe6-415c-a247-c48bf9349f9a@tarantool.org> (raw)
In-Reply-To: <aMJzTO1sL5oc0E56@root>

[-- 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 --]

           reply	other threads:[~2025-09-11  7:35 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <aMJzTO1sL5oc0E56@root>]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2add5454-ffe6-415c-a247-c48bf9349f9a@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] Fwd: [PATCH luajit] Invalidate SCEV entry when returning to lower frame.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox