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 7B7B4C7D0C4; Wed, 21 Aug 2024 19:52:59 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7B7B4C7D0C4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1724259179; bh=csItnlDrT/OpxA6DWqZfgLzHSLIKIuz5w9ZsDusQgtQ=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=yj5/1CSqtJIWWL5qi8He5F6Mu2kCjS+hurPFqf/18lSuDl82c4cODLhqw+tX5hbTY Jotnz7mNZFQ7Q2/RoVGnMOItaL/+tkmYy8R2LSovA3ejuxoJuqP1txxAqrq+HqQmq2 WPA8U6rTSp7lBYKaNT+V8Fl/lMEnwBlGW1zDEms8= Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [95.163.41.86]) (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 5A243C7D0C0 for ; Wed, 21 Aug 2024 19:52:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5A243C7D0C0 Received: by smtp48.i.mail.ru with esmtpa (envelope-from ) id 1sgoZt-0000000CGhj-0kZ0; Wed, 21 Aug 2024 19:52:57 +0300 To: Maxim Kokryashkin , Sergey Bronnikov Date: Wed, 21 Aug 2024 19:52:50 +0300 Message-ID: <20240821165250.11087-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.45.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9BB2AFC06FD50F0EFD8D5FC3422C0A13DDF0D9CE0ED0E12C9182A05F5380850405D413ED05A2996C80578E6996F3834137D396E30DD47D23A91ED963891B6DFFD70B57125E92D9390 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E628FE8A185FCFBEEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063743447F216C7C64BDEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38B043BF0FB74779F36DBCB33CEDE95882C39626ED47DB41E6113F057A5F8C4339EA471835C12D1D9774AD6D5ED66289B5278DA827A17800CE77A825AB47F0FC8649FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3A367EA73E0D98AAD117882F4460429728AD0CFFFB425014E868A13BD56FB6657E2021AF6380DFAD1A18204E546F3947C2FFDA4F57982C5F42E808ACE2090B5E1725E5C173C3A84C317B107DEF921CE79089D37D7C0E48F6C8AA50765F7900637FB177F6A8366F17BEFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A5B9A5C8FAAA3A85EC5002B1117B3ED6961DC7CBB306981A404869453249F34FA4823CB91A9FED034534781492E4B8EEAD5C661552D0158648C79554A2A72441328621D336A7BC284946AD531847A6065A535571D14F44ED41 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF375A8C887D85D0C08E4E8F794DC66A1066DE552AD7BA19571AA7ADAFF47FDCB03D4CAFFB26F46A3DD14E51FF79EEE603A4B05F7CAC2F2D175960A4CD89CBF71B241F6C25F9E6E4C9C226CC413062362A913E6812662D5F2A5EAB5682573093F7837F15F2B5E4A70B33F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojegttVMu7AX8hw7bjLd8JVw== X-DA7885C5: DF66F1975F109901F255D290C0D534F94503420015333977193A31E879712B9B1A11AE1036E19DDB5B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F7393C6D0B12EA33CAA9B832FB03CAACA63D658D96AB1F15567F75234E45A72A6750FE49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] FFI: Fix various issues in recff_cdata_arith. 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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Thanks to Sergey Kaplun. (cherry picked from commit 7a608e4425ce0777f5c980dad9f4fdc1bcce0b8c) The aforementioned function doesn't handle gentle recording of the cdata addition to `nil` or some string, presuming that the interpreter will throw an error. This may lead to an assertion due to an uninitialized ctype state or an attempt to use in the fold engine the non-cdata summand (casted to `IR_KPTR`) as the (invalid) GC pointer. This patch handles such cases by: * Initializing the ctype state where it is needed. * Raising an error when the argument has a suspicious type. Since the interpreter will throw the error anyway, these traces will abort anyway. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#10199 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1224-fix-jit-cdata-arith Related issues: * https://github.com/tarantool/tarantool/issues/10199 * https://github.com/LuaJIT/LuaJIT/issues/1224 src/lj_crecord.c | 10 ++-- .../lj-1224-fix-cdata-arith-ptr.test.lua | 48 +++++++++++++++++++ .../lj-1224-fix-cdata-arith-side.test.lua | 20 ++++++++ .../lj-1224-fix-cdata-arith-side/script.lua | 16 +++++++ .../lj-1224-fix-cdata-arith.test.lua | 20 ++++++++ .../lj-1224-fix-cdata-arith/script.lua | 18 +++++++ 6 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 test/tarantool-tests/lj-1224-fix-cdata-arith-ptr.test.lua create mode 100644 test/tarantool-tests/lj-1224-fix-cdata-arith-side.test.lua create mode 100644 test/tarantool-tests/lj-1224-fix-cdata-arith-side/script.lua create mode 100644 test/tarantool-tests/lj-1224-fix-cdata-arith.test.lua create mode 100644 test/tarantool-tests/lj-1224-fix-cdata-arith/script.lua diff --git a/src/lj_crecord.c b/src/lj_crecord.c index 255bfa45..81ae6dfa 100644 --- a/src/lj_crecord.c +++ b/src/lj_crecord.c @@ -1473,7 +1473,8 @@ static TRef crec_arith_meta(jit_State *J, TRef *sp, CType **s, CTState *cts, void LJ_FASTCALL recff_cdata_arith(jit_State *J, RecordFFData *rd) { - CTState *cts = ctype_ctsG(J2G(J)); + CTState *cts = ctype_cts(J->L); + MMS mm = (MMS)rd->data; TRef sp[2]; CType *s[2]; MSize i; @@ -1523,6 +1524,8 @@ void LJ_FASTCALL recff_cdata_arith(jit_State *J, RecordFFData *rd) } } } else if (tref_isnil(tr)) { + if (!(mm == MM_len || mm == MM_eq || mm == MM_lt || mm == MM_le)) + lj_trace_err(J, LJ_TRERR_BADTYPE); tr = lj_ir_kptr(J, NULL); ct = ctype_get(cts, CTID_P_VOID); } else if (tref_isinteger(tr)) { @@ -1541,12 +1544,12 @@ void LJ_FASTCALL recff_cdata_arith(jit_State *J, RecordFFData *rd) ct = ctype_child(cts, cct); tr = lj_ir_kint(J, (int32_t)ofs); } else { /* Interpreter will throw or return false. */ - ct = ctype_get(cts, CTID_P_VOID); + lj_trace_err(J, LJ_TRERR_BADTYPE); } } else if (ctype_isptr(ct->info)) { tr = emitir(IRT(IR_ADD, IRT_PTR), tr, lj_ir_kintp(J, sizeof(GCstr))); } else { - ct = ctype_get(cts, CTID_P_VOID); + lj_trace_err(J, LJ_TRERR_BADTYPE); } } else if (!tref_isnum(tr)) { tr = 0; @@ -1558,7 +1561,6 @@ void LJ_FASTCALL recff_cdata_arith(jit_State *J, RecordFFData *rd) } { TRef tr; - MMS mm = (MMS)rd->data; if ((mm == MM_len || mm == MM_concat || (!(tr = crec_arith_int64(J, sp, s, mm)) && !(tr = crec_arith_ptr(J, sp, s, mm)))) && diff --git a/test/tarantool-tests/lj-1224-fix-cdata-arith-ptr.test.lua b/test/tarantool-tests/lj-1224-fix-cdata-arith-ptr.test.lua new file mode 100644 index 00000000..9de9586d --- /dev/null +++ b/test/tarantool-tests/lj-1224-fix-cdata-arith-ptr.test.lua @@ -0,0 +1,48 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT's incorrect recording of cdata +-- arithmetic looks like pointer arithmetic, which raises the +-- error. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1224. + +local test = tap.test('lj-1224-fix-cdata-arith-ptr'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(2) + +local function arith_nil_protected() + local i = 1 + while i < 3 do + -- Before the patch, `nil` was "cast" to `IR_KPTR(NULL)` + -- during the recording of the cdata arith metamethod, and the + -- fold engine tried to add this value to the `-1LL`. This + -- obviously leads to the value being out-of-range for the GC + -- pointer. + local _ = -1LL + nil + i = i + 1 + end +end + +-- The similar test but for the string value instead of nil. +local function arith_str_protected() + local i = 1 + while i < 3 do + local _ = 1LL + '' + i = i + 1 + end +end + +local function check_error(subtest, test_f) + subtest:plan(2) + -- Reset counters. + jit.opt.start('hotloop=1') + local result, errmsg = pcall(test_f) + subtest:ok(not result, 'correct recording error with bad cdata arithmetic') + subtest:like(errmsg, 'attempt to perform arithmetic', 'correct error message') +end + +test:test('cdata arithmetic with nil', check_error, arith_nil_protected) +test:test('cdata arithmetic with string', check_error, arith_str_protected) + +test:done(true) diff --git a/test/tarantool-tests/lj-1224-fix-cdata-arith-side.test.lua b/test/tarantool-tests/lj-1224-fix-cdata-arith-side.test.lua new file mode 100644 index 00000000..3f3e6011 --- /dev/null +++ b/test/tarantool-tests/lj-1224-fix-cdata-arith-side.test.lua @@ -0,0 +1,20 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT's incorrect recording for the +-- side trace with cdata arithmetic, which raises the error. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1224. + +local test = tap.test('lj-1224-fix-cdata-arith-side'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +-- The loading of the 'tap' module initializes `cts->L` during +-- parsing. Run standalone script for testing. +local script = require('utils').exec.makecmd(arg) + +test:plan(1) + +local output = script() +test:is(output, 'OK', 'correct recording with uninitialized cts->L') + +test:done(true) diff --git a/test/tarantool-tests/lj-1224-fix-cdata-arith-side/script.lua b/test/tarantool-tests/lj-1224-fix-cdata-arith-side/script.lua new file mode 100644 index 00000000..e96877ff --- /dev/null +++ b/test/tarantool-tests/lj-1224-fix-cdata-arith-side/script.lua @@ -0,0 +1,16 @@ +local function test_record_protected() + -- Start the root trace here. + for _ = 1, 3 do end + -- An exit from the root trace triggered the side trace + -- recording. + local _ = 1LL + '' +end + +jit.opt.start('hotloop=1', 'hotexit=1') + +local status, errmsg = pcall(test_record_protected) + +assert(not status, 'recoding correctly handling the error') +assert(errmsg:match('attempt to perform arithmetic'), 'correct error message') + +print('OK') diff --git a/test/tarantool-tests/lj-1224-fix-cdata-arith.test.lua b/test/tarantool-tests/lj-1224-fix-cdata-arith.test.lua new file mode 100644 index 00000000..ba89b472 --- /dev/null +++ b/test/tarantool-tests/lj-1224-fix-cdata-arith.test.lua @@ -0,0 +1,20 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT's incorrect recording of cdata +-- arithmetic, which raises the error. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1224. + +local test = tap.test('lj-1224-fix-cdata-arith'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +-- The loading of the 'tap' module initializes `cts->L` during +-- parsing. Run standalone script for testing. +local script = require('utils').exec.makecmd(arg) + +test:plan(1) + +local output = script() +test:is(output, 'OK', 'correct recording with uninitialized cts->L') + +test:done(true) diff --git a/test/tarantool-tests/lj-1224-fix-cdata-arith/script.lua b/test/tarantool-tests/lj-1224-fix-cdata-arith/script.lua new file mode 100644 index 00000000..df79e430 --- /dev/null +++ b/test/tarantool-tests/lj-1224-fix-cdata-arith/script.lua @@ -0,0 +1,18 @@ +local function test_record_protected() + local i = 1 + while i < 3 do + -- Use `while` to start compilation of the trace at the first + -- iteration, before `cts->L` is uninitialized. + local _ = 1LL + nil + i = i + 1 + end +end + +jit.opt.start('hotloop=1') + +local status, errmsg = pcall(test_record_protected) + +assert(not status, 'recoding correctly handling the error') +assert(errmsg:match('attempt to perform arithmetic'), 'correct error message') + +print('OK') -- 2.45.2