[Tarantool-patches] [PATCH luajit] FFI: Always fall back to metamethods for cdata length/concat.
sergos
sergos at tarantool.org
Wed Aug 31 13:02:55 MSK 2022
Hi!
Thanks for the patch!
Just a minor message update and a test extension - otherwise LGTM.
Sergos
> On 23 Aug 2022, at 17:27, Sergey Kaplun <skaplun at tarantool.org> wrote:
>
> From: Mike Pall <mike>
>
> Thanks to Egor Skriptunoff.
>
> (cherry picked from commit cc4bbec483d3f3250b519ccb7cc22f1a8e6fe6f0)
>
> When user tries to concatenate 2 cdata objects without declared
> metamethod, the assertion is raised in `carith_int64()`, due to
> concatenation operation is not specified and default (assert) branch is
> taken.
The above doesn’t explain the behavior - the default branch leads to what?
Neither it explains the expected behavior.
>
> This patch forcifies usage of metamethod for concatenation on cdata
> objects. Also, as far as the behaviour for length operation is the same,
> the `lj_carith_len()` routine is removed, its call is replaced with
> `ffi_arith()`.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#7230
> ---
>
> Issue: https://github.com/tarantool/tarantool/issues/7230
> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-cdata-ll-concat-full-ci
> PR: https://github.com/tarantool/tarantool/pull/7598
> ML: https://www.freelists.org/post/luajit/cdata-concatenation
>
> src/lj_carith.c | 3 +--
> src/lj_crecord.c | 6 ++++--
> test/tarantool-tests/fix-cdata-concat.test.lua | 15 +++++++++++++++
> 3 files changed, 20 insertions(+), 4 deletions(-)
> create mode 100644 test/tarantool-tests/fix-cdata-concat.test.lua
>
> diff --git a/src/lj_carith.c b/src/lj_carith.c
> index 218abd26..04c18054 100644
> --- a/src/lj_carith.c
> +++ b/src/lj_carith.c
> @@ -265,7 +265,7 @@ int lj_carith_op(lua_State *L, MMS mm)
> {
> CTState *cts = ctype_cts(L);
> CDArith ca;
> - if (carith_checkarg(L, cts, &ca)) {
> + if (carith_checkarg(L, cts, &ca) && mm != MM_len && mm != MM_concat) {
> if (carith_int64(L, cts, &ca, mm) || carith_ptr(L, cts, &ca, mm)) {
> copyTV(L, &G(L)->tmptv2, L->top-1); /* Remember for trace recorder. */
> return 1;
> @@ -347,7 +347,6 @@ uint64_t lj_carith_check64(lua_State *L, int narg, CTypeID *id)
> }
> }
>
> -
> /* -- 64 bit integer arithmetic helpers ----------------------------------- */
>
> #if LJ_32 && LJ_HASJIT
> diff --git a/src/lj_crecord.c b/src/lj_crecord.c
> index 0d7b71f0..3d562d9a 100644
> --- a/src/lj_crecord.c
> +++ b/src/lj_crecord.c
> @@ -1546,8 +1546,10 @@ void LJ_FASTCALL recff_cdata_arith(jit_State *J, RecordFFData *rd)
> }
> {
> TRef tr;
> - if (!(tr = crec_arith_int64(J, sp, s, (MMS)rd->data)) &&
> - !(tr = crec_arith_ptr(J, sp, s, (MMS)rd->data)) &&
> + MMS mm = (MMS)rd->data;
> + if ((mm == MM_len || mm == MM_concat ||
> + (!(tr = crec_arith_int64(J, sp, s, mm)) &&
> + !(tr = crec_arith_ptr(J, sp, s, mm)))) &&
> !(tr = crec_arith_meta(J, sp, s, cts, rd)))
> return;
> J->base[0] = tr;
> diff --git a/test/tarantool-tests/fix-cdata-concat.test.lua b/test/tarantool-tests/fix-cdata-concat.test.lua
> new file mode 100644
> index 00000000..aaeb36fa
> --- /dev/null
> +++ b/test/tarantool-tests/fix-cdata-concat.test.lua
> @@ -0,0 +1,15 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate incorrect behaviour of cdata
> +-- concatenation in LuaJIT.
> +-- See also
> +-- https://www.freelists.org/post/luajit/cdata-concatenation.
> +local test = tap.test('cdata-concat')
> +test:plan(1)
> +
> +local r, e = pcall(function()
> + return 1LL .. 2LL
> +end)
> +test:ok(not r and e:match('attempt to concatenate'), 'cdata concatenation')
As with programmer’s joke about full and empty glass - let’s have a second case
for the existent metamethod?
> +
> +os.exit(test:check() and 0 or 1)
> --
> 2.34.1
>
More information about the Tarantool-patches
mailing list