[Tarantool-patches] [PATCH luajit] Prevent compile of __concat with tailcall to fast function.

[Tarantool-patches] [PATCH luajit] Prevent compile of __concat with tailcall to fast function.

[Maksim Kokryashkin via Tarantool-patches]

From: Mike Pall <mike>

E.g. __concat = function() return setmetatable(...) end
Reported by Fezile Manana.

(cherry-picked from commit 75ee3a6159f1831fa57992df3caf5a2c303c281a)

During the recording of concat with tailcall to fast function,
the fast function recording is postponed. This implementation
may lead to the absence of side effects from fast-function and
incorrect behavior as a result. For a comprehensive example,
see the comment in the test.

Maxim Kokryashkin:
* added the description and the test for the problem

Part of tarantool/tarantool#9145
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/lj-690-concat-tail-call
PR: https://github.com/tarantool/tarantool/pull/9304
Issue: https://github.com/LuaJIT/LuaJIT/issues/690

 src/lj_record.c                               |  3 ++
 .../lj-690-concat-tail-call.test.lua          | 34 +++++++++++++++++++
 2 files changed, 37 insertions(+)
 create mode 100644 test/tarantool-tests/lj-690-concat-tail-call.test.lua

diff --git a/src/lj_record.c b/src/lj_record.c
index 48a5481b..3189a7c3 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -918,6 +918,9 @@ void lj_record_ret(jit_State *J, BCReg rbase, ptrdiff_t gotresults)
       TRef tr = gotresults ? J->base[cbase+rbase] : TREF_NIL;
       if (bslot != J->maxslot) {  /* Concatenate the remainder. */
 	TValue *b = J->L->base, save;  /* Simulate lower frame and result. */
+	/* Can't handle MM_concat + CALLT + fast func side-effects. */
+	if (J->postproc != LJ_POST_NONE)
+	  lj_trace_err(J, LJ_TRERR_NYIRETL);
 	J->base[J->maxslot] = tr;
 	copyTV(J->L, &save, b-(2<<LJ_FR2));
 	if (gotresults)
diff --git a/test/tarantool-tests/lj-690-concat-tail-call.test.lua b/test/tarantool-tests/lj-690-concat-tail-call.test.lua
new file mode 100644
index 00000000..20775417
--- /dev/null
+++ b/test/tarantool-tests/lj-690-concat-tail-call.test.lua
@@ -0,0 +1,34 @@
+local tap = require('tap')
+local test = tap.test('lj-690-concat-tail-call'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+-- XXX: Test execution results in an `unbalanced stack after
+-- hot instruction` assertion fail, only if the LuaJIT is built
+-- with assertion support. Otherwise, the behavior is undefined
+-- (it usually fails with a bus error instead).
+
+-- The 'setmetatable' tailcall is delayed, but the non-trivial
+-- MM_concat continuation is not delayed, and recording fails
+-- since there is no metatable with a defined concat for one of
+-- the arguments. During the continuation processing, the stack
+-- delta is altered and fixed up after the continuation frame
+-- recording. Since continuation frame recording fails, the
+-- stack delta is not restored, and the stack becomes unbalanced.
+
+local t = {}
+t.__concat = function ()
+  return setmetatable({}, t)
+end
+local a = setmetatable({}, t)
+
+jit.opt.start('hotloop=1')
+for _ = 1, 4 do
+  -- XXX: Extra concat is needed for a non-trivial continuation.
+  local _ =  '' .. '' .. a
+end
+
+test:ok(true, 'stack is balanced')
+test:done(true)
--
2.39.3 (Apple Git-145)

Re: [Tarantool-patches] [PATCH luajit] Prevent compile of __concat with tailcall to fast function.

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the patch!
LGTM!

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Prevent compile of __concat with tailcall to fast function.

[Sergey Bronnikov via Tarantool-patches]

Hi, Max!

Thanks for the patch! LGTM

see my minor comments below

On 10/25/23 14:40, Maksim Kokryashkin wrote:
> From: Mike Pall <mike>
<snipped>
> diff --git a/test/tarantool-tests/lj-690-concat-tail-call.test.lua b/test/tarantool-tests/lj-690-concat-tail-call.test.lua
> new file mode 100644
> index 00000000..20775417
> --- /dev/null
> +++ b/test/tarantool-tests/lj-690-concat-tail-call.test.lua
> @@ -0,0 +1,34 @@
> +local tap = require('tap')
> +local test = tap.test('lj-690-concat-tail-call'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(1)
> +
> +-- XXX: Test execution results in an `unbalanced stack after
> +-- hot instruction` assertion fail, only if the LuaJIT is built
> +-- with assertion support. Otherwise, the behavior is undefined
> +-- (it usually fails with a bus error instead).
> +
> +-- The 'setmetatable' tailcall is delayed, but the non-trivial
> +-- MM_concat continuation is not delayed, and recording fails
> +-- since there is no metatable with a defined concat for one of
> +-- the arguments. During the continuation processing, the stack
> +-- delta is altered and fixed up after the continuation frame
> +-- recording. Since continuation frame recording fails, the
> +-- stack delta is not restored, and the stack becomes unbalanced.
> +
> +local t = {}
> +t.__concat = function ()

excess whitespace after "function"


> +  return setmetatable({}, t)
> +end
> +local a = setmetatable({}, t)
> +
> +jit.opt.start('hotloop=1')
> +for _ = 1, 4 do
> +  -- XXX: Extra concat is needed for a non-trivial continuation.
> +  local _ =  '' .. '' .. a
> +end
> +
> +test:ok(true, 'stack is balanced')
> +test:done(true)
> --
> 2.39.3 (Apple Git-145)
>

Re: [Tarantool-patches] [PATCH luajit] Prevent compile of __concat with tailcall to fast function.

[Igor Munkin via Tarantool-patches]

Max,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/2.11 and
release/2.10.

On 25.10.23, Maksim Kokryashkin via Tarantool-patches wrote:
> From: Mike Pall <mike>
> 
> E.g. __concat = function() return setmetatable(...) end
> Reported by Fezile Manana.
> 
> (cherry-picked from commit 75ee3a6159f1831fa57992df3caf5a2c303c281a)
> 
> During the recording of concat with tailcall to fast function,
> the fast function recording is postponed. This implementation
> may lead to the absence of side effects from fast-function and
> incorrect behavior as a result. For a comprehensive example,
> see the comment in the test.
> 
> Maxim Kokryashkin:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#9145
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/lj-690-concat-tail-call
> PR: https://github.com/tarantool/tarantool/pull/9304
> Issue: https://github.com/LuaJIT/LuaJIT/issues/690
> 
>  src/lj_record.c                               |  3 ++
>  .../lj-690-concat-tail-call.test.lua          | 34 +++++++++++++++++++
>  2 files changed, 37 insertions(+)
>  create mode 100644 test/tarantool-tests/lj-690-concat-tail-call.test.lua
> 

<snipped>

> --
> 2.39.3 (Apple Git-145)
> 

-- 
Best regards,
IM