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 CB12ECCA891; Fri, 13 Sep 2024 16:07:06 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CB12ECCA891 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1726232826; bh=8OxNwBwOGe3NgAtbi3JWnMYCNZl89bOc9KZ0nW2lrCU=; 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=NRYPEGVn1oxbsMxEaggl7Lu8nranNQ7gGb8ZfzCOV5yhXN+gt/SJykkauZxpD75q9 RH5tNwiVmetznyGku5pHmTcoT0fuKr01WSoZUOun4HnDubWT9ZXJPOrzj3H7p345ro aO6+EHrUgRIaeZ61ek566n5JMwQXUUnYepWi6t30= Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [95.163.41.89]) (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 2D49ECCA891 for ; Fri, 13 Sep 2024 16:07:06 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2D49ECCA891 Received: by smtp54.i.mail.ru with esmtpa (envelope-from ) id 1sp60u-00000002Df7-0Jss; Fri, 13 Sep 2024 16:07:04 +0300 Content-Type: multipart/alternative; boundary="------------UpOhd6sNx02pL2k81LjpXSOH" Message-ID: <695c49d0-f2f0-4cfb-b5d3-e4ac52d1f455@tarantool.org> Date: Fri, 13 Sep 2024 16:07:03 +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: <20240821082316.4880-1-skaplun@tarantool.org> In-Reply-To: <20240821082316.4880-1-skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9149934E261B3C850136D25067E1DFD7C8D3A3D81EBC71DE5182A05F538085040BE08B1D0F6AC655BF378A8CA21F699D6C805A33C07CD7F7955D9AFB566F70C9D906DE0852CB239E4 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE736691C7D10565E03C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE78C722B68A3D10D1CEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38B043BF0FB74779F3622A38E864E42F2B50A7B8F729D27D3BD9D167E9890FCABACA471835C12D1D9774AD6D5ED66289B5259CC434672EE6371117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE7850F8B975A76562C9FA2833FD35BB23DF004C90652538430302FCEF25BFAB3454AD6D5ED66289B5278DA827A17800CE7D36F53DD076E3CB7D32BA5DBAC0009BE395957E7521B51C2330BD67F2E7D9AF1090A508E0FED6299176DF2183F8FC7C0E3E3FB6EC827F0A0CD04E86FAF290E2DB606B96278B59C421DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B60425ED1B70270490089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A549F052E3575895015002B1117B3ED696559EE9A7AC2526F8B91D2EB2DEE3878C823CB91A9FED034534781492E4B8EEAD21D4E6D365FE45D1BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFC99F2AC42404D758B7FABB25C5553A2C4534A2EC3F1595EA147801BFF10FDADEE547CA128C9EF438BEF149BEF2827C689ABE51FE634DD056C950CB7621D88276944ACB3049DF0FBF5F4332CA8FE04980913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojmn7hiCWKTugnVKBXO0KgJQ== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D6140151ACDFB04536BB822D5C4874F525AA3E2F7611AC1822FDD0152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Restore state when recording __concat metamethod throws an error. 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" This is a multi-part message in MIME format. --------------UpOhd6sNx02pL2k81LjpXSOH Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Sergey, thanks for the patch! LGTM with a minor comment. See below. On 21.08.2024 11:23, Sergey Kaplun wrote: > diff --git a/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua > new file mode 100644 > index 00000000..9abaeba5 > --- /dev/null > +++ b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua > @@ -0,0 +1,43 @@ > +local tap = require('tap') > + > +-- Test file to demonstrate the crash during the concat recording > +-- if it throws an error. > +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1234. > + > +local test = tap.test('lj-1234-err-in-record-concat'):skipcond({ > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +test:plan(2) > + > +jit.opt.start('hotloop=1') > + > +local __concat = function(v1, v2) > + return tostring(v1) .. tostring(v2) > +end > + > +-- Need to use metamethod call in the concat recording. > +debug.setmetatable(nil, { I propose to add a comment that explain why `nil` is used here. > + __concat = __concat, > +}) > + > +local function test_concat_p() > + local counter = 0 > + while counter < 1 do > + counter = counter + 1 > + -- The first result is placed on the Lua stack before the > + -- error is raised. When the error is raised, it is handled by > + -- the trace recorder, but since neither `rec_cat()` nor > + -- `lj_record_ret()` restore the Lua stack (before the patch), > + -- it becomes unbalanced after the instruction recording > + -- attempt. > + local _ = {} .. (nil .. nil) > + end > +end > + > +local result, errmsg = pcall(test_concat_p) > + > +test:ok(not result, 'the error is raised') > +test:like(errmsg, 'attempt to concatenate a table value', 'correct error') > + > +test:done(true) --------------UpOhd6sNx02pL2k81LjpXSOH Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hi, Sergey,

thanks for the patch! LGTM with a minor comment. See below.

On 21.08.2024 11:23, Sergey Kaplun wrote:


<snipped>

diff --git a/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua
new file mode 100644
index 00000000..9abaeba5
--- /dev/null
+++ b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua
@@ -0,0 +1,43 @@
+local tap = require('tap')
+
+-- Test file to demonstrate the crash during the concat recording
+-- if it throws an error.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1234.
+
+local test = tap.test('lj-1234-err-in-record-concat'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(2)
+
+jit.opt.start('hotloop=1')
+
+local __concat = function(v1, v2)
+    return tostring(v1) .. tostring(v2)
+end
+
+-- Need to use metamethod call in the concat recording.
+debug.setmetatable(nil, {
I propose to add a comment that explain why `nil` is used here.
+  __concat = __concat,
+})
+
+local function test_concat_p()
+  local counter = 0
+  while counter < 1 do
+    counter = counter + 1
+    -- The first result is placed on the Lua stack before the
+    -- error is raised. When the error is raised, it is handled by
+    -- the trace recorder, but since neither `rec_cat()` nor
+    -- `lj_record_ret()` restore the Lua stack (before the patch),
+    -- it becomes unbalanced after the instruction recording
+    -- attempt.
+    local _ = {} .. (nil .. nil)
+  end
+end
+
+local result, errmsg = pcall(test_concat_p)
+
+test:ok(not result, 'the error is raised')
+test:like(errmsg, 'attempt to concatenate a table value', 'correct error')
+
+test:done(true)
--------------UpOhd6sNx02pL2k81LjpXSOH--