Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit] Restore state when recording __concat metamethod throws an error.
@ 2024-08-21  8:23 Sergey Kaplun via Tarantool-patches
  2024-09-13 13:07 ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 1 reply; 5+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-08-21  8:23 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

From: Mike Pall <mike>

Thanks to Sergey Kaplun.

(cherry picked from commit 7421a1b33c7ea46f12bba9700c15b5c90253fee0)

Since neither `rec_cat()` nor `lj_record_ret()` restore the Lua stack,
if the error is raised, it leads either to a crash in `BC_RET` or to the
"unbalanced stack" assertion failure.

This patch protects the `rec_mm_arith()`, which can raise an error. Its
caller returns the negated error code to be rethrown in case of the
caught error.

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

Part of tarantool/tarantool#10199
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1234-err-in-record-concat
Related issues:
* https://github.com/tarantool/tarantool/issues/10199
* https://github.com/LuaJIT/LuaJIT/issues/1234

 src/lj_record.c                               | 26 ++++++++++-
 .../lj-1234-err-in-record-concat.test.lua     | 43 +++++++++++++++++++
 2 files changed, 67 insertions(+), 2 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1234-err-in-record-concat.test.lua

diff --git a/src/lj_record.c b/src/lj_record.c
index 96fe26d8..64da3256 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -940,7 +940,9 @@ void lj_record_ret(jit_State *J, BCReg rbase, ptrdiff_t gotresults)
 	J->L->base = b + baseadj;
 	copyTV(J->L, b-(2<<LJ_FR2), &save);
       }
-      if (tr) {  /* Store final result. */
+      if (tr >= 0xffffff00) {
+	lj_err_throw(J->L, -(int32_t)tr);  /* Propagate errors. */
+      } else if (tr) {  /* Store final result. */
 	BCReg dst = bc_a(*(frame_contpc(frame)-1));
 	J->base[dst] = tr;
 	if (dst >= J->maxslot) {
@@ -1936,12 +1938,27 @@ static TRef rec_tnew(jit_State *J, uint32_t ah)
 
 /* -- Concatenation ------------------------------------------------------- */
 
+typedef struct RecCatDataCP {
+  jit_State *J;
+  RecordIndex *ix;
+} 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)
 {
   TRef *top = &J->base[topslot];
   TValue savetv[5+LJ_FR2];
   BCReg s;
   RecordIndex ix;
+  RecCatDataCP rcd;
+  int errcode;
   lj_assertJ(baseslot < topslot, "bad CAT arg");
   for (s = baseslot; s <= topslot; s++)
     (void)getslot(J, s);  /* Ensure all arguments have a reference. */
@@ -1977,8 +1994,11 @@ static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot)
   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.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. */
 }
 
@@ -2300,6 +2320,8 @@ void lj_record_ins(jit_State *J)
 
   case BC_CAT:
     rc = rec_cat(J, rb, rc);
+    if (rc >= 0xffffff00)
+      lj_err_throw(J->L, -(int32_t)rc);  /* Propagate errors. */
     break;
 
   /* -- Constant and move ops --------------------------------------------- */
diff --git a/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua
new file mode 100644
index 00000000..9abaeba5
--- /dev/null
+++ b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua
@@ -0,0 +1,43 @@
+local tap = require('tap')
+
+-- Test file to demonstrate the crash during the concat recording
+-- if it throws an error.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1234.
+
+local test = tap.test('lj-1234-err-in-record-concat'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(2)
+
+jit.opt.start('hotloop=1')
+
+local __concat = function(v1, v2)
+    return tostring(v1) .. tostring(v2)
+end
+
+-- Need to use metamethod call in the concat recording.
+debug.setmetatable(nil, {
+  __concat = __concat,
+})
+
+local function test_concat_p()
+  local counter = 0
+  while counter < 1 do
+    counter = counter + 1
+    -- The first result is placed on the Lua stack before the
+    -- error is raised. When the error is raised, it is handled by
+    -- the trace recorder, but since neither `rec_cat()` nor
+    -- `lj_record_ret()` restore the Lua stack (before the patch),
+    -- it becomes unbalanced after the instruction recording
+    -- attempt.
+    local _ = {} .. (nil .. nil)
+  end
+end
+
+local result, errmsg = pcall(test_concat_p)
+
+test:ok(not result, 'the error is raised')
+test:like(errmsg, 'attempt to concatenate a table value', 'correct error')
+
+test:done(true)
-- 
2.45.2


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-09-17  6:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-21  8:23 [Tarantool-patches] [PATCH luajit] Restore state when recording __concat metamethod throws an error Sergey Kaplun via Tarantool-patches
2024-09-13 13:07 ` Sergey Bronnikov via Tarantool-patches
2024-09-13 15:27   ` Sergey Kaplun via Tarantool-patches
2024-09-13 15:48     ` Sergey Kaplun via Tarantool-patches
2024-09-17  6:33       ` Sergey Bronnikov via Tarantool-patches

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