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 2A299128A8CC; Tue, 11 Mar 2025 20:52:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2A299128A8CC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1741715554; bh=jvB+J3LNwohwGokzZT4PAWPy7kAFlMW8q4AuAuHZ0gc=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=lh3iUPgZvHRbqkqV1HEHdUzUpWpGhOZnE64SCtH9sMN+wTruCnDwSLwCr7TAYbOsR PgXVO1I0a9KHmuutUG9nIThdfxd7MXyr/7lNNf1M61WHrJzYpcWjkA3V4nmY9gXTEh mabnnKOncV19kZp007z+8l6GegO/7iLhHGCDCuFE= Received: from send149.i.mail.ru (send149.i.mail.ru [89.221.237.244]) (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 556354A0A20 for ; Tue, 11 Mar 2025 20:52:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 556354A0A20 Received: by exim-smtp-69cc44787d-kw6nd with esmtpa (envelope-from ) id 1ts3mJ-000000001rT-0bgi; Tue, 11 Mar 2025 20:52:31 +0300 To: Sergey Bronnikov Date: Tue, 11 Mar 2025 20:52:26 +0300 Message-ID: <20250311175226.7707-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.48.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD96E8ECD36702C4DEF0CD6703A3A4BAF3440E306C4386145C5182A05F5380850404EB017A2F17E1F7A3DE06ABAFEAF6705E0FA91AF98B2F8F19BA39E10C1DA4E27FF996861A82B3D22 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE752E71F0C64B7C834EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB55337566A1031DB47C569B6095393EFCBA1F1F3731DB1C53A3DA8C5FEA7D737C5402F8B4389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C058C1844A7A85E7B68941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6042F1592492B88C6CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE75A9E79F66F1C28F3731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A509ABF4F1D3EA81995002B1117B3ED696676787D4C17F326CB2920F75BA9A967F823CB91A9FED034534781492E4B8EEAD39B22F2BA948E1C8C79554A2A72441328621D336A7BC284946AD531847A6065A535571D14F44ED41 X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34738A88CA0C70A55872656589C0238EF7B8B3CACC826880BAD70EFA31408A749ACEE3F1B4E1C271701D7E09C32AA3244CAF9E1968D4A4E5CD77DD89D51EBB774210C01A1BB09A8AF0EA455F16B58544A2557BDE0DD54B3590A5AE236DF995FB59829709634694AABAED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVYrk7BQKFwEtPI/aAyG0Cr4+LBshncmLbg== X-DA7885C5: A16DF2A1458739ADF255D290C0D534F9A49A4E137D0287E2B51CB48C9A7C8C7F4D938924B8C89E885B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F739381B31377CF4CA21934100DB0A410C5D5AE1159948440FB9AA844701626710C36E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] Avoid unpatching bytecode twice after a trace flush. 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 85c3f2fb6f59276ebf07312859a69d6d5a897f62) When flushing the already flushed trace, it is possible that another trace is recorded for the same bytecode as the first trace. In that case, the assertion fails in `trace_unpatch()`. This patch fixes it by unpatching the trace only in the case if it persists in the trace chain. Also, it deletes the dead code for the trace unpatching logic, since the root trace can't start from `BC_JMP` and the child traces are handled differently. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11055 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1345-flushing-trace-twice Note: CI is red due to problems with the integration testing. See also: https://github.com/tarantool/tarantool/pull/11220 Related issues: * https://github.com/tarantool/tarantool/issues/11055 * https://github.com/LuaJIT/LuaJIT/issues/1345 src/lj_trace.c | 15 +++------ .../lj-1345-flushing-trace-twice.test.lua | 31 +++++++++++++++++++ 2 files changed, 35 insertions(+), 11 deletions(-) create mode 100644 test/tarantool-tests/lj-1345-flushing-trace-twice.test.lua diff --git a/src/lj_trace.c b/src/lj_trace.c index 6b97cc13..f9b8ff00 100644 --- a/src/lj_trace.c +++ b/src/lj_trace.c @@ -235,14 +235,6 @@ static void trace_unpatch(jit_State *J, GCtrace *T) "bad original bytecode %d", op); *pc = T->startins; break; - case BC_JMP: - lj_assertJ(op == BC_ITERL, "bad original bytecode %d", op); - pc += bc_j(*pc)+2; - if (bc_op(*pc) == BC_JITERL) { - lj_assertJ(traceref(J, bc_d(*pc)) == T, "JITERL references other trace"); - *pc = T->startins; - } - break; case BC_JFUNCF: lj_assertJ(op == BC_FUNCF, "bad original bytecode %d", op); *pc = T->startins; @@ -258,18 +250,19 @@ static void trace_flushroot(jit_State *J, GCtrace *T) GCproto *pt = &gcref(T->startpt)->pt; lj_assertJ(T->root == 0, "not a root trace"); lj_assertJ(pt != NULL, "trace has no prototype"); - /* First unpatch any modified bytecode. */ - trace_unpatch(J, T); /* Unlink root trace from chain anchored in prototype. */ if (pt->trace == T->traceno) { /* Trace is first in chain. Easy. */ pt->trace = T->nextroot; +unpatch: + /* Unpatch modified bytecode only if the trace has not been flushed. */ + trace_unpatch(J, T); } else if (pt->trace) { /* Otherwise search in chain of root traces. */ GCtrace *T2 = traceref(J, pt->trace); if (T2) { for (; T2->nextroot; T2 = traceref(J, T2->nextroot)) if (T2->nextroot == T->traceno) { T2->nextroot = T->nextroot; /* Unlink from chain. */ - break; + goto unpatch; } } } diff --git a/test/tarantool-tests/lj-1345-flushing-trace-twice.test.lua b/test/tarantool-tests/lj-1345-flushing-trace-twice.test.lua new file mode 100644 index 00000000..d5345227 --- /dev/null +++ b/test/tarantool-tests/lj-1345-flushing-trace-twice.test.lua @@ -0,0 +1,31 @@ +local tap = require('tap') + +-- Test file to demonstrate incorrect behaviour when LuaJIT +-- flushes the trace twice when another trace for the same +-- starting bytecode was recorded. +-- See also https://github.com/LuaJIT/LuaJIT/issues/1345. +local test = tap.test('lj-1345-flushing-trace-twice'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +-- Reset JIT. +jit.flush() +collectgarbage() + +jit.opt.start('hotloop=1') + +for _ = 1, 3 do + -- Nothing to flush on the first iteration. On the second + -- iteration, flushing the trace for the loop below (from the + -- first iteration). On the third iteration, another trace (from + -- the second iteration) is recorded for that loop. + -- This leads to the assertion failure before this patch. + jit.flush(1) + -- Record the loop with a trace. + for _ = 1, 4 do end +end + +test:ok(true, 'no assertion failure during trace flushing') +test:done(true) -- 2.48.1