[Tarantool-patches] [PATCH luajit 0/4] Fix LuaJIT tests on aarch64, odroid

[Tarantool-patches] [PATCH luajit 0/4] Fix LuaJIT tests on aarch64, odroid

[Sergey Kaplun via Tarantool-patches]

This patchset fixes failing tests on odroid, aarch64.

Branch:
https://github.com/tarantool/luajit/tree/skaplun/gh-6084-missed-carg1-in-bctsetr-fallback

Test branch:
https://github.com/tarantool/tarantool/tree/skaplun/gh-6084-missed-carg1-in-bctsetr-fallback

Issues:
* https://github.com/tarantool/tarantool/issues/5629
* https://github.com/tarantool/tarantool/issues/6084
* https://github.com/tarantool/tarantool/issues/6093

Honestly, I've failed to find a corresponding issue related to the first
patch neither in the issue tracker, nor in the LuaJIT ML, so I've only
referenced the original commit in the #6084.

For last two patches I don't add a new test case, because it is
literally the same as the test in lua-Harness suite <301-basic.t:832>.

Mike Pall (3):
  ARM, ARM64, PPC: Fix TSETR fallback.
  ARM64: Fix xpcall() error case.
  ARM64: Fix xpcall() error case (really).

Sergey Kaplun (1):
  test: add skipcond on architectures for memprof

 src/vm_arm.dasc                               |  1 +
 src/vm_arm64.dasc                             |  6 +++--
 src/vm_ppc.dasc                               |  1 +
 test/tarantool-tests/CMakeLists.txt           |  9 ++++---
 ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
 .../misclib-memprof-lapi.test.lua             |  6 +++++
 test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
 7 files changed, 64 insertions(+), 6 deletions(-)
 create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua

-- 
2.31.0

[Tarantool-patches] [PATCH luajit 1/4] ARM, ARM64, PPC: Fix TSETR fallback.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Thanks to Javier Guerra Giraldez.

(cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)

This patch fixes the issue introduced by commits
f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
infrastructure and initial port of interpreter.') for arm64 and
73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
builtins.') for arm and ppc. Within the mentioned commits the new
bytecode TSETR is introduced for the corresponding architectures.

When the new index of the table processed during this bytecode is the
integer, that is greater than asize of the table, the VM fallbacks to
vmeta_tsetr, for calling
lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
CARG1 is not set by the VM and contains an invalid value, so the
mentioned call leads to crash.
This patch adds the missed set of CARG1 to the right value.

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

Resolves tarantool/tarantool#6084
Part of tarantool/tarantool#5629
---
 src/vm_arm.dasc                               |  1 +
 src/vm_arm64.dasc                             |  1 +
 src/vm_ppc.dasc                               |  1 +
 test/tarantool-tests/CMakeLists.txt           |  9 ++++---
 ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
 test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
 6 files changed, 55 insertions(+), 4 deletions(-)
 create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua

diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
index ae2efdfd..21f7fecb 100644
--- a/src/vm_arm.dasc
+++ b/src/vm_arm.dasc
@@ -701,6 +701,7 @@ static void build_subroutines(BuildCtx *ctx)
   |->vmeta_tsetr:
   |  str BASE, L->base
   |  .IOS mov RC, BASE
+  |  mov CARG1, L
   |  str PC, SAVE_PC
   |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
   |  // Returns TValue *.
diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
index f783428f..6bf59509 100644
--- a/src/vm_arm64.dasc
+++ b/src/vm_arm64.dasc
@@ -711,6 +711,7 @@ static void build_subroutines(BuildCtx *ctx)
   |->vmeta_tsetr:
   |  sxtw CARG3, TMP1w
   |  str BASE, L->base
+  |  mov CARG1, L
   |  str PC, SAVE_PC
   |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
   |  // Returns TValue *.
diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc
index 62e9b681..3f48b7ff 100644
--- a/src/vm_ppc.dasc
+++ b/src/vm_ppc.dasc
@@ -995,6 +995,7 @@ static void build_subroutines(BuildCtx *ctx)
   |
   |->vmeta_tsetr:
   |  stp BASE, L->base
+  |  mr CARG1, L
   |  stw PC, SAVE_PC
   |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
   |  // Returns TValue *.
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 475e2e5d..2fdb4d1f 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -61,11 +61,12 @@ add_subdirectory(lj-flush-on-trace)
 add_subdirectory(misclib-getmetrics-capi)
 
 # The part of the memory profiler toolchain is located in tools
-# directory and auxiliary tests-related modules are located in the
-# current directory (but tests are run in the binary directory),
-# so LUA_PATH need to be updated.
+# directory, jit, profiler, and bytecode toolchains are located
+# in src/ directory and auxiliary tests-related modules are
+# located in the current directory (but tests are run in the
+# binary directory), so LUA_PATH need to be updated.
 set(LUA_PATH
-  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua"
+  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua\;${PROJECT_SOURCE_DIR}/src/?.lua"
 )
 set(LUA_TEST_SUFFIX .test.lua)
 set(LUA_TEST_FLAGS --failures --shuffle)
diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
new file mode 100644
index 00000000..26344274
--- /dev/null
+++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
@@ -0,0 +1,25 @@
+local tap = require("tap")
+local utils = require("utils")
+
+local test = tap.test("gh-6084-missed-carg1-in-bctsetr-fallback")
+test:plan(1)
+
+-- Bytecode TSETR appears only in built-ins libraries, when doing
+-- fixups for fast function written in Lua (i.e. `table.move()`),
+-- by replacing all TSETV bytecodes with the TSETR.
+-- See <src/host/genlibbc.lua> for more details.
+
+-- This test checks that fallback path, when the index of the new
+-- set element is greater than the table's asize, doesn't lead
+-- to a crash.
+
+-- We need to make sure the bytecode is present in the chosen
+-- built-in to make sure our test is still valid.
+assert(utils.hasbc(table.move, "TSETR"))
+
+-- Empty table has asize equals 0. Just copy its element (equals
+-- nil) to the field by index 1 > 0, to fallback inside TSETR.
+table.move({}, 1, 1, 1)
+
+test:ok(true)
+os.exit(test:check() and 0 or 1)
diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index c0403cf1..61d4de7a 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -2,11 +2,14 @@ local M = {}
 
 local ffi = require('ffi')
 local tap = require('tap')
+local bc = require('jit.bc')
 
 ffi.cdef([[
   int setenv(const char *name, const char *value, int overwrite);
 ]])
 
+local function noop() end
+
 local function luacmd(args)
   -- arg[-1] is guaranteed to be not nil.
   local idx = -2
@@ -89,4 +92,23 @@ function M.tweakenv(condition, variable)
   ffi.C.setenv(variable, testvar, 0)
 end
 
+function M.hasbc(f, bytecode)
+  assert(type(f) == 'function', 'argument #1 should be a function')
+  assert(type(bytecode) == 'string', 'argument #2 should be a string')
+  local hasbc = false
+  -- Check the bytecode entry line by line.
+  local out = {
+    write = function(out, line)
+      if line:match(bytecode) then
+        hasbc = true
+        out.write = noop
+      end
+    end,
+    flush = noop,
+    close = noop,
+  }
+  bc.dump(f, out)
+  return hasbc
+end
+
 return M
-- 
2.31.0

Re: [Tarantool-patches] [PATCH luajit 1/4] ARM, ARM64, PPC: Fix TSETR fallback.

[Sergey Ostanevich via Tarantool-patches]

Hi!
Thanks for the patch!

See my 3 cents below.

Sergos


> On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> From: Mike Pall <mike>
> 
> Thanks to Javier Guerra Giraldez.
> 
> (cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)
> 
> This patch fixes the issue introduced by commits
> f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
> infrastructure and initial port of interpreter.') for arm64 and
> 73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
> builtins.') for arm and ppc. Within the mentioned commits the new
> bytecode TSETR is introduced for the corresponding architectures.
> 
> When the new index of the table processed during this bytecode is the
> integer, that is greater than asize of the table, the VM fallbacks to
> vmeta_tsetr, for calling
> lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
> CARG1 is not set by the VM and contains an invalid value, so the
> mentioned call leads to crash.
> This patch adds the missed set of CARG1 to the right value.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Resolves tarantool/tarantool#6084
> Part of tarantool/tarantool#5629
> ---
> src/vm_arm.dasc                               |  1 +
> src/vm_arm64.dasc                             |  1 +
> src/vm_ppc.dasc                               |  1 +
> test/tarantool-tests/CMakeLists.txt           |  9 ++++---
> ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
> test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
> 6 files changed, 55 insertions(+), 4 deletions(-)
> create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> 
> diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
> index ae2efdfd..21f7fecb 100644
> --- a/src/vm_arm.dasc
> +++ b/src/vm_arm.dasc
> @@ -701,6 +701,7 @@ static void build_subroutines(BuildCtx *ctx)
>   |->vmeta_tsetr:
>   |  str BASE, L->base
>   |  .IOS mov RC, BASE
> +  |  mov CARG1, L
>   |  str PC, SAVE_PC
>   |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
>   |  // Returns TValue *.
> diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
> index f783428f..6bf59509 100644
> --- a/src/vm_arm64.dasc
> +++ b/src/vm_arm64.dasc
> @@ -711,6 +711,7 @@ static void build_subroutines(BuildCtx *ctx)
>   |->vmeta_tsetr:
>   |  sxtw CARG3, TMP1w
>   |  str BASE, L->base
> +  |  mov CARG1, L
>   |  str PC, SAVE_PC
>   |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
>   |  // Returns TValue *.
> diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc
> index 62e9b681..3f48b7ff 100644
> --- a/src/vm_ppc.dasc
> +++ b/src/vm_ppc.dasc
> @@ -995,6 +995,7 @@ static void build_subroutines(BuildCtx *ctx)
>   |
>   |->vmeta_tsetr:
>   |  stp BASE, L->base
> +  |  mr CARG1, L
>   |  stw PC, SAVE_PC
>   |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
>   |  // Returns TValue *.
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 475e2e5d..2fdb4d1f 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -61,11 +61,12 @@ add_subdirectory(lj-flush-on-trace)
> add_subdirectory(misclib-getmetrics-capi)
> 
> # The part of the memory profiler toolchain is located in tools
> -# directory and auxiliary tests-related modules are located in the
> -# current directory (but tests are run in the binary directory),
> -# so LUA_PATH need to be updated.
> +# directory, jit, profiler, and bytecode toolchains are located
> +# in src/ directory and auxiliary tests-related modules are
> +# located in the current directory (but tests are run in the
> +# binary directory), so LUA_PATH need to be updated.
> set(LUA_PATH
> -  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua"
> +  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua\;${PROJECT_SOURCE_DIR}/src/?.lua"
> )
> set(LUA_TEST_SUFFIX .test.lua)
> set(LUA_TEST_FLAGS --failures --shuffle)
> diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> new file mode 100644
> index 00000000..26344274
> --- /dev/null
> +++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> @@ -0,0 +1,25 @@
> +local tap = require("tap")
> +local utils = require("utils")

Sorry, but

s-ostanevich:tarantool-tests s.ostanevich$ egrep -l "\<require\>.*\"" *.lua  | wc -l
       6
s-ostanevich:tarantool-tests s.ostanevich$ egrep -l "\<require\>.*\'" *.lua  | wc -l
      14

