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 96C5C6EC40; Wed, 7 Jul 2021 17:37:19 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 96C5C6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1625668639; bh=F41gfuLF6uYdz0OYcXwe2n+FsTfRwAekKIJs2xVJJG0=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=QgAtiFsyOk1I2qaTuSX6siutbVb0CBqrHKYncdjXaGC18gV38zV5wwMfmBaI3r1kR hm7MjPkrPlmk8ci5RSfhq89vVeszZrZC2RypOFDIVH5XiilyEbrsUnfpYzXd5tli4G Hpo99Pd3SNn5v4yL9BDVLmkgTjCAKA3I68kCy2CE= Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id F39346EC40 for ; Wed, 7 Jul 2021 17:37:16 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F39346EC40 Received: by smtp33.i.mail.ru with esmtpa (envelope-from ) id 1m18fr-0003JD-RV; Wed, 07 Jul 2021 17:37:16 +0300 To: Igor Munkin , Sergey Ostanevich Date: Wed, 7 Jul 2021 17:36:06 +0300 Message-Id: <20210707143606.3499-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.31.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD954DFF1DC42D673FB96E19CC2B9345E2B1F8975EC27617E56182A05F538085040CA1895B7F363A624B68E1FB1368317D45F715ABDA0182AABA1943288EA34F9BA X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE74260010D6E447003EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063707C4856229E8E7E48638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D81E4F5B1BD8F1F95789F0E876308A4A9F117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC05EF1B56D39DD5F7389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8D23BF7408B3F9022F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BB07C9E286C61B7F975ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975C44FE9905A23AFB60E88DAA7F7D651454289228A744609FD49C2B6934AE262D3EE7EAB7254005DCEDA0EE70D6C6970CA79510FB958DCE06DB6ED91DBE5ABE359AA347268583507A9B7D06A436E56C8DB493EDB24507CE13387DFF0A840B692CF8 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D342E8FEDBD32DECB6DE5566F457DB92DDCE9157C2220279D312FBCF231B3C1C153B5C5390BFC348E361D7E09C32AA3244C84C24606695C5AE94C0A42093E85C57EF2F5F14F68F1805B927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojwWhFrYo6Pn15I3iyiOT8ow== X-Mailru-Sender: 3B9A0136629DC91206CBC582EFEF4CB41BD795B15F39B0EB130B8379C3B473499671DB1F4478FE73F2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A89437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] ARM64: Fix write barrier in BC_USETS. 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 Contributed by Javier Guerra Giraldez. (cherry picked from commit c785131ca5a6d24adc519e5e0bf1b69b671d912f) Closed upvalues are never gray. So after closed upvalue is marked, it is marked as black. Black objects can't refer white objects, so for storing a white value in closed upvalue, we need to move the barrier forward and color our value to gray by using `lj_gc_barrieruv()`. This function can't be called on closed upvalues with non-white values (at least there is no need to mark it again). USETS bytecode for arm64 architecture has the incorrect instruction to check that upvalue is closed: | ccmp TMP0w, #0, #0, ne | beq <1 // branch out from barrier movement `TMP0w` contains `upvalue->closed` field. If it equals NULL (the first `#0`). The second zero is the value of NZCV condition flags set if the condition (`ne`) is FALSE [1][2]. If the set value is not white, then flags are set to zero and branch is not taken (no Zero flag). If it happens at propagate or atomic GC State and the `lj_gc_barrieruv()` function is called then the gray value to set is marked as white. That leads to the assertion failure in the `gc_mark()` function. This patch changes yielded NZCV condition flag to 4 (Zero flag is up) to take the correct branch after `ccmp` instruction. Sergey Kaplun: * added the description and the test for the problem [1]: https://developer.arm.com/documentation/dui0801/g/pge1427897656225 [2]: https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/condition-codes-1-condition-flags-and-codes --- LuaJIT branch: https://github.com/tarantool/luajit/tree/skaplun/lj-426-incorrect-check-closed-uv Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-426-incorrect-check-closed-uv Assertion failure [1] is not related to the patch (I've reproduced it on master branch). Looks like another one GC64 issue. How to reproduce: 1) Run the following command from the Tarantool repo on Odroid: | $ i=0; while [[ $? == 0 ]]; do i=$(($i+1)); echo $i; make LuaJIT-tests; done 2) Wait (need 4-15 iterations). [1]: https://github.com/tarantool/tarantool/runs/3009273464#step:4:4013 Side note: Thanks to the Lord, that there is no #0 issue and it is not mentioned that way... src/vm_arm64.dasc | 2 +- ...6-arm64-incorrect-check-closed-uv.test.lua | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/tarantool-tests/lj-426-arm64-incorrect-check-closed-uv.test.lua diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc index 6e298255..ddcf0f11 100644 --- a/src/vm_arm64.dasc +++ b/src/vm_arm64.dasc @@ -2781,7 +2781,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) |2: // Check if string is white and ensure upvalue is closed. | ldrb TMP0w, UPVAL:CARG1->closed | tst TMP1w, #LJ_GC_WHITES // iswhite(str) - | ccmp TMP0w, #0, #0, ne + | ccmp TMP0w, #0, #4, ne | beq <1 | // Crossed a write barrier. Move the barrier forward. | mov CARG1, GL diff --git a/test/tarantool-tests/lj-426-arm64-incorrect-check-closed-uv.test.lua b/test/tarantool-tests/lj-426-arm64-incorrect-check-closed-uv.test.lua new file mode 100644 index 00000000..b757133f --- /dev/null +++ b/test/tarantool-tests/lj-426-arm64-incorrect-check-closed-uv.test.lua @@ -0,0 +1,38 @@ +local tap = require('tap') + +local test = tap.test('lj-426-arm64-incorrect-check-closed-uv') +test:plan(1) + +-- Test file to demonstrate LuaJIT USETS bytecode incorrect +-- behaviour on arm64 in case when non-white object is set to +-- closed upvalue. +-- See also, https://github.com/LuaJIT/LuaJIT/issues/426. + +-- First, create a closed upvalue. +do + local uv -- luacheck: no unused + -- The function's prototype is created with the following + -- constants at chunk parsing. After adding this constant to + -- the function's prototype it will be marked as gray during + -- propogate phase. + local function usets() uv = '' end + _G.usets = usets +end + +-- Set GC state to GCpause. +collectgarbage() +-- Do GC step as often as possible. +collectgarbage('setstepmul', 100) + +-- We don't know on what exactly step our upvalue is marked as +-- black and USETS become dangerous, so just check it at each +-- step. +-- Don't need to do the full GC cycle step by step. +local old_steps_atomic = misc.getmetrics().gc_steps_atomic +while (misc.getmetrics().gc_steps_atomic == old_steps_atomic) do + collectgarbage('step') + usets() -- luacheck: no global +end + +test:ok(true) +os.exit(test:check() and 0 or 1) -- 2.31.0