From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id C379DA61CB1; Tue, 26 Mar 2024 18:08:46 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C379DA61CB1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1711465726; bh=YXGHQ5Z+D6gScmbDfEObrVk3XG5s+Nty2tCqlpM04hg=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=vin97DeOMdrutvGwQkxU9V//aOCd/O//wv0JQEjbaXQsq1UKZNi02x49kifVs9Pn/ 9a7YB0u8Jps/7qNFF9WPscGcrNuU05yNYa35aqtqqxVZxaAmmKW2bT1biCotMWfMqq DGIljjb35XXvnNTYYZycMvXpoChTIKZr4HvXIlL4= Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [95.163.41.84]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id C254EA61CAD for ; Tue, 26 Mar 2024 18:08:45 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C254EA61CAD Received: by smtp46.i.mail.ru with esmtpa (envelope-from ) id 1rp8Pt-00000000W1W-01Bk; Tue, 26 Mar 2024 18:08:45 +0300 Message-ID: Date: Tue, 26 Mar 2024 18:08:44 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun , Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org References: <20240319164148.22506-1-skaplun@tarantool.org> In-Reply-To: <20240319164148.22506-1-skaplun@tarantool.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD905284D235AF2CB533336A7736FE0774623EFF31830551C89182A05F538085040CB616A8FB22DADF83DE06ABAFEAF6705194499F8288204D1FED4BE3BDDF694672EB2DA79D556EF8B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75B37E0A1C175363BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637911538129B0A8D078638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8C0AA98DEB9727FEAEFD83B18B7E5EBF8F29982873168CD03CC7F00164DA146DAFE8445B8C89999728AA50765F790063793270F7220657A0A389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8989FD0BDF65E50FBF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C2D01283D1ACF37BA6E0066C2D8992A164AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C31BD302B0759A9E1EBA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CFE478A468B35FE7671DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C36DD0439D4B94EFE535872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A55AABEB22338856005002B1117B3ED6963761AC59497C002792B673A2F5DDD7E7823CB91A9FED034534781492E4B8EEADA2D5570B22232E1EBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF2273D5A266404C2F3ADA03A0402BE695606D34E45913D34637CFE7DAE810CA971D17C9D1E1925344F212F37558E7246619D2D9FF8C8FCDCF0949984FB2E4EBE3724A3F99797763645F4332CA8FE04980913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojhu0AwaaKnHqZDqMeRJdV2Q== X-Mailru-Sender: 520A125C2F17F0B1A9638AD358559B5936E21882D14BB4693DE06ABAFEAF6705194499F8288204D1B7CBEF92542CD7C8795FA72BAB74744FC77752E0C033A69EA16A481184E8BB1C9B38E6EA4F046BE03A5DB60FBEB33A8A0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Prevent down-recursion for side traces. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergey, thanks for the patch. LGTM with three minor comments below. Sergey On 3/19/24 19:41, Sergey Kaplun wrote: > From: Mike Pall > > Thanks to Sergey Kaplun. > > (cherry picked from commit cae361187e7e1e3545353fb560c032cdace32d5f) > > Assume we have the root trace that uses some spill slots with the > corresponding stack adjustment. Then its side trace will restore stack s/stack/the stack/ > only at its tail. It may look like the following: > > | ---- TRACE 4 mcode 1247 > | 55557f7df953 mov rax, [r14-0xe28] > | 55557f7df95a mov rax, [rax+0x30] > | 55557f7df95e sub rax, rdx > | 55557f7df961 cmp rax, +0x68 > | 55557f7df965 jb 0x55557f7d004c ->0 > | 55557f7df96b add rsp, -0x10 > | ... > | 55557f6efa71 cmp dword [rdx+0x4], -0x05 > | 55557f6efa75 jnz 0x55557f6e004c ->0 > | ... > | 55557f7dfe29 add rsp, +0x10 > | 55557f7dfe2d jmp 0x5555556fe573 > | ---- TRACE 4 stop -> stitch > | > | ---- TRACE 5 start 4/0 > | ---- TRACE 5 mcode 101 > | 55557f6ef9d4 mov dword [0x40000518], 0x5 > | ... > | 55557f6efa30 add rsp, +0x10 > | 55557f6efa34 jmp 0x55557f6ef9d4 > | ---- TRACE 5 stop -> down-recursion > > Such side traces have no stack addjustment at their heads since their s/addjustment/adjustment/ > stack addjustment is inherited from the parent trace. The issue occurs > if the side trace has a down-recursion, as mentioned above. Before any > exit, we can jump back to the start of the trace several times with > growing `rsp`. In that case, the `rsp` is restored incorrectly after > exiting from the trace. > > This patch forbids down-recursion for non-root traces. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#9595 > --- > > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1169-down-rec-side > Related issues: > * https://github.com/tarantool/tarantool/issues/9595 > * https://github.com/LuaJIT/LuaJIT/issues/1169 > > src/lj_record.c | 2 +- > .../lj-1169-down-rec-side.test.lua | 89 +++++++++++++++++++ > 2 files changed, 90 insertions(+), 1 deletion(-) > create mode 100644 test/tarantool-tests/lj-1169-down-rec-side.test.lua > > diff --git a/src/lj_record.c b/src/lj_record.c > index a6c48747..7fa56834 100644 > --- a/src/lj_record.c > +++ b/src/lj_record.c > @@ -867,7 +867,7 @@ void lj_record_ret(jit_State *J, BCReg rbase, ptrdiff_t gotresults) > if ((pt->flags & PROTO_NOJIT)) > lj_trace_err(J, LJ_TRERR_CJITOFF); > if (J->framedepth == 0 && J->pt && frame == J->L->base - 1) { > - if (check_downrec_unroll(J, pt)) { > + if (!J->cur.root && check_downrec_unroll(J, pt)) { > J->maxslot = (BCReg)(rbase + gotresults); > lj_snap_purge(J); > lj_record_stop(J, LJ_TRLINK_DOWNREC, J->cur.traceno); /* Down-rec. */ > diff --git a/test/tarantool-tests/lj-1169-down-rec-side.test.lua b/test/tarantool-tests/lj-1169-down-rec-side.test.lua > new file mode 100644 > index 00000000..63f9925f > --- /dev/null > +++ b/test/tarantool-tests/lj-1169-down-rec-side.test.lua > @@ -0,0 +1,89 @@ > +local tap = require('tap') > + > +-- Test file to demonstrate the LuaJIT misbehaviour when recording > +-- and executing a side trace with down-recursion. > +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1169. > + > +local test = tap.test('lj-1169-down-rec-side'):skipcond({ > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +local ffi = require('ffi') > + > +-- XXX: simplify `jit.dump()` output. > +local modf = math.modf > +local ffi_new = ffi.new > + > +local int64_t = ffi.typeof('int64_t') > + > +test:plan(1) > + > +-- If a parent trace has more than the default amount of spill > +-- slots, the `rsp` register is adjusted at the start of the trace > +-- and restored after. If there is a side trace created, it > +-- modifies the stack only at exit (since adjustment is inherited > +-- from a parent trace). If the side trace has down-recursion (for s/own-recursion/a down-recursion/ > +-- now only down-recursion to itself is used), `rsp` may be > +-- modified several times before exit, so the host stack becomes > +-- corrupted. > +-- > +-- This test provides the example of a side trace (5) with > +-- down-recursion. > + > +local function trace_ret(arg) -- TRACE 1 start. > + return arg -- TRACE 4 start 1/0; TRACE 5 start 4/0. > +end > + > +local function extra_frame() > + -- Stitch the trace (4) to prevent early down-recursion. > + modf(1) > + -- Start the side trace (5) with a down-recursion. > + return trace_ret({}) > +end > + > +local call_counter = 0 > +local function recursive_f() -- TRACE 2 start. > + -- XXX: 4 calls are needed to record the necessary traces after > + -- return. With the 5th call, the traces are executed. > + if call_counter > 4 then return end -- TRACE 3 start 2/1. > + call_counter = call_counter + 1 > + > + -- Compile the root trace first. > + trace_ret(1) > + trace_ret(1) > + > + recursive_f() > + -- Stop the side trace (3) here after exiting the trace (2) with > + -- up-recursion. > + modf(1) > + > + -- Start the side trace (4). > + trace_ret('') > + -- luacheck: no unused > + -- Generate register pressure to force spills. > + -- The amount is well-suited for arm64 and x86_64. > + local l1 = ffi_new(int64_t, call_counter + 1) > + local l2 = ffi_new(int64_t, call_counter + 2) > + local l3 = ffi_new(int64_t, call_counter + 3) > + local l4 = ffi_new(int64_t, call_counter + 4) > + local l5 = ffi_new(int64_t, call_counter + 5) > + local l6 = ffi_new(int64_t, call_counter + 6) > + local l7 = ffi_new(int64_t, call_counter + 7) > + local l8 = ffi_new(int64_t, call_counter + 8) > + local l9 = ffi_new(int64_t, call_counter + 9) > + local l10 = ffi_new(int64_t, call_counter + 10) > + local l11 = ffi_new(int64_t, call_counter + 11) > + local l12 = ffi_new(int64_t, call_counter + 12) > + local l13 = ffi_new(int64_t, call_counter + 13) > + -- Simulate the return to the same function using the additional > + -- frame for down-recursion. > + return trace_ret(extra_frame()) > +end > + > +jit.opt.start('hotloop=1', 'hotexit=1', 'recunroll=1') > + > +recursive_f() > + > +test:ok(true, 'no crash during execution of a trace with down-recursion') > + > +test:done(true)