clearly votes for require(‘tap') against require("tap”)

> +
> +local test = tap.test("gh-6084-missed-carg1-in-bctsetr-fallback")
> +test:plan(1)
> +
> +-- Bytecode TSETR appears only in built-ins libraries, when doing
> +-- fixups for fast function written in Lua (i.e. `table.move()`),
> +-- by replacing all TSETV bytecodes with the TSETR.
> +-- See <src/host/genlibbc.lua> for more details.
> +
> +-- This test checks that fallback path, when the index of the new
> +-- set element is greater than the table's asize, doesn't lead
> +-- to a crash.
> +
> +-- We need to make sure the bytecode is present in the chosen
> +-- built-in to make sure our test is still valid.
> +assert(utils.hasbc(table.move, "TSETR"))
> +
> +-- Empty table has asize equals 0. Just copy its element (equals
> +-- nil) to the field by index 1 > 0, to fallback inside TSETR.
> +table.move({}, 1, 1, 1)

I would like to see the move is correctly performed, rather the fact
there were no crash. It gives a bigger space for unexpected behavior.

> +
> +test:ok(true)
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index c0403cf1..61d4de7a 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua
> @@ -2,11 +2,14 @@ local M = {}
> 
> local ffi = require('ffi')
> local tap = require('tap')
> +local bc = require('jit.bc')
> 
> ffi.cdef([[
>   int setenv(const char *name, const char *value, int overwrite);
> ]])
> 
> +local function noop() end

Name of this one in a patch that messess with bytecodes is confusing. Could it
be a simpler one, like ‘empty’?

> +
> local function luacmd(args)
>   -- arg[-1] is guaranteed to be not nil.
>   local idx = -2
> @@ -89,4 +92,23 @@ function M.tweakenv(condition, variable)
>   ffi.C.setenv(variable, testvar, 0)
> end
> 
> +function M.hasbc(f, bytecode)
> +  assert(type(f) == 'function', 'argument #1 should be a function')
> +  assert(type(bytecode) == 'string', 'argument #2 should be a string')
> +  local hasbc = false
> +  -- Check the bytecode entry line by line.
> +  local out = {
> +    write = function(out, line)
> +      if line:match(bytecode) then
> +        hasbc = true
> +        out.write = noop
> +      end
> +    end,
> +    flush = noop,
> +    close = noop,
> +  }
> +  bc.dump(f, out)
> +  return hasbc
> +end
> +
> return M
> -- 
> 2.31.0
> 

Re: [Tarantool-patches] [PATCH luajit 1/4] ARM, ARM64, PPC: Fix TSETR fallback.

[Sergey Kaplun via Tarantool-patches]

Hi!

Thanks for the review!

On 02.06.21, Sergey Ostanevich wrote:
> Hi!
> Thanks for the patch!
> 
> See my 3 cents below.
> 
> Sergos
> 
> 
> > On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > From: Mike Pall <mike>
> > 
> > Thanks to Javier Guerra Giraldez.
> > 
> > (cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)
> > 
> > This patch fixes the issue introduced by commits
> > f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
> > infrastructure and initial port of interpreter.') for arm64 and
> > 73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
> > builtins.') for arm and ppc. Within the mentioned commits the new
> > bytecode TSETR is introduced for the corresponding architectures.
> > 
> > When the new index of the table processed during this bytecode is the
> > integer, that is greater than asize of the table, the VM fallbacks to
> > vmeta_tsetr, for calling
> > lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
> > CARG1 is not set by the VM and contains an invalid value, so the
> > mentioned call leads to crash.
> > This patch adds the missed set of CARG1 to the right value.
> > 
> > Sergey Kaplun:
> > * added the description and the test for the problem
> > 
> > Resolves tarantool/tarantool#6084
> > Part of tarantool/tarantool#5629
> > ---
> > src/vm_arm.dasc                               |  1 +
> > src/vm_arm64.dasc                             |  1 +
> > src/vm_ppc.dasc                               |  1 +
> > test/tarantool-tests/CMakeLists.txt           |  9 ++++---
> > ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
> > test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
> > 6 files changed, 55 insertions(+), 4 deletions(-)
> > create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> > 
> > diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
> > index ae2efdfd..21f7fecb 100644
> > --- a/src/vm_arm.dasc
> > +++ b/src/vm_arm.dasc
> > @@ -701,6 +701,7 @@ static void build_subroutines(BuildCtx *ctx)
> >   |->vmeta_tsetr:
> >   |  str BASE, L->base
> >   |  .IOS mov RC, BASE
> > +  |  mov CARG1, L
> >   |  str PC, SAVE_PC
> >   |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
> >   |  // Returns TValue *.
> > diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
> > index f783428f..6bf59509 100644
> > --- a/src/vm_arm64.dasc
> > +++ b/src/vm_arm64.dasc
> > @@ -711,6 +711,7 @@ static void build_subroutines(BuildCtx *ctx)
> >   |->vmeta_tsetr:
> >   |  sxtw CARG3, TMP1w
> >   |  str BASE, L->base
> > +  |  mov CARG1, L
> >   |  str PC, SAVE_PC
> >   |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
> >   |  // Returns TValue *.
> > diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc
> > index 62e9b681..3f48b7ff 100644
> > --- a/src/vm_ppc.dasc
> > +++ b/src/vm_ppc.dasc
> > @@ -995,6 +995,7 @@ static void build_subroutines(BuildCtx *ctx)
> >   |
> >   |->vmeta_tsetr:
> >   |  stp BASE, L->base
> > +  |  mr CARG1, L
> >   |  stw PC, SAVE_PC
> >   |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
> >   |  // Returns TValue *.
> > diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> > index 475e2e5d..2fdb4d1f 100644
> > --- a/test/tarantool-tests/CMakeLists.txt
> > +++ b/test/tarantool-tests/CMakeLists.txt
> > @@ -61,11 +61,12 @@ add_subdirectory(lj-flush-on-trace)
> > add_subdirectory(misclib-getmetrics-capi)
> > 
> > # The part of the memory profiler toolchain is located in tools
> > -# directory and auxiliary tests-related modules are located in the
> > -# current directory (but tests are run in the binary directory),
> > -# so LUA_PATH need to be updated.
> > +# directory, jit, profiler, and bytecode toolchains are located
> > +# in src/ directory and auxiliary tests-related modules are
> > +# located in the current directory (but tests are run in the
> > +# binary directory), so LUA_PATH need to be updated.
> > set(LUA_PATH
> > -  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua"
> > +  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua\;${PROJECT_SOURCE_DIR}/src/?.lua"
> > )
> > set(LUA_TEST_SUFFIX .test.lua)
> > set(LUA_TEST_FLAGS --failures --shuffle)
> > diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> > new file mode 100644
> > index 00000000..26344274
> > --- /dev/null
> > +++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> > @@ -0,0 +1,25 @@
> > +local tap = require("tap")
> > +local utils = require("utils")
> 
> Sorry, but
> 
> s-ostanevich:tarantool-tests s.ostanevich$ egrep -l "\<require\>.*\"" *.lua  | wc -l
>        6
> s-ostanevich:tarantool-tests s.ostanevich$ egrep -l "\<require\>.*\'" *.lua  | wc -l
>       14
> 
> clearly votes for require(‘tap') against require("tap”)

I've tried to follow the original code style from src/jit/ directory:

| src$ grep -l -P 'require.*"' */*.lua -r | wc -l
| 17
| src$ grep -l -P "require.*'" */*.lua -r | wc -l
| 0

Also, you count utils.lua 3 times.

But I don't mind :)
Branch is force-pushed.

===================================================================
diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
index 26344274..1a438c82 100644
--- a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
+++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
@@ -1,7 +1,7 @@
-local tap = require("tap")
-local utils = require("utils")
+local tap = require('tap')
+local utils = require('utils')
 
-local test = tap.test("gh-6084-missed-carg1-in-bctsetr-fallback")
+local test = tap.test('gh-6084-missed-carg1-in-bctsetr-fallback')
 test:plan(1)
 
 -- Bytecode TSETR appears only in built-ins libraries, when doing
@@ -15,7 +15,7 @@ test:plan(1)
 
 -- We need to make sure the bytecode is present in the chosen
 -- built-in to make sure our test is still valid.
-assert(utils.hasbc(table.move, "TSETR"))
+assert(utils.hasbc(table.move, 'TSETR'))
 
 -- Empty table has asize equals 0. Just copy its element (equals
 -- nil) to the field by index 1 > 0, to fallback inside TSETR.

===================================================================

Side note: we should document our LuaJIT codestyle...

> 
> > +
> > +local test = tap.test("gh-6084-missed-carg1-in-bctsetr-fallback")
> > +test:plan(1)
> > +
> > +-- Bytecode TSETR appears only in built-ins libraries, when doing
> > +-- fixups for fast function written in Lua (i.e. `table.move()`),
> > +-- by replacing all TSETV bytecodes with the TSETR.
> > +-- See <src/host/genlibbc.lua> for more details.
> > +
> > +-- This test checks that fallback path, when the index of the new
> > +-- set element is greater than the table's asize, doesn't lead
> > +-- to a crash.
> > +
> > +-- We need to make sure the bytecode is present in the chosen
> > +-- built-in to make sure our test is still valid.
> > +assert(utils.hasbc(table.move, "TSETR"))
> > +
> > +-- Empty table has asize equals 0. Just copy its element (equals
> > +-- nil) to the field by index 1 > 0, to fallback inside TSETR.
> > +table.move({}, 1, 1, 1)
> 
> I would like to see the move is correctly performed, rather the fact
> there were no crash. It gives a bigger space for unexpected behavior.

Fixed. Branch is force-pushed.

===================================================================
diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
index 1a438c82..95bf3bd7 100644
--- a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
+++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
@@ -2,7 +2,7 @@ local tap = require('tap')
 local utils = require('utils')
 
 local test = tap.test('gh-6084-missed-carg1-in-bctsetr-fallback')
-test:plan(1)
+test:plan(2)
 
 -- Bytecode TSETR appears only in built-ins libraries, when doing
 -- fixups for fast function written in Lua (i.e. `table.move()`),
@@ -17,9 +17,11 @@ test:plan(1)
 -- built-in to make sure our test is still valid.
 assert(utils.hasbc(table.move, 'TSETR'))
 
--- Empty table has asize equals 0. Just copy its element (equals
--- nil) to the field by index 1 > 0, to fallback inside TSETR.
-table.move({}, 1, 1, 1)
+-- `t` table has asize equals 1. Just copy its first element (1)
+-- to the field by index 2 > 1, to fallback inside TSETR.
+local t = {1}
+local res = table.move(t, 1, 1, 2)
+test:ok(t == res, 'table.move returns the same table')
+test:ok(t[1] == t[2], 'table.move is correct')
 
-test:ok(true)
 os.exit(test:check() and 0 or 1)
===================================================================

> 
> > +
> > +test:ok(true)
> > +os.exit(test:check() and 0 or 1)
> > diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> > index c0403cf1..61d4de7a 100644
> > --- a/test/tarantool-tests/utils.lua
> > +++ b/test/tarantool-tests/utils.lua
> > @@ -2,11 +2,14 @@ local M = {}
> > 
> > local ffi = require('ffi')
> > local tap = require('tap')
> > +local bc = require('jit.bc')
> > 
> > ffi.cdef([[
> >   int setenv(const char *name, const char *value, int overwrite);
> > ]])
> > 
> > +local function noop() end
> 
> Name of this one in a patch that messess with bytecodes is confusing. Could it
> be a simpler one, like ‘empty’?

Fixed. Branch is force-pushed.

===================================================================
diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index 61d4de7a..57932c5d 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -8,7 +8,7 @@ ffi.cdef([[
   int setenv(const char *name, const char *value, int overwrite);
 ]])
 
-local function noop() end
+local function empty() end
 
 local function luacmd(args)
   -- arg[-1] is guaranteed to be not nil.
@@ -101,11 +101,11 @@ function M.hasbc(f, bytecode)
     write = function(out, line)
       if line:match(bytecode) then
         hasbc = true
-        out.write = noop
+        out.write = empty
       end
     end,
-    flush = noop,
-    close = noop,
+    flush = empty,
+    close = empty,
   }
   bc.dump(f, out)
   return hasbc
===================================================================

> 
> > +
> > local function luacmd(args)
> >   -- arg[-1] is guaranteed to be not nil.
> >   local idx = -2
> > @@ -89,4 +92,23 @@ function M.tweakenv(condition, variable)
> >   ffi.C.setenv(variable, testvar, 0)
> > end
> > 
> > +function M.hasbc(f, bytecode)
> > +  assert(type(f) == 'function', 'argument #1 should be a function')
> > +  assert(type(bytecode) == 'string', 'argument #2 should be a string')
> > +  local hasbc = false
> > +  -- Check the bytecode entry line by line.
> > +  local out = {
> > +    write = function(out, line)
> > +      if line:match(bytecode) then
> > +        hasbc = true
> > +        out.write = noop
> > +      end
> > +    end,
> > +    flush = noop,
> > +    close = noop,
> > +  }
> > +  bc.dump(f, out)
> > +  return hasbc
> > +end
> > +
> > return M
> > -- 
> > 2.31.0
> > 
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/4] ARM, ARM64, PPC: Fix TSETR fallback.

[Sergey Ostanevich via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 12278 bytes --]

Hi!

Thanks for the update, LGTM.

As for LuaJIT code style - should it follow the Tarantool Lua one?
https://www.tarantool.io/en/doc/latest/dev_guide/lua_style_guide/ <https://www.tarantool.io/en/doc/latest/dev_guide/lua_style_guide/>

Sergos.

