Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Bronnikov <sergeyb@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH luajit] ARM64: Fix IR_SLOAD assembly.
Date: Wed, 14 May 2025 14:56:56 +0300	[thread overview]
Message-ID: <20250514115656.13243-1-skaplun@tarantool.org> (raw)

From: Mike Pall <mike>

Reported by Gate88.

(cherry picked from commit 6c4826f12c4d33b8b978004bc681eb1eef2be977)

The issue is in the case when IR SLOAD is unused on a trace, persists
only for typecheck, and has the `num` type. In this case, the `dest`
register is `RID_NONE`. Hence, the `fmov` instruction is emitted
unconditionally, where the destination register is `d0` (`RID_NONE &
31`). So, the value of this register is spoiled. If it holds any value
evaluated before and used after this SLOAD, it leads to incorrect
behaviour.

This patch adds the check that the register is in use before emitting
the instruction.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#11278
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-903-arm64-unused-number-sload-typecheck
Related issues:
* https://github.com/LuaJIT/LuaJIT/issues/903
* https://github.com/tarantool/tarantool/issues/11278

 src/lj_asm_arm64.h                            |  2 +-
 ...m64-unused-number-sload-typecheck.test.lua | 45 +++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-903-arm64-unused-number-sload-typecheck.test.lua

diff --git a/src/lj_asm_arm64.h b/src/lj_asm_arm64.h
index 554bb60a..9b27473c 100644
--- a/src/lj_asm_arm64.h
+++ b/src/lj_asm_arm64.h
@@ -1177,7 +1177,7 @@ dotypecheck:
       tmp = ra_scratch(as, allow);
       rset_clear(allow, tmp);
     }
-    if (irt_isnum(t) && !(ir->op2 & IRSLOAD_CONVERT))
+    if (ra_hasreg(dest) && irt_isnum(t) && !(ir->op2 & IRSLOAD_CONVERT))
       emit_dn(as, A64I_FMOV_D_R, (dest & 31), tmp);
     /* Need type check, even if the load result is unused. */
     asm_guardcc(as, irt_isnum(t) ? CC_LS : CC_NE);
diff --git a/test/tarantool-tests/lj-903-arm64-unused-number-sload-typecheck.test.lua b/test/tarantool-tests/lj-903-arm64-unused-number-sload-typecheck.test.lua
new file mode 100644
index 00000000..748b88e2
--- /dev/null
+++ b/test/tarantool-tests/lj-903-arm64-unused-number-sload-typecheck.test.lua
@@ -0,0 +1,45 @@
+local tap = require('tap')
+-- Test file to demonstrate the incorrect JIT assembling of unused
+-- `IR_SLOAD` with number type on arm64.
+-- See also https://github.com/LuaJIT/LuaJIT/issue/903.
+local test = tap.test('lj-903-arm64-unused-number-sload-typecheck'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+-- Just use any different numbers (but not integers to avoid
+-- integer IR type).
+local SLOT = 0.1
+local MARKER_VALUE = 4.2
+-- XXX: Special mapping to avoid folding and removing always true
+-- comparison.
+local anchor = {marker = MARKER_VALUE}
+
+-- Special function to inline on trace to generate SLOAD
+-- typecheck.
+local function sload_unused(x)
+  return x
+end
+
+-- The additional wrapper to use stackslots in the function.
+local function test_sload()
+  local sload = SLOT
+  for _ = 1, 4 do
+    -- This line should use the `d0` register.
+    local marker = anchor.marker - MARKER_VALUE
+    -- This generates unused IR_SLOAD with typecheck (number).
+    -- Before the patch, it occasionally overwrites the `d0`
+    -- register and causes the execution of the branch.
+    sload_unused(sload)
+    if marker ~= 0 then
+      return false
+    end
+  end
+  return true
+end
+
+jit.opt.start('hotloop=1')
+test:ok(test_sload(), 'correct SLOAD assembling')
+
+test:done(true)
-- 
2.49.0


             reply	other threads:[~2025-05-14 11:56 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-14 11:56 Sergey Kaplun via Tarantool-patches [this message]
2025-05-16 17:07 ` Sergey Bronnikov via Tarantool-patches
2025-05-20 12:40   ` Sergey Kaplun via Tarantool-patches
2025-05-20 14:01     ` Sergey Bronnikov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250514115656.13243-1-skaplun@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit] ARM64: Fix IR_SLOAD assembly.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox