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 1D760772EA1; Thu, 4 Apr 2024 19:18:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1D760772EA1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1712247494; bh=nDK1pcVjs2VQKQHagEvzSQ70No0+YwgLyKcJ9GicsoU=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=orYRFWuK/mJxly4tDaj7MQeuVRFdJuDfauNoXcKueV3hOuszREGD0uW72ZPP9LPSX 3PWNFwY2IHX18OTWyITXur7ZHTy458/syR35yioLox0UEVe0rY6NnnY59r1ukgnOsb 7VrlQdsSF45GRv0TwBI7v/4SrCkM4IE2VfJfiNjg= Received: from smtp30.i.mail.ru (smtp30.i.mail.ru [95.163.41.71]) (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 ECCB8772EA1 for ; Thu, 4 Apr 2024 19:18:11 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org ECCB8772EA1 Received: by smtp30.i.mail.ru with esmtpa (envelope-from ) id 1rsPn0-00000004DNZ-1afN; Thu, 04 Apr 2024 19:18:10 +0300 To: Maxim Kokryashkin , Sergey Bronnikov Date: Thu, 4 Apr 2024 19:14:11 +0300 Message-ID: <20240404161411.30284-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.44.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD96441E77B1D9F2D07F770E317D3DAD57A9495E77BC8B99A93182A05F538085040EAC83F8AC8F09C01D4FF92D56319F1970075C88ED00DE5A7ED1091800F09774647D41910D6C19DC5 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7195F30236A8D43B4EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637FB2D77E6174520AE8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D82389EF840892EB88112C53E46C0EBAB12EF7F485A872444DCC7F00164DA146DAFE8445B8C89999728AA50765F790063773DCDF0198120BE8389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC878444BBB7636F62AF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C2D01283D1ACF37BA9735652A29929C6C4AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C31FD302B0BF2DAEE8BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF17B107DEF921CE791DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C3CE9959E2676FD87735872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A5BCA5AC8A84D5C7A45002B1117B3ED696F0038F782FC073B369995D676B7B4CBE823CB91A9FED034534781492E4B8EEAD5973B86847D985D2C79554A2A72441328621D336A7BC284946AD531847A6065A17B107DEF921CE79BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34A541DB4061E9383291CAD2F94C3AA1635E41A40B94943771BD4F72B4140D13D4F4FC2DCD531346031D7E09C32AA3244CB13E04007F3841E2AB70F9BE574AE9C6D5A29FCB63E02D86EA455F16B58544A21C197AAF4D2E4732A5AE236DF995FB59829709634694AABAED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXjBDMSkNP8kNIBXPucGZpkv X-Mailru-Sender: 520A125C2F17F0B1A9638AD358559B59ADDE7300517D7D50D4FF92D56319F1970075C88ED00DE5A7B7CBEF92542CD7C88B0A2698F12F5C9EC77752E0C033A69E86920BD37369036789A8C6A0E60D2BB63A5DB60FBEB33A8A0DA7A0AF5A3A8387 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] Correct fix for stack check when recording BC_VARG. 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 Yichun Zhang. (cherry picked from commit b2791179ef96d652d00d78d2a8780af690537f6a) This patch is a follow-up for the commit 5f0a43ace8cfe98c3e8cd313bf809b4405ba395d ("bugfix: fixed assertion failure "lj_record.c:92: rec_check_slots: Assertion `nslots <= 250' failed" found by stressing our edgelang compiler."), which is identical to the commit e0388e6c00866c9ee1c7c9aab8a3ba9e15186b5c ("Fix stack check when recording BC_VARG.)" from the upstream. The error is raised too late, when buffer overflow of `J->slot` has already occurred and data in the `jit_State` structure is corrupted. This patch moves the corresponding check before using the `J->slot` buffer. The `J->maxslot` may overflow the buffer only in cases where the amount of the vararg results is unknown. The check is used only in this case since the trace recording for the undefined-on-trace varargs is not yet implemented for an unknown amount of varargs. 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-noticket-fix-slots-overflow-for-varg-record Issue: https://github.com/tarantool/tarantool/issues/9595 src/lj_record.c | 4 +- ...ix-slots-overflow-for-varg-record.test.lua | 99 +++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 test/tarantool-tests/fix-slots-overflow-for-varg-record.test.lua diff --git a/src/lj_record.c b/src/lj_record.c index 7fa56834..96fe26d8 100644 --- a/src/lj_record.c +++ b/src/lj_record.c @@ -1830,6 +1830,8 @@ static void rec_varg(jit_State *J, BCReg dst, ptrdiff_t nresults) } else if (dst >= J->maxslot) { J->maxslot = dst + 1; } + if (J->baseslot + J->maxslot >= LJ_MAX_JSLOTS) + lj_trace_err(J, LJ_TRERR_STACKOV); for (i = 0; i < nresults; i++) J->base[dst+i] = i < nvararg ? getslot(J, i - nvararg - 1 - LJ_FR2) : TREF_NIL; } else { /* Unknown number of varargs passed to trace. */ @@ -1913,8 +1915,6 @@ static void rec_varg(jit_State *J, BCReg dst, ptrdiff_t nresults) lj_trace_err_info(J, LJ_TRERR_NYIBC); } } - if (J->baseslot + J->maxslot >= LJ_MAX_JSLOTS) - lj_trace_err(J, LJ_TRERR_STACKOV); } /* -- Record allocations -------------------------------------------------- */ diff --git a/test/tarantool-tests/fix-slots-overflow-for-varg-record.test.lua b/test/tarantool-tests/fix-slots-overflow-for-varg-record.test.lua new file mode 100644 index 00000000..b09a722d --- /dev/null +++ b/test/tarantool-tests/fix-slots-overflow-for-varg-record.test.lua @@ -0,0 +1,99 @@ +local tap = require('tap') + +-- Test file to demonstrate `J->slots` buffer overflow when +-- recording the `BC_VARG` bytecode. + +local test = tap.test('fix-slots-overflow-for-varg-record'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), + ['Disabled on *BSD due to #4819'] = jit.os == 'BSD', +}) + +test:plan(1) + +-- Before the patch, the JIT compiler checked the slots overflow +-- for recording of the `BC_VARG` bytecode too late, when the +-- overflow of the `slot` buffer had already occurred. Hence, some +-- fields of the `jit_State` structure (see for +-- details) may be overwritten. Unfortunately, neither UBSAN, nor +-- ASAN, nor Valgrind can catch such misbehaviour for now. So we +-- should observe the results of overwritten fields in the +-- structure. +-- +-- In particular, the content of the `params` array with the JIT +-- engine settings is corrupted. One of the options to be +-- overwritten is the `maxirconst` that overflows, so no trace can +-- be compiled. An attempt to compile any trace will fail. +-- +-- The test fails before the commit on the GC64 build and may lead +-- to the core dump for the non-GC64 build. + +local traceinfo = require('jit.util').traceinfo + +-- XXX: Use a vararg function here to prevent its compilation +-- after failing recording for the main trace. +-- luacheck: no unused +local function empty(...) +end + +local function varg_with_slot_overflow(...) + -- Try to record `BC_VARG`. It should fail due to slots overflow + -- when trying to copy varargs from slots of the incoming + -- parameters to slots of arguments for the call. + empty(...) +end +_G.varg_with_slot_overflow = varg_with_slot_overflow + +-- Prevent the compilation of unrelated traces. +jit.off() +jit.flush() + +-- XXX: Generate the function with the maximum possible (to +-- preserve JIT compilation of the call) arguments given to the +-- vararg function to call. Use sizings for the GC64 mode since it +-- occupies more slots for the frame. +-- Reproduce is still valid for non-GC64 mode since the difference +-- is only several additional slots and buffer overflow is still +-- observed. +local LJ_MAX_JSLOTS = 250 +-- Amount of slots occupied for a call of a vararg function for +-- GC64 mode. +local MAX_VARG_FRAME_SZ = 4 +-- `J->baseslot` at the start of the recording of the call to the +-- vararg function for GC64 mode. +local MAX_BASESLOT = 8 +-- The maximum possible number of slots to occupy is +-- `LJ_MAX_JSLOTS` without: +-- * `J->baseslot` offset at the moment of the call, +-- * 2 vararg frames, +-- * 2 slots for the functions to call. +local chunk = 'varg_with_slot_overflow(1' +for i = 2, LJ_MAX_JSLOTS - MAX_BASESLOT - 2 * MAX_VARG_FRAME_SZ - 2 do + chunk = chunk .. (', %d'):format(i) +end +chunk = chunk .. ')' + +local caller_of_varg = assert(loadstring(chunk)) + +-- Use an additional frame to simplify the `J->baseslot` +-- calculation. +local function wrapper() + for _ = 1, 4 do + caller_of_varg() + end +end + +jit.on() +jit.opt.start('hotloop=1') + +wrapper() + +assert(not traceinfo(1), 'no traces recorded') + +-- The simplest trace to compile. +for _ = 1, 4 do end + +jit.off() + +test:ok(traceinfo(1), 'trace is recorded, so structure is not corrupted') + +test:done(true) -- 2.44.0