> On 4 Jun 2021, at 16:12, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> Hi!
> 
> Thanks for the review!
> 
> On 02.06.21, Sergey Ostanevich wrote:
>> Hi!
>> Thanks for the patch!
>> 
>> See my 3 cents below.
>> 
>> Sergos
>> 
>> 
>>> On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
>>> 
>>> From: Mike Pall <mike>
>>> 
>>> Thanks to Javier Guerra Giraldez.
>>> 
>>> (cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)
>>> 
>>> This patch fixes the issue introduced by commits
>>> f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
>>> infrastructure and initial port of interpreter.') for arm64 and
>>> 73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
>>> builtins.') for arm and ppc. Within the mentioned commits the new
>>> bytecode TSETR is introduced for the corresponding architectures.
>>> 
>>> When the new index of the table processed during this bytecode is the
>>> integer, that is greater than asize of the table, the VM fallbacks to
>>> vmeta_tsetr, for calling
>>> lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
>>> CARG1 is not set by the VM and contains an invalid value, so the
>>> mentioned call leads to crash.
>>> This patch adds the missed set of CARG1 to the right value.
>>> 
>>> Sergey Kaplun:
>>> * added the description and the test for the problem
>>> 
>>> Resolves tarantool/tarantool#6084
>>> Part of tarantool/tarantool#5629
>>> ---
>>> src/vm_arm.dasc                               |  1 +
>>> src/vm_arm64.dasc                             |  1 +
>>> src/vm_ppc.dasc                               |  1 +
>>> test/tarantool-tests/CMakeLists.txt           |  9 ++++---
>>> ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
>>> test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
>>> 6 files changed, 55 insertions(+), 4 deletions(-)
>>> create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
>>> 
>>> diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
>>> index ae2efdfd..21f7fecb 100644
>>> --- a/src/vm_arm.dasc
>>> +++ b/src/vm_arm.dasc
>>> @@ -701,6 +701,7 @@ static void build_subroutines(BuildCtx *ctx)
>>>  |->vmeta_tsetr:
>>>  |  str BASE, L->base
>>>  |  .IOS mov RC, BASE
>>> +  |  mov CARG1, L
>>>  |  str PC, SAVE_PC
>>>  |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
>>>  |  // Returns TValue *.
>>> diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
>>> index f783428f..6bf59509 100644
>>> --- a/src/vm_arm64.dasc
>>> +++ b/src/vm_arm64.dasc
>>> @@ -711,6 +711,7 @@ static void build_subroutines(BuildCtx *ctx)
>>>  |->vmeta_tsetr:
>>>  |  sxtw CARG3, TMP1w
>>>  |  str BASE, L->base
>>> +  |  mov CARG1, L
>>>  |  str PC, SAVE_PC
>>>  |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
>>>  |  // Returns TValue *.
>>> diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc
>>> index 62e9b681..3f48b7ff 100644
>>> --- a/src/vm_ppc.dasc
>>> +++ b/src/vm_ppc.dasc
>>> @@ -995,6 +995,7 @@ static void build_subroutines(BuildCtx *ctx)
>>>  |
>>>  |->vmeta_tsetr:
>>>  |  stp BASE, L->base
>>> +  |  mr CARG1, L
>>>  |  stw PC, SAVE_PC
>>>  |  bl extern lj_tab_setinth  // (lua_State *L, GCtab *t, int32_t key)
>>>  |  // Returns TValue *.
>>> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
>>> index 475e2e5d..2fdb4d1f 100644
>>> --- a/test/tarantool-tests/CMakeLists.txt
>>> +++ b/test/tarantool-tests/CMakeLists.txt
>>> @@ -61,11 +61,12 @@ add_subdirectory(lj-flush-on-trace)
>>> add_subdirectory(misclib-getmetrics-capi)
>>> 
>>> # The part of the memory profiler toolchain is located in tools
>>> -# directory and auxiliary tests-related modules are located in the
>>> -# current directory (but tests are run in the binary directory),
>>> -# so LUA_PATH need to be updated.
>>> +# directory, jit, profiler, and bytecode toolchains are located
>>> +# in src/ directory and auxiliary tests-related modules are
>>> +# located in the current directory (but tests are run in the
>>> +# binary directory), so LUA_PATH need to be updated.
>>> set(LUA_PATH
>>> -  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua"
>>> +  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua\;${PROJECT_SOURCE_DIR}/src/?.lua"
>>> )
>>> set(LUA_TEST_SUFFIX .test.lua)
>>> set(LUA_TEST_FLAGS --failures --shuffle)
>>> diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
>>> new file mode 100644
>>> index 00000000..26344274
>>> --- /dev/null
>>> +++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
>>> @@ -0,0 +1,25 @@
>>> +local tap = require("tap")
>>> +local utils = require("utils")
>> 
>> Sorry, but
>> 
>> s-ostanevich:tarantool-tests s.ostanevich$ egrep -l "\<require\>.*\"" *.lua  | wc -l
>>       6
>> s-ostanevich:tarantool-tests s.ostanevich$ egrep -l "\<require\>.*\'" *.lua  | wc -l
>>      14
>> 
>> clearly votes for require(‘tap') against require("tap”)
> 
> I've tried to follow the original code style from src/jit/ directory:
> 
> | src$ grep -l -P 'require.*"' */*.lua -r | wc -l
> | 17
> | src$ grep -l -P "require.*'" */*.lua -r | wc -l
> | 0
> 
> Also, you count utils.lua 3 times.
> 
> But I don't mind :)
> Branch is force-pushed.
> 
> ===================================================================
> diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> index 26344274..1a438c82 100644
> --- a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> +++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> @@ -1,7 +1,7 @@
> -local tap = require("tap")
> -local utils = require("utils")
> +local tap = require('tap')
> +local utils = require('utils')
> 
> -local test = tap.test("gh-6084-missed-carg1-in-bctsetr-fallback")
> +local test = tap.test('gh-6084-missed-carg1-in-bctsetr-fallback')
> test:plan(1)
> 
> -- Bytecode TSETR appears only in built-ins libraries, when doing
> @@ -15,7 +15,7 @@ test:plan(1)
> 
> -- We need to make sure the bytecode is present in the chosen
> -- built-in to make sure our test is still valid.
> -assert(utils.hasbc(table.move, "TSETR"))
> +assert(utils.hasbc(table.move, 'TSETR'))
> 
> -- Empty table has asize equals 0. Just copy its element (equals
> -- nil) to the field by index 1 > 0, to fallback inside TSETR.
> 
> ===================================================================
> 
> Side note: we should document our LuaJIT codestyle...
> 
>> 
>>> +
>>> +local test = tap.test("gh-6084-missed-carg1-in-bctsetr-fallback")
>>> +test:plan(1)
>>> +
>>> +-- Bytecode TSETR appears only in built-ins libraries, when doing
>>> +-- fixups for fast function written in Lua (i.e. `table.move()`),
>>> +-- by replacing all TSETV bytecodes with the TSETR.
>>> +-- See <src/host/genlibbc.lua> for more details.
>>> +
>>> +-- This test checks that fallback path, when the index of the new
>>> +-- set element is greater than the table's asize, doesn't lead
>>> +-- to a crash.
>>> +
>>> +-- We need to make sure the bytecode is present in the chosen
>>> +-- built-in to make sure our test is still valid.
>>> +assert(utils.hasbc(table.move, "TSETR"))
>>> +
>>> +-- Empty table has asize equals 0. Just copy its element (equals
>>> +-- nil) to the field by index 1 > 0, to fallback inside TSETR.
>>> +table.move({}, 1, 1, 1)
>> 
>> I would like to see the move is correctly performed, rather the fact
>> there were no crash. It gives a bigger space for unexpected behavior.
> 
> Fixed. Branch is force-pushed.
> 
> ===================================================================
> diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> index 1a438c82..95bf3bd7 100644
> --- a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> +++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> @@ -2,7 +2,7 @@ local tap = require('tap')
> local utils = require('utils')
> 
> local test = tap.test('gh-6084-missed-carg1-in-bctsetr-fallback')
> -test:plan(1)
> +test:plan(2)
> 
> -- Bytecode TSETR appears only in built-ins libraries, when doing
> -- fixups for fast function written in Lua (i.e. `table.move()`),
> @@ -17,9 +17,11 @@ test:plan(1)
> -- built-in to make sure our test is still valid.
> assert(utils.hasbc(table.move, 'TSETR'))
> 
> --- Empty table has asize equals 0. Just copy its element (equals
> --- nil) to the field by index 1 > 0, to fallback inside TSETR.
> -table.move({}, 1, 1, 1)
> +-- `t` table has asize equals 1. Just copy its first element (1)
> +-- to the field by index 2 > 1, to fallback inside TSETR.
> +local t = {1}
> +local res = table.move(t, 1, 1, 2)
> +test:ok(t == res, 'table.move returns the same table')
> +test:ok(t[1] == t[2], 'table.move is correct')
> 
> -test:ok(true)
> os.exit(test:check() and 0 or 1)
> ===================================================================
> 
>> 
>>> +
>>> +test:ok(true)
>>> +os.exit(test:check() and 0 or 1)
>>> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
>>> index c0403cf1..61d4de7a 100644
>>> --- a/test/tarantool-tests/utils.lua
>>> +++ b/test/tarantool-tests/utils.lua
>>> @@ -2,11 +2,14 @@ local M = {}
>>> 
>>> local ffi = require('ffi')
>>> local tap = require('tap')
>>> +local bc = require('jit.bc')
>>> 
>>> ffi.cdef([[
>>>  int setenv(const char *name, const char *value, int overwrite);
>>> ]])
>>> 
>>> +local function noop() end
>> 
>> Name of this one in a patch that messess with bytecodes is confusing. Could it
>> be a simpler one, like ‘empty’?
> 
> Fixed. Branch is force-pushed.
> 
> ===================================================================
> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index 61d4de7a..57932c5d 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua
> @@ -8,7 +8,7 @@ ffi.cdef([[
>   int setenv(const char *name, const char *value, int overwrite);
> ]])
> 
> -local function noop() end
> +local function empty() end
> 
> local function luacmd(args)
>   -- arg[-1] is guaranteed to be not nil.
> @@ -101,11 +101,11 @@ function M.hasbc(f, bytecode)
>     write = function(out, line)
>       if line:match(bytecode) then
>         hasbc = true
> -        out.write = noop
> +        out.write = empty
>       end
>     end,
> -    flush = noop,
> -    close = noop,
> +    flush = empty,
> +    close = empty,
>   }
>   bc.dump(f, out)
>   return hasbc
> ===================================================================
> 
>> 
>>> +
>>> local function luacmd(args)
>>>  -- arg[-1] is guaranteed to be not nil.
>>>  local idx = -2
>>> @@ -89,4 +92,23 @@ function M.tweakenv(condition, variable)
>>>  ffi.C.setenv(variable, testvar, 0)
>>> end
>>> 
>>> +function M.hasbc(f, bytecode)
>>> +  assert(type(f) == 'function', 'argument #1 should be a function')
>>> +  assert(type(bytecode) == 'string', 'argument #2 should be a string')
>>> +  local hasbc = false
>>> +  -- Check the bytecode entry line by line.
>>> +  local out = {
>>> +    write = function(out, line)
>>> +      if line:match(bytecode) then
>>> +        hasbc = true
>>> +        out.write = noop
>>> +      end
>>> +    end,
>>> +    flush = noop,
>>> +    close = noop,
>>> +  }
>>> +  bc.dump(f, out)
>>> +  return hasbc
>>> +end
>>> +
>>> return M
>>> -- 
>>> 2.31.0
>>> 
>> 
> 
> -- 
> Best regards,
> Sergey Kaplun


