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 AFCFB1270B27; Mon, 10 Mar 2025 17:52:39 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org AFCFB1270B27 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1741618359; bh=zDiKdGhsZ2vouDwXlKVoo4MeQ7P7f1x8WbyqqUG3D9k=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=Nj5/SE1bkyiDb9whJSzR8v0ZIn3Bf7gqOUmY83BJGn13iBGmL/MBk1pnF9XWk/W9v L1B9/Ixh+4OkT50/pQZAkAkdZjR3PwgglzOPs597W1ePGPM6Xk7P8DhcWTImkZXE2p iD9zp+0P1NtRpzoopeISY6dQr5TmN0raBTzgz4sY= Received: from send194.i.mail.ru (send194.i.mail.ru [95.163.59.33]) (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 788A31270B11 for ; Mon, 10 Mar 2025 17:51:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 788A31270B11 Received: by exim-smtp-8cb569c79-b7mkz with esmtpa (envelope-from ) id 1treTj-00000000Jsz-2eh2; Mon, 10 Mar 2025 17:51:40 +0300 To: Sergey Bronnikov Date: Mon, 10 Mar 2025 17:51:36 +0300 Message-ID: <9dbb7d455c1953fcab6a82944b073348c17e46ac.1741617766.git.skaplun@tarantool.org> X-Mailer: git-send-email 2.48.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9C6CD12EFD8DA450FE5F94F59E023066E5B155EFC5C897E0F00894C459B0CD1B988977EBCB090AFE78E7FD94675F3356575FFA5A87B9AA27B53C58652A57A77A2785316FF18B29525 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7CE54A8686262D0D1EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375666DAE1D1A19A1A03E731B7EF0D9A804C1163AB99D7AE6C33ADE13C47C491026A2389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0AF05157F0BAFB9978941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B68CC5112E3E56BCDBCC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C22490F250D17497FEF6176E601842F6C81A12EF20D2F80756B5FB606B96278B59C4276E601842F6C81A127C277FBC8AE2E8B8DBB596EC94336063AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E7356C9A9530EBF72002C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A54AB6BE7040D53B2B5002B1117B3ED6967F29E241A038DAD9E99897350C7C491E823CB91A9FED034534781492E4B8EEAD1664F48CFDD0DEADC79554A2A72441328621D336A7BC284946AD531847A6065A17B107DEF921CE79BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF698F8CEF598D76CE1628070B25105493ADED60FE3EEE9E3F988A22A85A12BB128853F4E7B5962328F13513DA892F5B9DD5D3EB0CC0FAD286DAD5AB2F7821127752274A29A37EDCE65F4332CA8FE04980913E6812662D5F2A5EAB5682573093F7837F15F2B5E4A70B33F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVQiWK+2I7Y2sdgh5SAGV9Gw= X-Mailru-Sender: 520A125C2F17F0B1A9638AD358559B59641165FBDA3BD7013DE06ABAFEAF670547933BE82C157479B7CBEF92542CD7C88B0A2698F12F5C9EC77752E0C033A69E86920BD37369036789A8C6A0E60D2BB63A5DB60FBEB33A8A0DA7A0AF5A3A8387 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit 2/3] Restore state when recording __concat metamethod throws OOM. 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 Reported by Sergey Kaplun. (cherry picked from commit 19878ec05c239ccaf5f3d17af27670a963e25b8b) Before the patch, the Lua stack isn't restored if the OOM related to string allocation happens during fold optimization inside `rec_cat()`. This patch moves most of the `rec_cat()` logic to the protected `rec_mm_concat_cp()` to be able to restore the stack in case of OOM. Unfortunately, after this patch, the test fails since the incorrect adjusting of the topslot. It will be fixed in the next patch. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11055 --- src/lj_record.c | 51 +++++++++++------- .../lj-1298-oom-on-concat-recording.test.lua | 53 +++++++++++++++++++ test/tarantool-tests/utils/CMakeLists.txt | 1 + test/tarantool-tests/utils/allocinject.c | 21 ++++++++ 4 files changed, 108 insertions(+), 18 deletions(-) create mode 100644 test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua diff --git a/src/lj_record.c b/src/lj_record.c index 5345fa63..7a481a51 100644 --- a/src/lj_record.c +++ b/src/lj_record.c @@ -1943,25 +1943,19 @@ static TRef rec_tnew(jit_State *J, uint32_t ah) typedef struct RecCatDataCP { jit_State *J; - RecordIndex *ix; + BCReg baseslot, topslot; + TRef tr; } RecCatDataCP; static TValue *rec_mm_concat_cp(lua_State *L, lua_CFunction dummy, void *ud) { RecCatDataCP *rcd = (RecCatDataCP *)ud; - UNUSED(L); UNUSED(dummy); - rec_mm_arith(rcd->J, rcd->ix, MM_concat); /* Call __concat metamethod. */ - return NULL; -} - -static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot) -{ + jit_State *J = rcd->J; + BCReg baseslot = rcd->baseslot, topslot = rcd->topslot; TRef *top = &J->base[topslot]; - TValue savetv[5+LJ_FR2]; BCReg s; RecordIndex ix; - RecCatDataCP rcd; - int errcode; + UNUSED(L); UNUSED(dummy); lj_assertJ(baseslot < topslot, "bad CAT arg"); for (s = baseslot; s <= topslot; s++) (void)getslot(J, s); /* Ensure all arguments have a reference. */ @@ -1983,7 +1977,10 @@ static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot) } while (trp <= top); tr = emitir(IRT(IR_BUFSTR, IRT_STR), tr, hdr); J->maxslot = (BCReg)(xbase - J->base); - if (xbase == base) return tr; /* Return simple concatenation result. */ + if (xbase == base) { + rcd->tr = tr; /* Return simple concatenation result. */ + return NULL; + } /* Pass partial result. */ topslot = J->maxslot--; *xbase = tr; @@ -1996,13 +1993,31 @@ static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot) copyTV(J->L, &ix.tabv, &J->L->base[topslot-1]); ix.tab = top[-1]; ix.key = top[0]; - memcpy(savetv, &J->L->base[topslot-1], sizeof(savetv)); /* Save slots. */ + rec_mm_arith(J, &ix, MM_concat); /* Call __concat metamethod. */ + rcd->tr = 0; /* No result yet. */ + return NULL; +} + +static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot) +{ + lua_State *L = J->L; + ptrdiff_t delta = L->top - L->base; + TValue savetv[5+LJ_FR2], errobj; + RecCatDataCP rcd; + int errcode; rcd.J = J; - rcd.ix = &ix; - errcode = lj_vm_cpcall(J->L, NULL, &rcd, rec_mm_concat_cp); - memcpy(&J->L->base[topslot-1], savetv, sizeof(savetv)); /* Restore slots. */ - if (errcode) return (TRef)(-errcode); - return 0; /* No result yet. */ + rcd.baseslot = baseslot; + rcd.topslot = topslot; + memcpy(savetv, &L->base[topslot-1], sizeof(savetv)); /* Save slots. */ + errcode = lj_vm_cpcall(L, NULL, &rcd, rec_mm_concat_cp); + if (errcode) copyTV(L, &errobj, L->top-1); + memcpy(&L->base[topslot-1], savetv, sizeof(savetv)); /* Restore slots. */ + if (errcode) { + L->top = L->base + delta; + copyTV(L, L->top++, &errobj); + return (TRef)(-errcode); + } + return rcd.tr; } /* -- Record bytecode ops ------------------------------------------------- */ diff --git a/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua new file mode 100644 index 00000000..961df798 --- /dev/null +++ b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua @@ -0,0 +1,53 @@ +local tap = require('tap') + +-- Test file to demonstrate unbalanced Lua stack after instruction +-- recording due to throwing an OOM error at the moment of +-- recording without restoring the Lua stack back. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1298. + +local test = tap.test('lj-1298-oom-on-concat-recording'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +local allocinject = require('allocinject') + +test:plan(2) + +jit.opt.start('hotloop=1') + +-- Allocation limit to return `NULL`. +local ALLOC_LIMIT = 2048 + +local bigstr = string.rep('x', ALLOC_LIMIT) +local __concat = function() + return 'concated' +end + +-- Need to use metamethod call in the concat recording. +-- We may use any object with a metamethod, but let's use a table +-- as the most common one. +local concatable_t = setmetatable({}, { + __concat = __concat, +}) + +local function concat_with_err() + local counter = 0 + local _ + while counter < 1 do + counter = counter + 1 + _ = bigstr .. concatable_t .. '' + end + assert(false, 'unreachable, should raise an error before') +end + +-- Returns `NULL` on any allocation beyond the given limit. +allocinject.enable_null_limited_alloc(ALLOC_LIMIT) + +local status, errmsg = pcall(concat_with_err) + +allocinject.disable() + +test:ok(not status, 'no assertion failure during recording, correct status') +test:like(errmsg, 'not enough memory', 'correct error message') + +test:done(true) diff --git a/test/tarantool-tests/utils/CMakeLists.txt b/test/tarantool-tests/utils/CMakeLists.txt index 624cc933..15871934 100644 --- a/test/tarantool-tests/utils/CMakeLists.txt +++ b/test/tarantool-tests/utils/CMakeLists.txt @@ -2,5 +2,6 @@ list(APPEND tests lj-1166-error-stitch-oom-ir-buff.test.lua lj-1166-error-stitch-oom-snap-buff.test.lua lj-1247-fin-tab-rehashing-on-trace.test.lua + lj-1298-oom-on-concat-recording.test.lua ) BuildTestCLib(allocinject allocinject.c "${tests}") diff --git a/test/tarantool-tests/utils/allocinject.c b/test/tarantool-tests/utils/allocinject.c index 21e1d611..7ac60625 100644 --- a/test/tarantool-tests/utils/allocinject.c +++ b/test/tarantool-tests/utils/allocinject.c @@ -34,6 +34,19 @@ static void *allocf_inj_null_doubling_realloc(void *ud, void *ptr, size_t osize, return old_allocf(ud, ptr, osize, nsize); } +static size_t limit = 0; +/* Returns `NULL` on allocations beyond the given limit. */ +static void *allocf_inj_null_limited_alloc(void *ud, void *ptr, size_t osize, + size_t nsize) +{ + assert(old_allocf != NULL); + assert(limit != 0); + /* Check the specific allocation. */ + if (osize == 0 && nsize > limit) + return NULL; + return old_allocf(ud, ptr, osize, nsize); +} + static int enable(lua_State *L, lua_Alloc allocf_with_injection) { assert(old_allocf == NULL); @@ -52,6 +65,13 @@ static int enable_null_doubling_realloc(lua_State *L) return enable(L, allocf_inj_null_doubling_realloc); } +static int enable_null_limited_alloc(lua_State *L) +{ + limit = lua_tointeger(L, 1); + assert(limit != 0); + return enable(L, allocf_inj_null_limited_alloc); +} + /* Restore the default allocator function. */ static int disable(lua_State *L) { @@ -65,6 +85,7 @@ static int disable(lua_State *L) static const struct luaL_Reg allocinject[] = { {"enable_null_alloc", enable_null_alloc}, {"enable_null_doubling_realloc", enable_null_doubling_realloc}, + {"enable_null_limited_alloc", enable_null_limited_alloc}, {"disable", disable}, {NULL, NULL} }; -- 2.48.1