[-- Attachment #2: Type: text/html, Size: 94546 bytes --]

Re: [Tarantool-patches] [PATCH luajit 1/4] ARM, ARM64, PPC: Fix TSETR fallback.

[Sergey Kaplun via Tarantool-patches]

On 04.06.21, Sergey Ostanevich wrote:
> Hi!
> 
> Thanks for the update, LGTM.
> 
> As for LuaJIT code style - should it follow the Tarantool Lua one?
> https://www.tarantool.io/en/doc/latest/dev_guide/lua_style_guide/ <https://www.tarantool.io/en/doc/latest/dev_guide/lua_style_guide/>

Not exactly -- LuaJIT prefers fake-quarter tabs (2 spaces indent level,
tab instead 8 spaces).

Same for the C style. (I can't tell that I am a big fun of it.)
I suppose that it's come from PUC-Rio Lua sources.

> 
> Sergos.
> 

<snipped>

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/4] ARM, ARM64, PPC: Fix TSETR fallback.

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for the patch! LGTM, with several nits below.

On 24.05.21, Sergey Kaplun wrote:
> From: Mike Pall <mike>
> 
> Thanks to Javier Guerra Giraldez.
> 
> (cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)
> 
> This patch fixes the issue introduced by commits
> f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
> infrastructure and initial port of interpreter.') for arm64 and
> 73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
> builtins.') for arm and ppc. Within the mentioned commits the new
> bytecode TSETR is introduced for the corresponding architectures.
> 
> When the new index of the table processed during this bytecode is the
> integer, that is greater than asize of the table, the VM fallbacks to
> vmeta_tsetr, for calling
> lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
> CARG1 is not set by the VM and contains an invalid value, so the
> mentioned call leads to crash.

Minor: IMHO, it's worth to explicitly mention the value that need to be
set before the call (strictly saying CARG1 is set by VM, but this is a
wrong value). Feel free to ignore.

> This patch adds the missed set of CARG1 to the right value.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Resolves tarantool/tarantool#6084
> Part of tarantool/tarantool#5629
> ---
>  src/vm_arm.dasc                               |  1 +
>  src/vm_arm64.dasc                             |  1 +
>  src/vm_ppc.dasc                               |  1 +
>  test/tarantool-tests/CMakeLists.txt           |  9 ++++---
>  ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
>  test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
>  6 files changed, 55 insertions(+), 4 deletions(-)
>  create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> 

<snipped>

> diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> new file mode 100644
> index 00000000..26344274
> --- /dev/null
> +++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> @@ -0,0 +1,25 @@
> +local tap = require("tap")
> +local utils = require("utils")
> +
> +local test = tap.test("gh-6084-missed-carg1-in-bctsetr-fallback")
> +test:plan(1)
> +
> +-- Bytecode TSETR appears only in built-ins libraries, when doing

Minor: It's worth to use 'XXX:' here.

> +-- fixups for fast function written in Lua (i.e. `table.move()`),
> +-- by replacing all TSETV bytecodes with the TSETR.
> +-- See <src/host/genlibbc.lua> for more details.
> +
> +-- This test checks that fallback path, when the index of the new
> +-- set element is greater than the table's asize, doesn't lead
> +-- to a crash.
> +
> +-- We need to make sure the bytecode is present in the chosen

Ditto.

> +-- built-in to make sure our test is still valid.
> +assert(utils.hasbc(table.move, "TSETR"))
> +
> +-- Empty table has asize equals 0. Just copy its element (equals

Typo: s/Empty table has asize equals 0/Empty table asize equals 0/.

> +-- nil) to the field by index 1 > 0, to fallback inside TSETR.
> +table.move({}, 1, 1, 1)

Side note: Totally agree with Sergos; Seen the changes on the branch.

> +
> +test:ok(true)
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index c0403cf1..61d4de7a 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua
> @@ -2,11 +2,14 @@ local M = {}
>  
>  local ffi = require('ffi')
>  local tap = require('tap')
> +local bc = require('jit.bc')
>  
>  ffi.cdef([[
>    int setenv(const char *name, const char *value, int overwrite);
>  ]])
>  
> +local function noop() end

This is a dummy function that is only required by <M.hasbc>, so move the
helper closer to it. I would even suggest to move it directly to
<M.hasbc>, or even use `function() end` three times, but this is not
our style ;)

> +
>  local function luacmd(args)
>    -- arg[-1] is guaranteed to be not nil.
>    local idx = -2
> @@ -89,4 +92,23 @@ function M.tweakenv(condition, variable)
>    ffi.C.setenv(variable, testvar, 0)
>  end
>  
> +function M.hasbc(f, bytecode)
> +  assert(type(f) == 'function', 'argument #1 should be a function')
> +  assert(type(bytecode) == 'string', 'argument #2 should be a string')
> +  local hasbc = false
> +  -- Check the bytecode entry line by line.
> +  local out = {
> +    write = function(out, line)
> +      if line:match(bytecode) then
> +        hasbc = true
> +        out.write = noop
> +      end
> +    end,
> +    flush = noop,
> +    close = noop,

Minor: This is excess for this function, since it doesn't close the
stream explicitly. Feel free to ignore.

> +  }
> +  bc.dump(f, out)
> +  return hasbc
> +end
> +
>  return M
> -- 
> 2.31.0
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 1/4] ARM, ARM64, PPC: Fix TSETR fallback.

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!

Thanks for the review!

On 10.06.21, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! LGTM, with several nits below.
> 
> On 24.05.21, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> > 
> > Thanks to Javier Guerra Giraldez.
> > 
> > (cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)
> > 
> > This patch fixes the issue introduced by commits
> > f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
> > infrastructure and initial port of interpreter.') for arm64 and
> > 73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
> > builtins.') for arm and ppc. Within the mentioned commits the new
> > bytecode TSETR is introduced for the corresponding architectures.
> > 
> > When the new index of the table processed during this bytecode is the
> > integer, that is greater than asize of the table, the VM fallbacks to
> > vmeta_tsetr, for calling
> > lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
> > CARG1 is not set by the VM and contains an invalid value, so the
> > mentioned call leads to crash.
> 
> Minor: IMHO, it's worth to explicitly mention the value that need to be
> set before the call (strictly saying CARG1 is set by VM, but this is a
> wrong value). Feel free to ignore.

Update commit message to the following:

===================================================================
ARM, ARM64, PPC: Fix TSETR fallback.

Thanks to Javier Guerra Giraldez.

(cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)

This patch fixes the issue introduced by commits
f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
infrastructure and initial port of interpreter.') for arm64 and
73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
builtins.') for arm and ppc. Within the mentioned commits the new
bytecode TSETR is introduced for the corresponding architectures.

When the new index of the table processed during this bytecode is the
integer, that is greater than asize of the table, the VM fallbacks to
vmeta_tsetr, for calling
lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
CARG1 is not set to lua_State by the VM and contains an invalid value,
so the mentioned call leads to crash.
This patch adds the missed set of CARG1 to the right value.

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

Resolves tarantool/tarantool#6084
Part of tarantool/tarantool#5629
===================================================================

Branch is force-pushed.

> 
> > This patch adds the missed set of CARG1 to the right value.
> > 
> > Sergey Kaplun:
> > * added the description and the test for the problem
> > 
> > Resolves tarantool/tarantool#6084
> > Part of tarantool/tarantool#5629
> > ---
> >  src/vm_arm.dasc                               |  1 +
> >  src/vm_arm64.dasc                             |  1 +
> >  src/vm_ppc.dasc                               |  1 +
> >  test/tarantool-tests/CMakeLists.txt           |  9 ++++---
> >  ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
> >  test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
> >  6 files changed, 55 insertions(+), 4 deletions(-)
> >  create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> > 
> 
> <snipped>
> 
> > diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> > new file mode 100644
> > index 00000000..26344274
> > --- /dev/null
> > +++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> > @@ -0,0 +1,25 @@
> > +local tap = require("tap")
> > +local utils = require("utils")
> > +
> > +local test = tap.test("gh-6084-missed-carg1-in-bctsetr-fallback")
> > +test:plan(1)
> > +
> > +-- Bytecode TSETR appears only in built-ins libraries, when doing
> 
> Minor: It's worth to use 'XXX:' here.
> 
> > +-- fixups for fast function written in Lua (i.e. `table.move()`),
> > +-- by replacing all TSETV bytecodes with the TSETR.
> > +-- See <src/host/genlibbc.lua> for more details.
> > +
> > +-- This test checks that fallback path, when the index of the new
> > +-- set element is greater than the table's asize, doesn't lead
> > +-- to a crash.
> > +
> > +-- We need to make sure the bytecode is present in the chosen
> 
> Ditto.
> 
> > +-- built-in to make sure our test is still valid.
> > +assert(utils.hasbc(table.move, "TSETR"))
> > +
> > +-- Empty table has asize equals 0. Just copy its element (equals
> 
> Typo: s/Empty table has asize equals 0/Empty table asize equals 0/.
> 
> > +-- nil) to the field by index 1 > 0, to fallback inside TSETR.
> > +table.move({}, 1, 1, 1)
> 
> Side note: Totally agree with Sergos; Seen the changes on the branch.

Fixed comment-related comments. See the iterative patch below:

===================================================================
diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
index 95bf3bd7..04b129d8 100644
--- a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
+++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
@@ -4,20 +4,20 @@ local utils = require('utils')
 local test = tap.test('gh-6084-missed-carg1-in-bctsetr-fallback')
 test:plan(2)
 
--- Bytecode TSETR appears only in built-ins libraries, when doing
--- fixups for fast function written in Lua (i.e. `table.move()`),
--- by replacing all TSETV bytecodes with the TSETR.
--- See <src/host/genlibbc.lua> for more details.
+-- XXX: Bytecode TSETR appears only in built-ins libraries, when
+-- doing fixups for fast function written in Lua
+-- (i.e. `table.move()`), by replacing all TSETV bytecodes with
+-- the TSETR. See <src/host/genlibbc.lua> for more details.
 
 -- This test checks that fallback path, when the index of the new
 -- set element is greater than the table's asize, doesn't lead
 -- to a crash.
 
--- We need to make sure the bytecode is present in the chosen
+-- XXX: We need to make sure the bytecode is present in the chosen
 -- built-in to make sure our test is still valid.
 assert(utils.hasbc(table.move, 'TSETR'))
 
--- `t` table has asize equals 1. Just copy its first element (1)
+-- `t` table asize equals 1. Just copy its first element (1)
 -- to the field by index 2 > 1, to fallback inside TSETR.
 local t = {1}
 local res = table.move(t, 1, 1, 2)
===================================================================

> 
> > +
> > +test:ok(true)
> > +os.exit(test:check() and 0 or 1)
> > diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> > index c0403cf1..61d4de7a 100644
> > --- a/test/tarantool-tests/utils.lua
> > +++ b/test/tarantool-tests/utils.lua
> > @@ -2,11 +2,14 @@ local M = {}
> >  
> >  local ffi = require('ffi')
> >  local tap = require('tap')
> > +local bc = require('jit.bc')
> >  
> >  ffi.cdef([[
> >    int setenv(const char *name, const char *value, int overwrite);
> >  ]])
> >  
> > +local function noop() end
> 
> This is a dummy function that is only required by <M.hasbc>, so move the
> helper closer to it. I would even suggest to move it directly to
> <M.hasbc>, or even use `function() end` three times, but this is not
> our style ;)

Moved to `M.hasbc()`.
See the iterative patch below:

===================================================================
diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index 57932c5d..5bd42b30 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -8,8 +8,6 @@ ffi.cdef([[
   int setenv(const char *name, const char *value, int overwrite);
 ]])
 
-local function empty() end
-
 local function luacmd(args)
   -- arg[-1] is guaranteed to be not nil.
   local idx = -2
@@ -95,6 +93,7 @@ end
 function M.hasbc(f, bytecode)
   assert(type(f) == 'function', 'argument #1 should be a function')
   assert(type(bytecode) == 'string', 'argument #2 should be a string')
+  local function empty() end
   local hasbc = false
   -- Check the bytecode entry line by line.
   local out = {
===================================================================

> 
> > +
> >  local function luacmd(args)
> >    -- arg[-1] is guaranteed to be not nil.
> >    local idx = -2
> > @@ -89,4 +92,23 @@ function M.tweakenv(condition, variable)
> >    ffi.C.setenv(variable, testvar, 0)
> >  end
> >  
> > +function M.hasbc(f, bytecode)
> > +  assert(type(f) == 'function', 'argument #1 should be a function')
> > +  assert(type(bytecode) == 'string', 'argument #2 should be a string')
> > +  local hasbc = false
> > +  -- Check the bytecode entry line by line.
> > +  local out = {
> > +    write = function(out, line)
> > +      if line:match(bytecode) then
> > +        hasbc = true
> > +        out.write = noop
> > +      end
> > +    end,
> > +    flush = noop,
> > +    close = noop,
> 
> Minor: This is excess for this function, since it doesn't close the
> stream explicitly. Feel free to ignore.

I've added this for consistency. Ignoring for now.

> 
> > +  }
> > +  bc.dump(f, out)
> > +  return hasbc
> > +end
> > +
> >  return M
> > -- 
> > 2.31.0
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/4] ARM, ARM64, PPC: Fix TSETR fallback.

[Sergey Kaplun via Tarantool-patches]

Hi, again!

I've added some minor updates to the commit message and comments.
Branch is force-pushed.

On 11.06.21, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Igor!
> 
> Thanks for the review!
> 
> On 10.06.21, Igor Munkin wrote:
> > Sergey,
> > 
> > Thanks for the patch! LGTM, with several nits below.
> > 
> > On 24.05.21, Sergey Kaplun wrote:
> > > From: Mike Pall <mike>
> > > 
> > > Thanks to Javier Guerra Giraldez.
> > > 
> > > (cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)
> > > 
> > > This patch fixes the issue introduced by commits
> > > f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
> > > infrastructure and initial port of interpreter.') for arm64 and
> > > 73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
> > > builtins.') for arm and ppc. Within the mentioned commits the new
> > > bytecode TSETR is introduced for the corresponding architectures.
> > > 
> > > When the new index of the table processed during this bytecode is the
> > > integer, that is greater than asize of the table, the VM fallbacks to
> > > vmeta_tsetr, for calling
> > > lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
> > > CARG1 is not set by the VM and contains an invalid value, so the
> > > mentioned call leads to crash.
> > 
> > Minor: IMHO, it's worth to explicitly mention the value that need to be
> > set before the call (strictly saying CARG1 is set by VM, but this is a
> > wrong value). Feel free to ignore.
> 
> Update commit message to the following:
> 
> ===================================================================
> ARM, ARM64, PPC: Fix TSETR fallback.
> 
> Thanks to Javier Guerra Giraldez.
> 
> (cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)
> 
> This patch fixes the issue introduced by commits
> f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
> infrastructure and initial port of interpreter.') for arm64 and
> 73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
> builtins.') for arm and ppc. Within the mentioned commits the new
> bytecode TSETR is introduced for the corresponding architectures.
> 
> When the new index of the table processed during this bytecode is the
> integer, that is greater than asize of the table, the VM fallbacks to
> vmeta_tsetr, for calling
> lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
> CARG1 is not set to lua_State by the VM and contains an invalid value,
> so the mentioned call leads to crash.
> This patch adds the missed set of CARG1 to the right value.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Resolves tarantool/tarantool#6084
> Part of tarantool/tarantool#5629
> ===================================================================

Clarified the commit message as the following:

===================================================================
ARM, ARM64, PPC: Fix TSETR fallback.

Thanks to Javier Guerra Giraldez.

(cherry picked from commit ae20998ff5aaacc8e3afd46c64e28a8e039b58a1)

This patch fixes the issue introduced by commits
f307d0adafc7e35d2dc1c461d50f6572c5e6bca8 ('ARM64: Add build
infrastructure and initial port of interpreter.') for arm64 and
73ef845fcaf65937ad63e9cf6b681cb3e61f4504 ('Add special bytecodes for
builtins.') for arm and ppc. Within the mentioned commits the new
bytecode TSETR is introduced for the corresponding architectures.

When the new index of the table processed during this bytecode is the
integer, that is greater than asize of the table, the VM fallbacks to
vmeta_tsetr, for calling
lj_tab_setinth(lua_State *L, GCtab *t, int32_t key). The first argument
CARG1 is not set to currently executed Lua thread by the VM and contains
an invalid value, so the mentioned call leads to crash.
This patch adds the missed set of CARG1 to the right value.

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

Resolves tarantool/tarantool#6084
Part of tarantool/tarantool#5629
===================================================================


> 
> Branch is force-pushed.
> 
> > 
> > > This patch adds the missed set of CARG1 to the right value.
> > > 
> > > Sergey Kaplun:
> > > * added the description and the test for the problem
> > > 
> > > Resolves tarantool/tarantool#6084
> > > Part of tarantool/tarantool#5629
> > > ---
> > >  src/vm_arm.dasc                               |  1 +
> > >  src/vm_arm64.dasc                             |  1 +
> > >  src/vm_ppc.dasc                               |  1 +
> > >  test/tarantool-tests/CMakeLists.txt           |  9 ++++---
> > >  ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
> > >  test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
> > >  6 files changed, 55 insertions(+), 4 deletions(-)
> > >  create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> > > 
> > 
> > <snipped>
> > 
> > > diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> > > new file mode 100644
> > > index 00000000..26344274
> > > --- /dev/null
> > > +++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> > > @@ -0,0 +1,25 @@
> > > +local tap = require("tap")
> > > +local utils = require("utils")
> > > +
> > > +local test = tap.test("gh-6084-missed-carg1-in-bctsetr-fallback")
> > > +test:plan(1)
> > > +
> > > +-- Bytecode TSETR appears only in built-ins libraries, when doing
> > 
> > Minor: It's worth to use 'XXX:' here.
> > 
> > > +-- fixups for fast function written in Lua (i.e. `table.move()`),
> > > +-- by replacing all TSETV bytecodes with the TSETR.
> > > +-- See <src/host/genlibbc.lua> for more details.
> > > +
> > > +-- This test checks that fallback path, when the index of the new
> > > +-- set element is greater than the table's asize, doesn't lead
> > > +-- to a crash.
> > > +
> > > +-- We need to make sure the bytecode is present in the chosen
> > 
> > Ditto.
> > 
> > > +-- built-in to make sure our test is still valid.
> > > +assert(utils.hasbc(table.move, "TSETR"))
> > > +
> > > +-- Empty table has asize equals 0. Just copy its element (equals
> > 
> > Typo: s/Empty table has asize equals 0/Empty table asize equals 0/.
> > 
> > > +-- nil) to the field by index 1 > 0, to fallback inside TSETR.
> > > +table.move({}, 1, 1, 1)
> > 
> > Side note: Totally agree with Sergos; Seen the changes on the branch.
> 
> Fixed comment-related comments. See the iterative patch below:
> 
> ===================================================================
> diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> index 95bf3bd7..04b129d8 100644
> --- a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> +++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> @@ -4,20 +4,20 @@ local utils = require('utils')
>  local test = tap.test('gh-6084-missed-carg1-in-bctsetr-fallback')
>  test:plan(2)
>  
> --- Bytecode TSETR appears only in built-ins libraries, when doing
> --- fixups for fast function written in Lua (i.e. `table.move()`),
> --- by replacing all TSETV bytecodes with the TSETR.
> --- See <src/host/genlibbc.lua> for more details.
> +-- XXX: Bytecode TSETR appears only in built-ins libraries, when
> +-- doing fixups for fast function written in Lua
> +-- (i.e. `table.move()`), by replacing all TSETV bytecodes with

Fix line underfullness here (in LaTEX terms).

===================================================================
diff --git a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
index 04b129d8..86bbabe3 100644
--- a/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
+++ b/test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
@@ -5,8 +5,8 @@ local test = tap.test('gh-6084-missed-carg1-in-bctsetr-fallback')
 test:plan(2)
 
 -- XXX: Bytecode TSETR appears only in built-ins libraries, when
--- doing fixups for fast function written in Lua
--- (i.e. `table.move()`), by replacing all TSETV bytecodes with
+-- doing fixups for fast function written in Lua (i.e.
+-- `table.move()`), by replacing all TSETV bytecodes with
 -- the TSETR. See <src/host/genlibbc.lua> for more details.
 
 -- This test checks that fallback path, when the index of the new
===================================================================


> +-- the TSETR. See <src/host/genlibbc.lua> for more details.
>  
>  -- This test checks that fallback path, when the index of the new
>  -- set element is greater than the table's asize, doesn't lead
>  -- to a crash.
>  
> --- We need to make sure the bytecode is present in the chosen
> +-- XXX: We need to make sure the bytecode is present in the chosen
>  -- built-in to make sure our test is still valid.
>  assert(utils.hasbc(table.move, 'TSETR'))
>  
> --- `t` table has asize equals 1. Just copy its first element (1)
> +-- `t` table asize equals 1. Just copy its first element (1)
>  -- to the field by index 2 > 1, to fallback inside TSETR.
>  local t = {1}
>  local res = table.move(t, 1, 1, 2)
> ===================================================================
> 
> > 
> > > +
> > > +test:ok(true)
> > > +os.exit(test:check() and 0 or 1)
> > > diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> > > index c0403cf1..61d4de7a 100644
> > > --- a/test/tarantool-tests/utils.lua
> > > +++ b/test/tarantool-tests/utils.lua
> > > @@ -2,11 +2,14 @@ local M = {}
> > >  
> > >  local ffi = require('ffi')
> > >  local tap = require('tap')
> > > +local bc = require('jit.bc')
> > >  
> > >  ffi.cdef([[
> > >    int setenv(const char *name, const char *value, int overwrite);
> > >  ]])
> > >  
> > > +local function noop() end
> > 
> > This is a dummy function that is only required by <M.hasbc>, so move the
> > helper closer to it. I would even suggest to move it directly to
> > <M.hasbc>, or even use `function() end` three times, but this is not
> > our style ;)
> 
> Moved to `M.hasbc()`.
> See the iterative patch below:
> 
> ===================================================================
> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index 57932c5d..5bd42b30 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua
> @@ -8,8 +8,6 @@ ffi.cdef([[
>    int setenv(const char *name, const char *value, int overwrite);
>  ]])
>  
> -local function empty() end
> -
>  local function luacmd(args)
>    -- arg[-1] is guaranteed to be not nil.
>    local idx = -2
> @@ -95,6 +93,7 @@ end
>  function M.hasbc(f, bytecode)
>    assert(type(f) == 'function', 'argument #1 should be a function')
>    assert(type(bytecode) == 'string', 'argument #2 should be a string')
> +  local function empty() end
>    local hasbc = false
>    -- Check the bytecode entry line by line.
>    local out = {
> ===================================================================
> 
> > 
> > > +
> > >  local function luacmd(args)
> > >    -- arg[-1] is guaranteed to be not nil.
> > >    local idx = -2
> > > @@ -89,4 +92,23 @@ function M.tweakenv(condition, variable)
> > >    ffi.C.setenv(variable, testvar, 0)
> > >  end
> > >  
> > > +function M.hasbc(f, bytecode)
> > > +  assert(type(f) == 'function', 'argument #1 should be a function')
> > > +  assert(type(bytecode) == 'string', 'argument #2 should be a string')
> > > +  local hasbc = false
> > > +  -- Check the bytecode entry line by line.
> > > +  local out = {
> > > +    write = function(out, line)
> > > +      if line:match(bytecode) then
> > > +        hasbc = true
> > > +        out.write = noop
> > > +      end
> > > +    end,
> > > +    flush = noop,
> > > +    close = noop,
> > 
> > Minor: This is excess for this function, since it doesn't close the
> > stream explicitly. Feel free to ignore.
> 
> I've added this for consistency. Ignoring for now.
> 
> > 
> > > +  }
> > > +  bc.dump(f, out)
> > > +  return hasbc
> > > +end
> > > +
> > >  return M
> > > -- 
> > > 2.31.0
> > > 
> > 
> > -- 
> > Best regards,
> > IM
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
Sergey Kaplun

[Tarantool-patches] [PATCH luajit 2/4] test: add skipcond on architectures for memprof

[Sergey Kaplun via Tarantool-patches]

Memprof's tests fail for architectures different from x86 and x64,
because memprof is not yet implemented for them.

This patch adds skip condition to corresponding test.
---
 test/tarantool-tests/misclib-memprof-lapi.test.lua | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index b4d66509..4df9cfd8 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -1,3 +1,9 @@
+-- Memprof is implemented only for x86 and x64 architectures.
+require("utils").skipcond(
+  jit.arch ~= "x86" or jit.arch ~= "x64",
+  jit.arch.." architecture is NIY for memprof"
+)
+
 local tap = require("tap")
 
 local test = tap.test("misc-memprof-lapi")
-- 
2.31.0

Re: [Tarantool-patches] [PATCH luajit 2/4] test: add skipcond on architectures for memprof

[Sergey Ostanevich via Tarantool-patches]

Hi!

Thanks for the patch!
Just one fixup is needed.

Sergos

> On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> Memprof's tests fail for architectures different from x86 and x64,
> because memprof is not yet implemented for them.
> 
> This patch adds skip condition to corresponding test.
> ---
> test/tarantool-tests/misclib-memprof-lapi.test.lua | 6 ++++++
> 1 file changed, 6 insertions(+)
> 
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index b4d66509..4df9cfd8 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -1,3 +1,9 @@
> +-- Memprof is implemented only for x86 and x64 architectures.
                             ^^^^ ——>                          ^
> +require("utils").skipcond(
> +  jit.arch ~= "x86" or jit.arch ~= "x64",

I believe you overcomplicated the condition so it’ll skip even for
mentioned archs

tarantool> x='x86'
---
...

tarantool> x~='x86' or x~='x64'
---
- true
...



> +  jit.arch.." architecture is NIY for memprof"
> +)
> +
> local tap = require("tap")
> 
> local test = tap.test("misc-memprof-lapi")
> -- 
> 2.31.0
> 

Re: [Tarantool-patches] [PATCH luajit 2/4] test: add skipcond on architectures for memprof

[Sergey Kaplun via Tarantool-patches]

Hi!

Thanks for the review!

On 02.06.21, Sergey Ostanevich wrote:
> Hi!
> 
> Thanks for the patch!
> Just one fixup is needed.
> 
> Sergos
> 
> > On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > Memprof's tests fail for architectures different from x86 and x64,
> > because memprof is not yet implemented for them.
> > 
> > This patch adds skip condition to corresponding test.
> > ---
> > test/tarantool-tests/misclib-memprof-lapi.test.lua | 6 ++++++
> > 1 file changed, 6 insertions(+)
> > 
> > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > index b4d66509..4df9cfd8 100644
> > --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > @@ -1,3 +1,9 @@
> > +-- Memprof is implemented only for x86 and x64 architectures.
>                              ^^^^ ——>                          ^
> > +require("utils").skipcond(
> > +  jit.arch ~= "x86" or jit.arch ~= "x64",
> 
> I believe you overcomplicated the condition so it’ll skip even for
> mentioned archs
> 
> tarantool> x='x86'
> ---
> ...
> 
> tarantool> x~='x86' or x~='x64'
> ---
> - true
> ...
> 

Thanks, nice catch!

Fixed.
Branch is force-pushed.

===================================================================
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index 4df9cfd8..96beb680 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -1,6 +1,6 @@
--- Memprof is implemented only for x86 and x64 architectures.
+-- Memprof is implemented for only x86 and x64 architectures.
 require("utils").skipcond(
-  jit.arch ~= "x86" or jit.arch ~= "x64",
+  jit.arch ~= "x86" and jit.arch ~= "x64",
   jit.arch.." architecture is NIY for memprof"
 )
 
@@ -131,9 +131,9 @@ local free = fill_ev_type(events, symbols, "free")
 -- the number of allocations.
 -- 1 event - alocation of table by itself + 1 allocation
 -- of array part as far it is bigger than LJ_MAX_COLOSIZE (16).
-test:ok(check_alloc_report(alloc, 21, 19, 2))
+test:ok(check_alloc_report(alloc, 27, 25, 2))
 -- 100 strings allocations.
-test:ok(check_alloc_report(alloc, 26, 19, 100))
+test:ok(check_alloc_report(alloc, 32, 25, 100))
 
 -- Collect all previous allocated objects.
 test:ok(free.INTERNAL.num == 102)
@@ -141,8 +141,8 @@ test:ok(free.INTERNAL.num == 102)
 -- Tests for leak-only option.
 -- See also https://github.com/tarantool/tarantool/issues/5812.
 local heap_delta = process.form_heap_delta(events, symbols)
-local tab_alloc_stats = heap_delta[form_source_line(21)]
-local str_alloc_stats = heap_delta[form_source_line(26)]
+local tab_alloc_stats = heap_delta[form_source_line(27)]
+local str_alloc_stats = heap_delta[form_source_line(32)]
 test:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree)
 test:ok(tab_alloc_stats.dbytes == 0)
 test:ok(str_alloc_stats.nalloc == str_alloc_stats.nfree)
===================================================================

> 
> 
> > +  jit.arch.." architecture is NIY for memprof"
> > +)
> > +
> > local tap = require("tap")
> > 
> > local test = tap.test("misc-memprof-lapi")
> > -- 
> > 2.31.0
> > 
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/4] test: add skipcond on architectures for memprof

[Sergey Ostanevich via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 3858 bytes --]

Hi!

Thanks for the update. 
I was bad in my ASCII art. The change I expected was

> +-- Memprof is implemented for x86 and x64 architectures only.

LGTM
Sergos


> On 4 Jun 2021, at 16:37, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> Hi!
> 
> Thanks for the review!
> 
> On 02.06.21, Sergey Ostanevich wrote:
>> Hi!
>> 
>> Thanks for the patch!
>> Just one fixup is needed.
>> 
>> Sergos
>> 
>>> On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
>>> 
>>> Memprof's tests fail for architectures different from x86 and x64,
>>> because memprof is not yet implemented for them.
>>> 
>>> This patch adds skip condition to corresponding test.
>>> ---
>>> test/tarantool-tests/misclib-memprof-lapi.test.lua | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>> 
>>> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>> index b4d66509..4df9cfd8 100644
>>> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>> @@ -1,3 +1,9 @@
>>> +-- Memprof is implemented only for x86 and x64 architectures.
>>                             ^^^^ ——>                          ^
>>> +require("utils").skipcond(
>>> +  jit.arch ~= "x86" or jit.arch ~= "x64",
>> 
>> I believe you overcomplicated the condition so it’ll skip even for
>> mentioned archs
>> 
>> tarantool> x='x86'
>> ---
>> ...
>> 
>> tarantool> x~='x86' or x~='x64'
>> ---
>> - true
>> ...
>> 
> 
> Thanks, nice catch!
> 
> Fixed.
> Branch is force-pushed.
> 
> ===================================================================
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index 4df9cfd8..96beb680 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -1,6 +1,6 @@
> --- Memprof is implemented only for x86 and x64 architectures.
> +-- Memprof is implemented for only x86 and x64 architectures.
> require("utils").skipcond(
> -  jit.arch ~= "x86" or jit.arch ~= "x64",
> +  jit.arch ~= "x86" and jit.arch ~= "x64",
>   jit.arch.." architecture is NIY for memprof"
> )
> 
> @@ -131,9 +131,9 @@ local free = fill_ev_type(events, symbols, "free")
> -- the number of allocations.
> -- 1 event - alocation of table by itself + 1 allocation
> -- of array part as far it is bigger than LJ_MAX_COLOSIZE (16).
> -test:ok(check_alloc_report(alloc, 21, 19, 2))
> +test:ok(check_alloc_report(alloc, 27, 25, 2))
> -- 100 strings allocations.
> -test:ok(check_alloc_report(alloc, 26, 19, 100))
> +test:ok(check_alloc_report(alloc, 32, 25, 100))
> 
> -- Collect all previous allocated objects.
> test:ok(free.INTERNAL.num == 102)
> @@ -141,8 +141,8 @@ test:ok(free.INTERNAL.num == 102)
> -- Tests for leak-only option.
> -- See also https://github.com/tarantool/tarantool/issues/5812 <https://github.com/tarantool/tarantool/issues/5812>.
> local heap_delta = process.form_heap_delta(events, symbols)
> -local tab_alloc_stats = heap_delta[form_source_line(21)]
> -local str_alloc_stats = heap_delta[form_source_line(26)]
> +local tab_alloc_stats = heap_delta[form_source_line(27)]
> +local str_alloc_stats = heap_delta[form_source_line(32)]
> test:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree)
> test:ok(tab_alloc_stats.dbytes == 0)
> test:ok(str_alloc_stats.nalloc == str_alloc_stats.nfree)
> ===================================================================
> 
>> 
>> 
>>> +  jit.arch.." architecture is NIY for memprof"
>>> +)
>>> +
>>> local tap = require("tap")
>>> 
>>> local test = tap.test("misc-memprof-lapi")
>>> -- 
>>> 2.31.0
>>> 
>> 
> 
> -- 
> Best regards,
> Sergey Kaplun


[-- Attachment #2: Type: text/html, Size: 42526 bytes --]

Re: [Tarantool-patches] [PATCH luajit 2/4] test: add skipcond on architectures for memprof

[Sergey Kaplun via Tarantool-patches]

On 04.06.21, Sergey Ostanevich wrote:
> Hi!
> 
> Thanks for the update. 
> I was bad in my ASCII art. The change I expected was
> 
> > +-- Memprof is implemented for x86 and x64 architectures only.

Sure!
Fixed. Branch is force-pushed.

===================================================================
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index 96beb680..06d96b3b 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -1,4 +1,4 @@
--- Memprof is implemented for only x86 and x64 architectures.
+-- Memprof is implemented for x86 and x64 architectures only.
 require("utils").skipcond(
   jit.arch ~= "x86" and jit.arch ~= "x64",
   jit.arch.." architecture is NIY for memprof"
===================================================================

> 
> LGTM
> Sergos
> 
> 
> > On 4 Jun 2021, at 16:37, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > Hi!
> > 
> > Thanks for the review!
> > 
> > On 02.06.21, Sergey Ostanevich wrote:
> >> Hi!
> >> 
> >> Thanks for the patch!
> >> Just one fixup is needed.
> >> 
> >> Sergos
> >> 
> >>> On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
> >>> 
> >>> Memprof's tests fail for architectures different from x86 and x64,
> >>> because memprof is not yet implemented for them.
> >>> 
> >>> This patch adds skip condition to corresponding test.
> >>> ---
> >>> test/tarantool-tests/misclib-memprof-lapi.test.lua | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>> 
> >>> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> >>> index b4d66509..4df9cfd8 100644
> >>> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> >>> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> >>> @@ -1,3 +1,9 @@
> >>> +-- Memprof is implemented only for x86 and x64 architectures.
> >>                             ^^^^ ——>                          ^
> >>> +require("utils").skipcond(
> >>> +  jit.arch ~= "x86" or jit.arch ~= "x64",
> >> 
> >> I believe you overcomplicated the condition so it’ll skip even for
> >> mentioned archs
> >> 
> >> tarantool> x='x86'
> >> ---
> >> ...
> >> 
> >> tarantool> x~='x86' or x~='x64'
> >> ---
> >> - true
> >> ...
> >> 
> > 
> > Thanks, nice catch!
> > 
> > Fixed.
> > Branch is force-pushed.
> > 
> > ===================================================================
> > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > index 4df9cfd8..96beb680 100644
> > --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > @@ -1,6 +1,6 @@
> > --- Memprof is implemented only for x86 and x64 architectures.
> > +-- Memprof is implemented for only x86 and x64 architectures.
> > require("utils").skipcond(
> > -  jit.arch ~= "x86" or jit.arch ~= "x64",
> > +  jit.arch ~= "x86" and jit.arch ~= "x64",
> >   jit.arch.." architecture is NIY for memprof"
> > )
> > 
> > @@ -131,9 +131,9 @@ local free = fill_ev_type(events, symbols, "free")
> > -- the number of allocations.
> > -- 1 event - alocation of table by itself + 1 allocation
> > -- of array part as far it is bigger than LJ_MAX_COLOSIZE (16).
> > -test:ok(check_alloc_report(alloc, 21, 19, 2))
> > +test:ok(check_alloc_report(alloc, 27, 25, 2))
> > -- 100 strings allocations.
> > -test:ok(check_alloc_report(alloc, 26, 19, 100))
> > +test:ok(check_alloc_report(alloc, 32, 25, 100))
> > 
> > -- Collect all previous allocated objects.
> > test:ok(free.INTERNAL.num == 102)
> > @@ -141,8 +141,8 @@ test:ok(free.INTERNAL.num == 102)
> > -- Tests for leak-only option.
> > -- See also https://github.com/tarantool/tarantool/issues/5812 <https://github.com/tarantool/tarantool/issues/5812>.
> > local heap_delta = process.form_heap_delta(events, symbols)
> > -local tab_alloc_stats = heap_delta[form_source_line(21)]
> > -local str_alloc_stats = heap_delta[form_source_line(26)]
> > +local tab_alloc_stats = heap_delta[form_source_line(27)]
> > +local str_alloc_stats = heap_delta[form_source_line(32)]
> > test:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree)
> > test:ok(tab_alloc_stats.dbytes == 0)
> > test:ok(str_alloc_stats.nalloc == str_alloc_stats.nfree)
> > ===================================================================
> > 
> >> 
> >> 
> >>> +  jit.arch.." architecture is NIY for memprof"
> >>> +)
> >>> +
> >>> local tap = require("tap")
> >>> 
> >>> local test = tap.test("misc-memprof-lapi")
> >>> -- 
> >>> 2.31.0
> >>> 
> >> 
> > 
> > -- 
> > Best regards,
> > Sergey Kaplun
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/4] test: add skipcond on architectures for memprof

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for the patch! Consider the couple of nits below regarding the
commit wording. Otherwise, LGTM.

Minor: It's hard to parse the patch subject IMHO. I propose the
following rewording:
| test: add arch-specific skipcond for memprof

On 24.05.21, Sergey Kaplun wrote:
> Memprof's tests fail for architectures different from x86 and x64,
> because memprof is not yet implemented for them.
> 
> This patch adds skip condition to corresponding test.

Typo: s/to corresponding/to the corresponding/.

> ---
>  test/tarantool-tests/misclib-memprof-lapi.test.lua | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index b4d66509..4df9cfd8 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -1,3 +1,9 @@
> +-- Memprof is implemented only for x86 and x64 architectures.
> +require("utils").skipcond(
> +  jit.arch ~= "x86" or jit.arch ~= "x64",

Side note: Seen the bunch with the related fixes on the branch.

> +  jit.arch.." architecture is NIY for memprof"
> +)
> +
>  local tap = require("tap")
>  
>  local test = tap.test("misc-memprof-lapi")
> -- 
> 2.31.0
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 2/4] test: add skipcond on architectures for memprof

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!

Thanks for the review!

On 10.06.21, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! Consider the couple of nits below regarding the
> commit wording. Otherwise, LGTM.
> 
> Minor: It's hard to parse the patch subject IMHO. I propose the
> following rewording:
> | test: add arch-specific skipcond for memprof
> 
> On 24.05.21, Sergey Kaplun wrote:
> > Memprof's tests fail for architectures different from x86 and x64,
> > because memprof is not yet implemented for them.
> > 
> > This patch adds skip condition to corresponding test.
> 
> Typo: s/to corresponding/to the corresponding/.

Fixed. Branch is force-pushed. The new commit message is:

===================================================================
test: add arch-specific skipcond for memprof

Memprof's tests fail for architectures different from x86 and x64,
because memprof is not yet implemented for them.

This patch adds skip condition to the corresponding test.
===================================================================

> 
> > ---
> >  test/tarantool-tests/misclib-memprof-lapi.test.lua | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > index b4d66509..4df9cfd8 100644
> > --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > @@ -1,3 +1,9 @@
> > +-- Memprof is implemented only for x86 and x64 architectures.
> > +require("utils").skipcond(
> > +  jit.arch ~= "x86" or jit.arch ~= "x64",
> 
> Side note: Seen the bunch with the related fixes on the branch.
> 
> > +  jit.arch.." architecture is NIY for memprof"
> > +)
> > +
> >  local tap = require("tap")
> >  
> >  local test = tap.test("misc-memprof-lapi")
> > -- 
> > 2.31.0
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

[Tarantool-patches] [PATCH luajit 3/4] ARM64: Fix xpcall() error case.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Thanks to Stefan Pejic.

(cherry picked from commit 33082a6f4778aa152f6a4a684a7fe79436f1ecb6)

Premature incrementing VM's BASE register before switch to fff_fallback
handler during processing `xpcall()` fast function leads to incorrect
L->base value in case, when `xpcall()` calls without a second argument
or if it equals nil (see <301-basic.t> test in lua-Harness test suite).
While further error processing it leads to crash, due to stack
inconsistency.

This patch moves BASE incrementing after possible switching to
fallback handler.

Sergey Kaplun:
* added the description for the problem

Part of tarantool/tarantool#6093
Part of tarantool/tarantool#5629
---
 src/vm_arm64.dasc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
index 6bf59509..e16a77ab 100644
--- a/src/vm_arm64.dasc
+++ b/src/vm_arm64.dasc
@@ -1186,12 +1186,12 @@ static void build_subroutines(BuildCtx *ctx)
   |   subs NARGS8:RC, NARGS8:RC, #16
   |   blo ->fff_fallback
   |    mov RB, BASE
-  |    add BASE, BASE, #24
   |     asr ITYPE, CARG2, #47
   |  ubfx TMP0w, TMP0w, #HOOK_ACTIVE_SHIFT, #1
   |     cmn ITYPE, #-LJ_TFUNC
   |  add PC, TMP0, #24+FRAME_PCALL
   |     bne ->fff_fallback		// Traceback must be a function.
+  |    add BASE, BASE, #24
   |     stp CARG2, CARG1, [RB]		// Swap function and traceback.
   |   cbz NARGS8:RC, ->vm_call_dispatch
   |  b <1
-- 
2.31.0

Re: [Tarantool-patches] [PATCH luajit 3/4] ARM64: Fix xpcall() error case.

[Sergey Ostanevich via Tarantool-patches]

Hi!

Thanks for the patch!

Some comments facelift, otherwise LGTM.

Sergos


> On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> From: Mike Pall <mike>	
> 
> Thanks to Stefan Pejic.
> 
> (cherry picked from commit 33082a6f4778aa152f6a4a684a7fe79436f1ecb6)
> 
> Premature incrementing VM's BASE register before switch to fff_fallback
	   increment of
> handler during processing `xpcall()` fast function leads to incorrect
> L->base value in case, when `xpcall()` calls without a second argument
					is called
> or if it equals nil (see <301-basic.t> test in lua-Harness test suite).
> While further error processing it leads to crash, due to stack
> inconsistency.

Please, mention explicitly if this test is the one for the patch.

> 
> This patch moves BASE incrementing after possible switching to
			increment          the switch (mentioned in first line)
> fallback handler.
the (aforementioned) 
> 
> Sergey Kaplun:
> * added the description for the problem
> 
> Part of tarantool/tarantool#6093
> Part of tarantool/tarantool#5629
> ---
> src/vm_arm64.dasc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
> index 6bf59509..e16a77ab 100644
> --- a/src/vm_arm64.dasc
> +++ b/src/vm_arm64.dasc
> @@ -1186,12 +1186,12 @@ static void build_subroutines(BuildCtx *ctx)
>   |   subs NARGS8:RC, NARGS8:RC, #16
>   |   blo ->fff_fallback
>   |    mov RB, BASE
> -  |    add BASE, BASE, #24
>   |     asr ITYPE, CARG2, #47
>   |  ubfx TMP0w, TMP0w, #HOOK_ACTIVE_SHIFT, #1
>   |     cmn ITYPE, #-LJ_TFUNC
>   |  add PC, TMP0, #24+FRAME_PCALL
>   |     bne ->fff_fallback		// Traceback must be a function.
> +  |    add BASE, BASE, #24
>   |     stp CARG2, CARG1, [RB]		// Swap function and traceback.
>   |   cbz NARGS8:RC, ->vm_call_dispatch
>   |  b <1
> -- 
> 2.31.0
> 

Re: [Tarantool-patches] [PATCH luajit 3/4] ARM64: Fix xpcall() error case.

[Sergey Kaplun via Tarantool-patches]

Hi!

Thanks for the review!

On 02.06.21, Sergey Ostanevich wrote:
> Hi!
> 
> Thanks for the patch!
> 
> Some comments facelift, otherwise LGTM.
> 
> Sergos
> 
> 
> > On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > From: Mike Pall <mike>	
> > 
> > Thanks to Stefan Pejic.
> > 
> > (cherry picked from commit 33082a6f4778aa152f6a4a684a7fe79436f1ecb6)
> > 
> > Premature incrementing VM's BASE register before switch to fff_fallback
> 	   increment of
> > handler during processing `xpcall()` fast function leads to incorrect
> > L->base value in case, when `xpcall()` calls without a second argument
> 					is called
> > or if it equals nil (see <301-basic.t> test in lua-Harness test suite).
> > While further error processing it leads to crash, due to stack
> > inconsistency.
> 
> Please, mention explicitly if this test is the one for the patch.
> 
> > 
> > This patch moves BASE incrementing after possible switching to
> 			increment          the switch (mentioned in first line)
> > fallback handler.
> the (aforementioned) 
> > 
> > Sergey Kaplun:
> > * added the description for the problem
> > 
> > Part of tarantool/tarantool#6093
> > Part of tarantool/tarantool#5629
> > ---

The new commit message is:
Branch is force-pushed.

===================================================================
ARM64: Fix xpcall() error case.

Thanks to Stefan Pejic.

(cherry picked from commit 33082a6f4778aa152f6a4a684a7fe79436f1ecb6)

Premature increment of VM's BASE register before switch to fff_fallback
handler during processing `xpcall()` fast function leads to incorrect
L->base value in the case, when `xpcall()` is called without a second
argument or if it equals nil (see <301-basic.t> test in lua-Harness test
suite). While further error processing it leads to crash (see the
test case in <test/lua-Harness-tests/301-basic.t:832>), due to stack
inconsistency.

This patch moves BASE increment after the switch (mentioned in the first
line) to the fallback handler (aforementioned).

Sergey Kaplun:
* added the description for the problem

Part of tarantool/tarantool#6093
Part of tarantool/tarantool#5629
===================================================================

> > src/vm_arm64.dasc | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
> > index 6bf59509..e16a77ab 100644
> > --- a/src/vm_arm64.dasc
> > +++ b/src/vm_arm64.dasc
> > @@ -1186,12 +1186,12 @@ static void build_subroutines(BuildCtx *ctx)
> >   |   subs NARGS8:RC, NARGS8:RC, #16
> >   |   blo ->fff_fallback
> >   |    mov RB, BASE
> > -  |    add BASE, BASE, #24
> >   |     asr ITYPE, CARG2, #47
> >   |  ubfx TMP0w, TMP0w, #HOOK_ACTIVE_SHIFT, #1
> >   |     cmn ITYPE, #-LJ_TFUNC
> >   |  add PC, TMP0, #24+FRAME_PCALL
> >   |     bne ->fff_fallback		// Traceback must be a function.
> > +  |    add BASE, BASE, #24
> >   |     stp CARG2, CARG1, [RB]		// Swap function and traceback.
> >   |   cbz NARGS8:RC, ->vm_call_dispatch
> >   |  b <1
> > -- 
> > 2.31.0
> > 
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 3/4] ARM64: Fix xpcall() error case.

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for the patch! LGTM, considering the changes made after Sergos
review comments.

-- 
Best regards,
IM

[Tarantool-patches] [PATCH luajit 4/4] ARM64: Fix xpcall() error case (really).

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Thanks to François Perrad and Stefan Pejic.

(cherry picked from commit d417ded17945b4211608d497d50b509e0274f5e0)

Premature decrementing VM's RC register before switch to fff_fallback
handler during processing `xpcall()` fast function leads to incorrect
stack layout (not enough arguments on stack), when `xpcall()` calls
without a second argument or if it is not a function (see <301-basic.t>
test in lua-Harness test suite). While further error processing it leads
to incorrect error message, due to stack inconsistency.

This patch stores intermediate result into TMP1 register (it does not
determine fallback's behaviour and there is no way to return from
fallback back to xpcall processing with spoiled TMP1) and moves RC
setting after possible switching to fallback handler.

Sergey Kaplun:
* added the description for the problem

Resolves tarantool/tarantool#6093
Part of tarantool/tarantool#5629
---
 src/vm_arm64.dasc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
index e16a77ab..6e298255 100644
--- a/src/vm_arm64.dasc
+++ b/src/vm_arm64.dasc
@@ -1183,7 +1183,7 @@ static void build_subroutines(BuildCtx *ctx)
   |.ffunc xpcall
   |     ldp CARG1, CARG2, [BASE]
   |  ldrb TMP0w, GL->hookmask
-  |   subs NARGS8:RC, NARGS8:RC, #16
+  |   subs NARGS8:TMP1, NARGS8:RC, #16
   |   blo ->fff_fallback
   |    mov RB, BASE
   |     asr ITYPE, CARG2, #47
@@ -1191,6 +1191,7 @@ static void build_subroutines(BuildCtx *ctx)
   |     cmn ITYPE, #-LJ_TFUNC
   |  add PC, TMP0, #24+FRAME_PCALL
   |     bne ->fff_fallback		// Traceback must be a function.
+  |   mov NARGS8:RC, NARGS8:TMP1
   |    add BASE, BASE, #24
   |     stp CARG2, CARG1, [RB]		// Swap function and traceback.
   |   cbz NARGS8:RC, ->vm_call_dispatch
-- 
2.31.0

Re: [Tarantool-patches] [PATCH luajit 4/4] ARM64: Fix xpcall() error case (really).

[Sergey Ostanevich via Tarantool-patches]

Hi!

Thanks for the patch!

Just some updates to the message, LGTM.

Sergos

> On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> From: Mike Pall <mike>
> 
> Thanks to François Perrad and Stefan Pejic.
> 
> (cherry picked from commit d417ded17945b4211608d497d50b509e0274f5e0)
> 
> Premature decrementing VM's RC register before switch to fff_fallback
	    decrement of

> handler during processing `xpcall()` fast function leads to incorrect
> stack layout (not enough arguments on stack), when `xpcall()` calls
> without a second argument or if it is not a function (see <301-basic.t>
> test in lua-Harness test suite). While further error processing it leads
> to incorrect error message, due to stack inconsistency.

Mention this test verifies the patch behavior.

> 
> This patch stores intermediate result into TMP1 register (it does not
> determine fallback's behaviour and there is no way to return from
> fallback back to xpcall processing with spoiled TMP1) and moves RC
> setting after possible switching to fallback handler.
		the switch            the

> 
> Sergey Kaplun:
> * added the description for the problem
> 
> Resolves tarantool/tarantool#6093
> Part of tarantool/tarantool#5629
> ---
> src/vm_arm64.dasc | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
> index e16a77ab..6e298255 100644
> --- a/src/vm_arm64.dasc
> +++ b/src/vm_arm64.dasc
> @@ -1183,7 +1183,7 @@ static void build_subroutines(BuildCtx *ctx)
>   |.ffunc xpcall
>   |     ldp CARG1, CARG2, [BASE]
>   |  ldrb TMP0w, GL->hookmask
> -  |   subs NARGS8:RC, NARGS8:RC, #16
> +  |   subs NARGS8:TMP1, NARGS8:RC, #16
>   |   blo ->fff_fallback
>   |    mov RB, BASE
>   |     asr ITYPE, CARG2, #47
> @@ -1191,6 +1191,7 @@ static void build_subroutines(BuildCtx *ctx)
>   |     cmn ITYPE, #-LJ_TFUNC
>   |  add PC, TMP0, #24+FRAME_PCALL
>   |     bne ->fff_fallback		// Traceback must be a function.
> +  |   mov NARGS8:RC, NARGS8:TMP1
>   |    add BASE, BASE, #24
>   |     stp CARG2, CARG1, [RB]		// Swap function and traceback.
>   |   cbz NARGS8:RC, ->vm_call_dispatch
> -- 
> 2.31.0
> 

Re: [Tarantool-patches] [PATCH luajit 4/4] ARM64: Fix xpcall() error case (really).

[Sergey Kaplun via Tarantool-patches]

Hi!

Thanks for the review!

On 02.06.21, Sergey Ostanevich wrote:
> Hi!
> 
> Thanks for the patch!
> 
> Just some updates to the message, LGTM.
> 
> Sergos
> 
> > On 24 May 2021, at 16:27, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > From: Mike Pall <mike>
> > 
> > Thanks to François Perrad and Stefan Pejic.
> > 
> > (cherry picked from commit d417ded17945b4211608d497d50b509e0274f5e0)
> > 
> > Premature decrementing VM's RC register before switch to fff_fallback
> 	    decrement of
> 
> > handler during processing `xpcall()` fast function leads to incorrect
> > stack layout (not enough arguments on stack), when `xpcall()` calls
> > without a second argument or if it is not a function (see <301-basic.t>
> > test in lua-Harness test suite). While further error processing it leads
> > to incorrect error message, due to stack inconsistency.
> 
> Mention this test verifies the patch behavior.
> 
> > 
> > This patch stores intermediate result into TMP1 register (it does not
> > determine fallback's behaviour and there is no way to return from
> > fallback back to xpcall processing with spoiled TMP1) and moves RC
> > setting after possible switching to fallback handler.
> 		the switch            the
> 
> > 
> > Sergey Kaplun:
> > * added the description for the problem
> > 
> > Resolves tarantool/tarantool#6093
> > Part of tarantool/tarantool#5629
> > ---

The new commit message is:
Branch is force-pushed.

===================================================================
ARM64: Fix xpcall() error case (really).

Thanks to François Perrad and Stefan Pejic.

(cherry picked from commit d417ded17945b4211608d497d50b509e0274f5e0)

Premature decrement of VM's RC register before switch to fff_fallback
handler during processing `xpcall()` fast function leads to incorrect
stack layout (not enough arguments on stack), when `xpcall()` calls
without a second argument or if it is not a function (see
<test/lua-Harness-tests/301-basic.t:832>
test in lua-Harness test suite). While further error processing it leads
to incorrect error message, due to stack inconsistency.

This patch stores intermediate result into TMP1 register (it does not
determine fallback's behaviour and there is no way to return from
fallback back to xpcall processing with spoiled TMP1) and moves RC
setting after the switch to the fallback handler.

Sergey Kaplun:
* added the description for the problem

Resolves tarantool/tarantool#6093
Part of tarantool/tarantool#5629
===================================================================

> > src/vm_arm64.dasc | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
> > index e16a77ab..6e298255 100644
> > --- a/src/vm_arm64.dasc
> > +++ b/src/vm_arm64.dasc
> > @@ -1183,7 +1183,7 @@ static void build_subroutines(BuildCtx *ctx)
> >   |.ffunc xpcall
> >   |     ldp CARG1, CARG2, [BASE]
> >   |  ldrb TMP0w, GL->hookmask
> > -  |   subs NARGS8:RC, NARGS8:RC, #16
> > +  |   subs NARGS8:TMP1, NARGS8:RC, #16
> >   |   blo ->fff_fallback
> >   |    mov RB, BASE
> >   |     asr ITYPE, CARG2, #47
> > @@ -1191,6 +1191,7 @@ static void build_subroutines(BuildCtx *ctx)
> >   |     cmn ITYPE, #-LJ_TFUNC
> >   |  add PC, TMP0, #24+FRAME_PCALL
> >   |     bne ->fff_fallback		// Traceback must be a function.
> > +  |   mov NARGS8:RC, NARGS8:TMP1
> >   |    add BASE, BASE, #24
> >   |     stp CARG2, CARG1, [RB]		// Swap function and traceback.
> >   |   cbz NARGS8:RC, ->vm_call_dispatch
> > -- 
> > 2.31.0
> > 
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 4/4] ARM64: Fix xpcall() error case (really).

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for the patch! There are still typos left after fixing Sergos
review comments (the commit message is taken from the branch).
Otherwise, LGTM.

| ARM64: Fix xpcall() error case (really).
|
| Thanks to François Perrad and Stefan Pejic.
|
| (cherry picked from commit d417ded17945b4211608d497d50b509e0274f5e0)
|
| Premature decrement of VM's RC register before switch to fff_fallback
| handler during processing `xpcall()` fast function leads to incorrect
| stack layout (not enough arguments on stack), when `xpcall()` calls

Typo: s/calls/is called/.

| without a second argument or if it is not a function (see
| <test/lua-Harness-tests/301-basic.t:832>

Typo: looks like line underfull (in LaTeX terms).

| test in lua-Harness test suite). While further error processing it leads
| to incorrect error message, due to stack inconsistency.
|
| This patch stores intermediate result into TMP1 register (it does not
| determine fallback's behaviour and there is no way to return from
| fallback back to xpcall processing with spoiled TMP1) and moves RC
| setting after the switch to the fallback handler.
|
| Sergey Kaplun:
| * added the description for the problem
|
| Resolves tarantool/tarantool#6093
| Part of tarantool/tarantool#5629

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 4/4] ARM64: Fix xpcall() error case (really).

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!

Thanks for the review!

On 10.06.21, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! There are still typos left after fixing Sergos
> review comments (the commit message is taken from the branch).
> Otherwise, LGTM.
> 
> | ARM64: Fix xpcall() error case (really).
> |
> | Thanks to François Perrad and Stefan Pejic.
> |
> | (cherry picked from commit d417ded17945b4211608d497d50b509e0274f5e0)
> |
> | Premature decrement of VM's RC register before switch to fff_fallback
> | handler during processing `xpcall()` fast function leads to incorrect
> | stack layout (not enough arguments on stack), when `xpcall()` calls
> 
> Typo: s/calls/is called/.
> 
> | without a second argument or if it is not a function (see
> | <test/lua-Harness-tests/301-basic.t:832>
> 
> Typo: looks like line underfull (in LaTeX terms).
> 
> | test in lua-Harness test suite). While further error processing it leads
> | to incorrect error message, due to stack inconsistency.
> |
> | This patch stores intermediate result into TMP1 register (it does not
> | determine fallback's behaviour and there is no way to return from
> | fallback back to xpcall processing with spoiled TMP1) and moves RC
> | setting after the switch to the fallback handler.
> |
> | Sergey Kaplun:
> | * added the description for the problem
> |
> | Resolves tarantool/tarantool#6093
> | Part of tarantool/tarantool#5629

Fixed. Branch is force-pushed:

The new commit message is:

===================================================================
ARM64: Fix xpcall() error case (really).

Thanks to François Perrad and Stefan Pejic.

(cherry picked from commit d417ded17945b4211608d497d50b509e0274f5e0)

Premature decrement of VM's RC register before switch to fff_fallback
handler during processing `xpcall()` fast function leads to incorrect
stack layout (not enough arguments on stack), when `xpcall()` is called
without a second argument or if it is not a function (see
<test/lua-Harness-tests/301-basic.t:832> test in lua-Harness test
suite). While further error processing it leads to incorrect error
message, due to stack inconsistency.

This patch stores intermediate result into TMP1 register (it does not
determine fallback's behaviour and there is no way to return from
fallback back to xpcall processing with spoiled TMP1) and moves RC
setting after the switch to the fallback handler.

Sergey Kaplun:
* added the description for the problem

Resolves tarantool/tarantool#6093
Part of tarantool/tarantool#5629
===================================================================

> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 0/4] Fix LuaJIT tests on aarch64, odroid

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for the series!

On 24.05.21, Sergey Kaplun wrote:
> This patchset fixes failing tests on odroid, aarch64.
> 
> Branch:
> https://github.com/tarantool/luajit/tree/skaplun/gh-6084-missed-carg1-in-bctsetr-fallback
> 
> Test branch:
> https://github.com/tarantool/tarantool/tree/skaplun/gh-6084-missed-carg1-in-bctsetr-fallback
> 
> Issues:
> * https://github.com/tarantool/tarantool/issues/5629
> * https://github.com/tarantool/tarantool/issues/6084
> * https://github.com/tarantool/tarantool/issues/6093
> 
> Honestly, I've failed to find a corresponding issue related to the first
> patch neither in the issue tracker, nor in the LuaJIT ML, so I've only
> referenced the original commit in the #6084.
> 
> For last two patches I don't add a new test case, because it is
> literally the same as the test in lua-Harness suite <301-basic.t:832>.

I guess this is not a problem at all, considering the fact Francois is
mentioned in the second patch. Unfortunately, the assertion you are
talking about is introduced within the "big bang" commit[1] with "some
tests" added, so I see no reason to refer such a huge changeset.

> 
> Mike Pall (3):
>   ARM, ARM64, PPC: Fix TSETR fallback.
>   ARM64: Fix xpcall() error case.
>   ARM64: Fix xpcall() error case (really).
> 
> Sergey Kaplun (1):
>   test: add skipcond on architectures for memprof
> 
>  src/vm_arm.dasc                               |  1 +
>  src/vm_arm64.dasc                             |  6 +++--
>  src/vm_ppc.dasc                               |  1 +
>  test/tarantool-tests/CMakeLists.txt           |  9 ++++---
>  ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
>  .../misclib-memprof-lapi.test.lua             |  6 +++++
>  test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
>  7 files changed, 64 insertions(+), 6 deletions(-)
>  create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> 
> -- 
> 2.31.0
> 

[1]: https://framagit.org/fperrad/lua-Harness/-/commit/48964e0

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 0/4] Fix LuaJIT tests on aarch64, odroid

[Igor Munkin via Tarantool-patches]

Sergey,

I've checked the patchset into tarantool branch in tarantool/luajit and
bumped a new version in master.

On 24.05.21, Sergey Kaplun wrote:
> This patchset fixes failing tests on odroid, aarch64.
> 
> Branch:
> https://github.com/tarantool/luajit/tree/skaplun/gh-6084-missed-carg1-in-bctsetr-fallback
> 
> Test branch:
> https://github.com/tarantool/tarantool/tree/skaplun/gh-6084-missed-carg1-in-bctsetr-fallback
> 
> Issues:
> * https://github.com/tarantool/tarantool/issues/5629
> * https://github.com/tarantool/tarantool/issues/6084
> * https://github.com/tarantool/tarantool/issues/6093
> 
> Honestly, I've failed to find a corresponding issue related to the first
> patch neither in the issue tracker, nor in the LuaJIT ML, so I've only
> referenced the original commit in the #6084.
> 
> For last two patches I don't add a new test case, because it is
> literally the same as the test in lua-Harness suite <301-basic.t:832>.
> 
> Mike Pall (3):
>   ARM, ARM64, PPC: Fix TSETR fallback.
>   ARM64: Fix xpcall() error case.
>   ARM64: Fix xpcall() error case (really).
> 
> Sergey Kaplun (1):
>   test: add skipcond on architectures for memprof
> 
>  src/vm_arm.dasc                               |  1 +
>  src/vm_arm64.dasc                             |  6 +++--
>  src/vm_ppc.dasc                               |  1 +
>  test/tarantool-tests/CMakeLists.txt           |  9 ++++---
>  ...-missed-carg1-in-bctsetr-fallback.test.lua | 25 +++++++++++++++++++
>  .../misclib-memprof-lapi.test.lua             |  6 +++++
>  test/tarantool-tests/utils.lua                | 22 ++++++++++++++++
>  7 files changed, 64 insertions(+), 6 deletions(-)
>  create mode 100644 test/tarantool-tests/gh-6084-missed-carg1-in-bctsetr-fallback.test.lua
> 
> -- 
> 2.31.0
> 

-- 
Best regards,
IM