[Tarantool-patches] [PATCH luajit 0/4] Fixes for ctypes with attributes

[Tarantool-patches] [PATCH luajit 0/4] Fixes for ctypes with attributes

[Sergey Kaplun via Tarantool-patches]

The first two patches fix the parsing and calculation for size and align
for C++ references. The last two patches fix the `ffi.metatype()`
behaviour for C types with attributes.

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-861-1005-ffi-fixes

Be aware that CI is red due to infrastructure problems with aarch64
hosts and being unable to download manifests for ecosystem integrational
CI.

Related issues:
* https://github.com/tarantool/tarantool/issues/9924
* https://github.com/LuaJIT/LuaJIT/issues/861
* https://github.com/LuaJIT/LuaJIT/issues/1005

Mike Pall (4):
  FFI: Fix ffi.alignof() for reference types.
  FFI: Fix sizeof expression in C parser for reference types.
  FFI: Allow ffi.metatype() for typedefs with attributes.
  FFI: Fix ffi.metatype() for non-raw types.

 src/lib_ffi.c                                 |  6 +--
 src/lj_cparse.c                               |  2 +-
 src/lj_ctype.c                                |  8 +++
 src/lj_ctype.h                                |  1 +
 .../lj-861-ctype-attributes.test.lua          | 53 +++++++++++++++++++
 5 files changed, 66 insertions(+), 4 deletions(-)
 create mode 100644 test/tarantool-tests/lj-861-ctype-attributes.test.lua

-- 
2.45.1

[Tarantool-patches] [PATCH luajit 1/4] FFI: Fix ffi.alignof() for reference types.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by Eric Gouyer.

(cherry picked from commit 36b2962d400db3981a7d7322f85c469240eb6f3b)

According to C++ Standard (5.3.6.3) [1], the `alignof()` for the
reference should be the same as for the referenced type. This patch
fixes the behaviour by following the reference to get a child id for
`ffi.alignof()`.

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

Part of tarantool/tarantool#9924

[1]: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/n4594.pdf#subsection.5.3.6
---
 src/lib_ffi.c                                 |  2 +-
 src/lj_ctype.c                                |  8 ++++++
 src/lj_ctype.h                                |  1 +
 .../lj-861-ctype-attributes.test.lua          | 26 +++++++++++++++++++
 4 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-861-ctype-attributes.test.lua

diff --git a/src/lib_ffi.c b/src/lib_ffi.c
index e60e7b19..2bfca25a 100644
--- a/src/lib_ffi.c
+++ b/src/lib_ffi.c
@@ -639,7 +639,7 @@ LJLIB_CF(ffi_alignof)	LJLIB_REC(ffi_xof FF_ffi_alignof)
   CTState *cts = ctype_cts(L);
   CTypeID id = ffi_checkctype(L, cts, NULL);
   CTSize sz = 0;
-  CTInfo info = lj_ctype_info(cts, id, &sz);
+  CTInfo info = lj_ctype_info_raw(cts, id, &sz);
   setintV(L->top-1, 1 << ctype_align(info));
   return 1;
 }
diff --git a/src/lj_ctype.c b/src/lj_ctype.c
index 0874fa61..83042118 100644
--- a/src/lj_ctype.c
+++ b/src/lj_ctype.c
@@ -345,6 +345,14 @@ CTInfo lj_ctype_info(CTState *cts, CTypeID id, CTSize *szp)
   return qual;
 }
 
+/* Ditto, but follow a reference. */
+CTInfo lj_ctype_info_raw(CTState *cts, CTypeID id, CTSize *szp)
+{
+  CType *ct = ctype_get(cts, id);
+  if (ctype_isref(ct->info)) id = ctype_cid(ct->info);
+  return lj_ctype_info(cts, id, szp);
+}
+
 /* Get ctype metamethod. */
 cTValue *lj_ctype_meta(CTState *cts, CTypeID id, MMS mm)
 {
diff --git a/src/lj_ctype.h b/src/lj_ctype.h
index fce29409..8edbd561 100644
--- a/src/lj_ctype.h
+++ b/src/lj_ctype.h
@@ -467,6 +467,7 @@ LJ_FUNC CType *lj_ctype_rawref(CTState *cts, CTypeID id);
 LJ_FUNC CTSize lj_ctype_size(CTState *cts, CTypeID id);
 LJ_FUNC CTSize lj_ctype_vlsize(CTState *cts, CType *ct, CTSize nelem);
 LJ_FUNC CTInfo lj_ctype_info(CTState *cts, CTypeID id, CTSize *szp);
+LJ_FUNC CTInfo lj_ctype_info_raw(CTState *cts, CTypeID id, CTSize *szp);
 LJ_FUNC cTValue *lj_ctype_meta(CTState *cts, CTypeID id, MMS mm);
 LJ_FUNC GCstr *lj_ctype_repr(lua_State *L, CTypeID id, GCstr *name);
 LJ_FUNC GCstr *lj_ctype_repr_int64(lua_State *L, uint64_t n, int isunsigned);
diff --git a/test/tarantool-tests/lj-861-ctype-attributes.test.lua b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
new file mode 100644
index 00000000..d88045a5
--- /dev/null
+++ b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
@@ -0,0 +1,26 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT incorrect behaviour during
+-- parsing and working with ctypes with attributes.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/861.
+
+local test = tap.test('lj-861-ctype-attributes')
+local ffi = require('ffi')
+
+test:plan(2)
+
+local EXPECTED_ALIGN = 4
+
+ffi.cdef([[
+struct __attribute__((aligned($))) s_aligned {
+  uint8_t a;
+};
+]], EXPECTED_ALIGN)
+
+local ref_align = ffi.alignof(ffi.typeof('struct s_aligned &'))
+
+test:is(ref_align, EXPECTED_ALIGN, 'the reference alignment is correct')
+test:is(ref_align, ffi.alignof(ffi.typeof('struct s_aligned')),
+        'the alignment of a reference is the same as for the referenced type')
+
+test:done(true)
-- 
2.45.1

Re: [Tarantool-patches] [PATCH luajit 1/4] FFI: Fix ffi.alignof() for reference types.

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey


thanks for the patch! LGTM


On 03.06.2024 17:33, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Eric Gouyer.
>
> (cherry picked from commit 36b2962d400db3981a7d7322f85c469240eb6f3b)
>
> According to C++ Standard (5.3.6.3) [1], the `alignof()` for the
> reference should be the same as for the referenced type. This patch
> fixes the behaviour by following the reference to get a child id for
> `ffi.alignof()`.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9924
>
> [1]:https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/n4594.pdf#subsection.5.3.6
> ---
>   src/lib_ffi.c                                 |  2 +-
>   src/lj_ctype.c                                |  8 ++++++
>   src/lj_ctype.h                                |  1 +
>   .../lj-861-ctype-attributes.test.lua          | 26 +++++++++++++++++++
>   4 files changed, 36 insertions(+), 1 deletion(-)
>   create mode 100644 test/tarantool-tests/lj-861-ctype-attributes.test.lua
>
> diff --git a/src/lib_ffi.c b/src/lib_ffi.c
> index e60e7b19..2bfca25a 100644
> --- a/src/lib_ffi.c
> +++ b/src/lib_ffi.c
> @@ -639,7 +639,7 @@ LJLIB_CF(ffi_alignof)	LJLIB_REC(ffi_xof FF_ffi_alignof)
>     CTState *cts = ctype_cts(L);
>     CTypeID id = ffi_checkctype(L, cts, NULL);
>     CTSize sz = 0;
> -  CTInfo info = lj_ctype_info(cts, id, &sz);
> +  CTInfo info = lj_ctype_info_raw(cts, id, &sz);
>     setintV(L->top-1, 1 << ctype_align(info));
>     return 1;
>   }
> diff --git a/src/lj_ctype.c b/src/lj_ctype.c
> index 0874fa61..83042118 100644
> --- a/src/lj_ctype.c
> +++ b/src/lj_ctype.c
> @@ -345,6 +345,14 @@ CTInfo lj_ctype_info(CTState *cts, CTypeID id, CTSize *szp)
>     return qual;
>   }
>   
> +/* Ditto, but follow a reference. */
> +CTInfo lj_ctype_info_raw(CTState *cts, CTypeID id, CTSize *szp)
> +{
> +  CType *ct = ctype_get(cts, id);
> +  if (ctype_isref(ct->info)) id = ctype_cid(ct->info);
> +  return lj_ctype_info(cts, id, szp);
> +}
> +
>   /* Get ctype metamethod. */
>   cTValue *lj_ctype_meta(CTState *cts, CTypeID id, MMS mm)
>   {
> diff --git a/src/lj_ctype.h b/src/lj_ctype.h
> index fce29409..8edbd561 100644
> --- a/src/lj_ctype.h
> +++ b/src/lj_ctype.h
> @@ -467,6 +467,7 @@ LJ_FUNC CType *lj_ctype_rawref(CTState *cts, CTypeID id);
>   LJ_FUNC CTSize lj_ctype_size(CTState *cts, CTypeID id);
>   LJ_FUNC CTSize lj_ctype_vlsize(CTState *cts, CType *ct, CTSize nelem);
>   LJ_FUNC CTInfo lj_ctype_info(CTState *cts, CTypeID id, CTSize *szp);
> +LJ_FUNC CTInfo lj_ctype_info_raw(CTState *cts, CTypeID id, CTSize *szp);
>   LJ_FUNC cTValue *lj_ctype_meta(CTState *cts, CTypeID id, MMS mm);
>   LJ_FUNC GCstr *lj_ctype_repr(lua_State *L, CTypeID id, GCstr *name);
>   LJ_FUNC GCstr *lj_ctype_repr_int64(lua_State *L, uint64_t n, int isunsigned);
> diff --git a/test/tarantool-tests/lj-861-ctype-attributes.test.lua b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> new file mode 100644
> index 00000000..d88045a5
> --- /dev/null
> +++ b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> @@ -0,0 +1,26 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate LuaJIT incorrect behaviour during
> +-- parsing and working with ctypes with attributes.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/861.
> +
> +local test = tap.test('lj-861-ctype-attributes')
> +local ffi = require('ffi')
> +
> +test:plan(2)
> +
> +local EXPECTED_ALIGN = 4
> +
> +ffi.cdef([[
> +struct __attribute__((aligned($))) s_aligned {
> +  uint8_t a;
> +};
> +]], EXPECTED_ALIGN)
> +
> +local ref_align = ffi.alignof(ffi.typeof('struct s_aligned &'))
> +
> +test:is(ref_align, EXPECTED_ALIGN, 'the reference alignment is correct')
> +test:is(ref_align, ffi.alignof(ffi.typeof('struct s_aligned')),
> +        'the alignment of a reference is the same as for the referenced type')
> +
> +test:done(true)

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

Re: [Tarantool-patches] [PATCH luajit 1/4] FFI: Fix ffi.alignof() for reference types.

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM

[Tarantool-patches] [PATCH luajit 2/4] FFI: Fix sizeof expression in C parser for reference types.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

(cherry picked from commit 899093a9e0fa5b16f27016381ef4b15529dadff2)

According to C++ Standard, the `alignof()` (5.3.6.3) [1] and `sizeof()`
(5.3.3.2) [2] for the reference should be the same as for the referenced
type. This patch fixes the behaviour by following the reference to get a
child id for `alignof()` and `sizeof()` while parsing C definitions via
`ffi.cdef()`.

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

Part of tarantool/tarantool#9924

[1]: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/n4594.pdf#subsection.5.3.6
[2]: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/n4594.pdf#subsection.5.3.3
---
 src/lj_cparse.c                                  |  2 +-
 .../lj-861-ctype-attributes.test.lua             | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/lj_cparse.c b/src/lj_cparse.c
index 01deb3bf..8506d719 100644
--- a/src/lj_cparse.c
+++ b/src/lj_cparse.c
@@ -468,7 +468,7 @@ static void cp_expr_sizeof(CPState *cp, CPValue *k, int wantsz)
   } else {
     cp_expr_unary(cp, k);
   }
-  info = lj_ctype_info(cp->cts, k->id, &sz);
+  info = lj_ctype_info_raw(cp->cts, k->id, &sz);
   if (wantsz) {
     if (sz != CTSIZE_INVALID)
       k->u32 = sz;
diff --git a/test/tarantool-tests/lj-861-ctype-attributes.test.lua b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
index d88045a5..e8b29d67 100644
--- a/test/tarantool-tests/lj-861-ctype-attributes.test.lua
+++ b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
@@ -7,7 +7,7 @@ local tap = require('tap')
 local test = tap.test('lj-861-ctype-attributes')
 local ffi = require('ffi')
 
-test:plan(2)
+test:plan(4)
 
 local EXPECTED_ALIGN = 4
 
@@ -15,6 +15,15 @@ ffi.cdef([[
 struct __attribute__((aligned($))) s_aligned {
   uint8_t a;
 };
+
+struct test_parsing_sizeof {
+  char a[sizeof(struct s_aligned &)];
+};
+
+struct test_parsing_alignof {
+  char a[__alignof__(struct s_aligned &)];
+};
+
 ]], EXPECTED_ALIGN)
 
 local ref_align = ffi.alignof(ffi.typeof('struct s_aligned &'))
@@ -23,4 +32,9 @@ test:is(ref_align, EXPECTED_ALIGN, 'the reference alignment is correct')
 test:is(ref_align, ffi.alignof(ffi.typeof('struct s_aligned')),
         'the alignment of a reference is the same as for the referenced type')
 
+test:is(ffi.sizeof('struct test_parsing_sizeof'), EXPECTED_ALIGN,
+        'correct sizeof during C parsing')
+test:is(ffi.sizeof('struct test_parsing_alignof'), EXPECTED_ALIGN,
+        'correct alignof during C parsing')
+
 test:done(true)
-- 
2.45.1

Re: [Tarantool-patches] [PATCH luajit 2/4] FFI: Fix sizeof expression in C parser for reference types.

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey

thanks for the patch! LGTM

On 03.06.2024 17:34, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> (cherry picked from commit 899093a9e0fa5b16f27016381ef4b15529dadff2)
>
> According to C++ Standard, the `alignof()` (5.3.6.3) [1] and `sizeof()`
> (5.3.3.2) [2] for the reference should be the same as for the referenced
> type. This patch fixes the behaviour by following the reference to get a
> child id for `alignof()` and `sizeof()` while parsing C definitions via
> `ffi.cdef()`.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9924
>
> [1]:https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/n4594.pdf#subsection.5.3.6
> [2]:https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/n4594.pdf#subsection.5.3.3
> ---
>   src/lj_cparse.c                                  |  2 +-
>   .../lj-861-ctype-attributes.test.lua             | 16 +++++++++++++++-
>   2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/src/lj_cparse.c b/src/lj_cparse.c
> index 01deb3bf..8506d719 100644
> --- a/src/lj_cparse.c
> +++ b/src/lj_cparse.c
> @@ -468,7 +468,7 @@ static void cp_expr_sizeof(CPState *cp, CPValue *k, int wantsz)
>     } else {
>       cp_expr_unary(cp, k);
>     }
> -  info = lj_ctype_info(cp->cts, k->id, &sz);
> +  info = lj_ctype_info_raw(cp->cts, k->id, &sz);
>     if (wantsz) {
>       if (sz != CTSIZE_INVALID)
>         k->u32 = sz;
> diff --git a/test/tarantool-tests/lj-861-ctype-attributes.test.lua b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> index d88045a5..e8b29d67 100644
> --- a/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> +++ b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> @@ -7,7 +7,7 @@ local tap = require('tap')
>   local test = tap.test('lj-861-ctype-attributes')
>   local ffi = require('ffi')
>   
> -test:plan(2)
> +test:plan(4)
>   
>   local EXPECTED_ALIGN = 4
>   
> @@ -15,6 +15,15 @@ ffi.cdef([[
>   struct __attribute__((aligned($))) s_aligned {
>     uint8_t a;
>   };
> +
> +struct test_parsing_sizeof {
> +  char a[sizeof(struct s_aligned &)];
> +};
> +
> +struct test_parsing_alignof {
> +  char a[__alignof__(struct s_aligned &)];
> +};
> +
>   ]], EXPECTED_ALIGN)
>   
>   local ref_align = ffi.alignof(ffi.typeof('struct s_aligned &'))
> @@ -23,4 +32,9 @@ test:is(ref_align, EXPECTED_ALIGN, 'the reference alignment is correct')
>   test:is(ref_align, ffi.alignof(ffi.typeof('struct s_aligned')),
>           'the alignment of a reference is the same as for the referenced type')
>   
> +test:is(ffi.sizeof('struct test_parsing_sizeof'), EXPECTED_ALIGN,
> +        'correct sizeof during C parsing')
> +test:is(ffi.sizeof('struct test_parsing_alignof'), EXPECTED_ALIGN,
> +        'correct alignof during C parsing')
> +
>   test:done(true)

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

Re: [Tarantool-patches] [PATCH luajit 2/4] FFI: Fix sizeof expression in C parser for reference types.

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM

[Tarantool-patches] [PATCH luajit 3/4] FFI: Allow ffi.metatype() for typedefs with attributes.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by Eric Gouyer.

(cherry picked from commit 27a6fee82e91319bbed8c3aa443aa2b0f1b0a470)

Before the patch, `ffi.metatype()` raises an error when calling it on
ctype with attributes.  This patch fixes the behaviour by using
`ctype_raw()` instead of `ctype_get()` to follow child ctypes unless
there are no attributes on the ctype.

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

Part of tarantool/tarantool#9924
---
 src/lib_ffi.c                                         |  2 +-
 test/tarantool-tests/lj-861-ctype-attributes.test.lua | 10 +++++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/lib_ffi.c b/src/lib_ffi.c
index 2bfca25a..dfb01658 100644
--- a/src/lib_ffi.c
+++ b/src/lib_ffi.c
@@ -770,7 +770,7 @@ LJLIB_CF(ffi_metatype)
   CTypeID id = ffi_checkctype(L, cts, NULL);
   GCtab *mt = lj_lib_checktab(L, 2);
   GCtab *t = cts->miscmap;
-  CType *ct = ctype_get(cts, id);  /* Only allow raw types. */
+  CType *ct = ctype_raw(cts, id);
   TValue *tv;
   GCcdata *cd;
   if (!(ctype_isstruct(ct->info) || ctype_iscomplex(ct->info) ||
diff --git a/test/tarantool-tests/lj-861-ctype-attributes.test.lua b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
index e8b29d67..a407fae0 100644
--- a/test/tarantool-tests/lj-861-ctype-attributes.test.lua
+++ b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
@@ -7,14 +7,14 @@ local tap = require('tap')
 local test = tap.test('lj-861-ctype-attributes')
 local ffi = require('ffi')
 
-test:plan(4)
+test:plan(5)
 
 local EXPECTED_ALIGN = 4
 
 ffi.cdef([[
-struct __attribute__((aligned($))) s_aligned {
+typedef struct __attribute__((aligned($))) s_aligned {
   uint8_t a;
-};
+} s_aligned;
 
 struct test_parsing_sizeof {
   char a[sizeof(struct s_aligned &)];
@@ -37,4 +37,8 @@ test:is(ffi.sizeof('struct test_parsing_sizeof'), EXPECTED_ALIGN,
 test:is(ffi.sizeof('struct test_parsing_alignof'), EXPECTED_ALIGN,
         'correct alignof during C parsing')
 
+local ok, _ = pcall(ffi.metatype, 's_aligned', {})
+
+test:ok(ok, 'ffi.metatype is called at the structure with attributes')
+
 test:done(true)
-- 
2.45.1

Re: [Tarantool-patches] [PATCH luajit 3/4] FFI: Allow ffi.metatype() for typedefs with attributes.

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey

thanks for the patch! LGTM with a nit below.

On 03.06.2024 17:34, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Eric Gouyer.
>
> (cherry picked from commit 27a6fee82e91319bbed8c3aa443aa2b0f1b0a470)
>
> Before the patch, `ffi.metatype()` raises an error when calling it on
> ctype with attributes.  This patch fixes the behaviour by using
extra whitespace above
> `ctype_raw()` instead of `ctype_get()` to follow child ctypes unless
> there are no attributes on the ctype.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9924
> ---
>   src/lib_ffi.c                                         |  2 +-
>   test/tarantool-tests/lj-861-ctype-attributes.test.lua | 10 +++++++---
>   2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/lib_ffi.c b/src/lib_ffi.c
> index 2bfca25a..dfb01658 100644
> --- a/src/lib_ffi.c
> +++ b/src/lib_ffi.c
> @@ -770,7 +770,7 @@ LJLIB_CF(ffi_metatype)
>     CTypeID id = ffi_checkctype(L, cts, NULL);
>     GCtab *mt = lj_lib_checktab(L, 2);
>     GCtab *t = cts->miscmap;
> -  CType *ct = ctype_get(cts, id);  /* Only allow raw types. */
> +  CType *ct = ctype_raw(cts, id);
>     TValue *tv;
>     GCcdata *cd;
>     if (!(ctype_isstruct(ct->info) || ctype_iscomplex(ct->info) ||
> diff --git a/test/tarantool-tests/lj-861-ctype-attributes.test.lua b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> index e8b29d67..a407fae0 100644
> --- a/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> +++ b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> @@ -7,14 +7,14 @@ local tap = require('tap')
>   local test = tap.test('lj-861-ctype-attributes')
>   local ffi = require('ffi')
>   
> -test:plan(4)
> +test:plan(5)
>   
>   local EXPECTED_ALIGN = 4
>   
>   ffi.cdef([[
> -struct __attribute__((aligned($))) s_aligned {
> +typedef struct __attribute__((aligned($))) s_aligned {
>     uint8_t a;
> -};
> +} s_aligned;
>   
>   struct test_parsing_sizeof {
>     char a[sizeof(struct s_aligned &)];
> @@ -37,4 +37,8 @@ test:is(ffi.sizeof('struct test_parsing_sizeof'), EXPECTED_ALIGN,
>   test:is(ffi.sizeof('struct test_parsing_alignof'), EXPECTED_ALIGN,
>           'correct alignof during C parsing')
>   
> +local ok, _ = pcall(ffi.metatype, 's_aligned', {})
> +
> +test:ok(ok, 'ffi.metatype is called at the structure with attributes')
> +
>   test:done(true)

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

Re: [Tarantool-patches] [PATCH luajit 3/4] FFI: Allow ffi.metatype() for typedefs with attributes.

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the review!
Fixed your comment and force-pushed the branch.

On 11.06.24, Sergey Bronnikov wrote:
> Hi, Sergey
> 
> thanks for the patch! LGTM with a nit below.
> 
> On 03.06.2024 17:34, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Reported by Eric Gouyer.
> >
> > (cherry picked from commit 27a6fee82e91319bbed8c3aa443aa2b0f1b0a470)
> >
> > Before the patch, `ffi.metatype()` raises an error when calling it on
> > ctype with attributes.  This patch fixes the behaviour by using
> extra whitespace above
> > `ctype_raw()` instead of `ctype_get()` to follow child ctypes unless
> > there are no attributes on the ctype.
> >
> > Sergey Kaplun:
> > * added the description and the test for the problem
> >
> > Part of tarantool/tarantool#9924
> > ---

<snipped>

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 3/4] FFI: Allow ffi.metatype() for typedefs with attributes.

[Sergey Bronnikov via Tarantool-patches]

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


On 13.06.2024 10:34, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Fixed your comment and force-pushed the branch.
Thanks! LGTM as I said before.
>
> On 11.06.24, Sergey Bronnikov wrote:
>> Hi, Sergey
>>
>> thanks for the patch! LGTM with a nit below.
>>
>> On 03.06.2024 17:34, Sergey Kaplun wrote:
>>> From: Mike Pall <mike>
>>>
>>> Reported by Eric Gouyer.
>>>
>>> (cherry picked from commit 27a6fee82e91319bbed8c3aa443aa2b0f1b0a470)
>>>
>>> Before the patch, `ffi.metatype()` raises an error when calling it on
>>> ctype with attributes.  This patch fixes the behaviour by using
>> extra whitespace above
>>> `ctype_raw()` instead of `ctype_get()` to follow child ctypes unless
>>> there are no attributes on the ctype.
>>>
>>> Sergey Kaplun:
>>> * added the description and the test for the problem
>>>
>>> Part of tarantool/tarantool#9924
>>> ---
> <snipped>
>

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

Re: [Tarantool-patches] [PATCH luajit 3/4] FFI: Allow ffi.metatype() for typedefs with attributes.

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM

[Tarantool-patches] [PATCH luajit 4/4] FFI: Fix ffi.metatype() for non-raw types.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by 999pingGG.

(cherry picked from commit 4c35a42d9cd3b8be98fa419c68e4724ccdf4f2cf)

This patch is a follow-up to the previous commit. Since the child id is
used for the index in the lookup table, it has no effect, and metatype
methods are not overloaded. This patch fixes the behaviour by using the
correct index.

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

Part of tarantool/tarantool#9924
---
 src/lib_ffi.c                                     |  2 +-
 .../lj-861-ctype-attributes.test.lua              | 15 ++++++++++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/lib_ffi.c b/src/lib_ffi.c
index dfb01658..7ed6fc78 100644
--- a/src/lib_ffi.c
+++ b/src/lib_ffi.c
@@ -776,7 +776,7 @@ LJLIB_CF(ffi_metatype)
   if (!(ctype_isstruct(ct->info) || ctype_iscomplex(ct->info) ||
 	ctype_isvector(ct->info)))
     lj_err_arg(L, 1, LJ_ERR_FFI_INVTYPE);
-  tv = lj_tab_setinth(L, t, -(int32_t)id);
+  tv = lj_tab_setinth(L, t, -(int32_t)ctype_typeid(cts, ct));
   if (!tvisnil(tv))
     lj_err_caller(L, LJ_ERR_PROTMT);
   settabV(L, tv, mt);
diff --git a/test/tarantool-tests/lj-861-ctype-attributes.test.lua b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
index a407fae0..79121cdb 100644
--- a/test/tarantool-tests/lj-861-ctype-attributes.test.lua
+++ b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
@@ -2,12 +2,14 @@ local tap = require('tap')
 
 -- Test file to demonstrate LuaJIT incorrect behaviour during
 -- parsing and working with ctypes with attributes.
--- See also: https://github.com/LuaJIT/LuaJIT/issues/861.
+-- See also:
+-- * https://github.com/LuaJIT/LuaJIT/issues/861,
+-- * https://github.com/LuaJIT/LuaJIT/issues/1005.
 
 local test = tap.test('lj-861-ctype-attributes')
 local ffi = require('ffi')
 
-test:plan(5)
+test:plan(6)
 
 local EXPECTED_ALIGN = 4
 
@@ -37,8 +39,15 @@ test:is(ffi.sizeof('struct test_parsing_sizeof'), EXPECTED_ALIGN,
 test:is(ffi.sizeof('struct test_parsing_alignof'), EXPECTED_ALIGN,
         'correct alignof during C parsing')
 
-local ok, _ = pcall(ffi.metatype, 's_aligned', {})
+local EXPECTED_TOSTR = '__tostring overloaded'
+local ok, obj = pcall(ffi.metatype, 's_aligned', {
+  __tostring = function()
+    return EXPECTED_TOSTR
+  end,
+})
 
 test:ok(ok, 'ffi.metatype is called at the structure with attributes')
+test:is(tostring(obj()), EXPECTED_TOSTR,
+        '__tostring is overloaded for the structure with attributes')
 
 test:done(true)
-- 
2.45.1

Re: [Tarantool-patches] [PATCH luajit 4/4] FFI: Fix ffi.metatype() for non-raw types.

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey


thanks for the patch! LGTM

On 03.06.2024 17:34, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by 999pingGG.
>
> (cherry picked from commit 4c35a42d9cd3b8be98fa419c68e4724ccdf4f2cf)
>
> This patch is a follow-up to the previous commit. Since the child id is
> used for the index in the lookup table, it has no effect, and metatype
> methods are not overloaded. This patch fixes the behaviour by using the
> correct index.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9924
> ---
>   src/lib_ffi.c                                     |  2 +-
>   .../lj-861-ctype-attributes.test.lua              | 15 ++++++++++++---
>   2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/src/lib_ffi.c b/src/lib_ffi.c
> index dfb01658..7ed6fc78 100644
> --- a/src/lib_ffi.c
> +++ b/src/lib_ffi.c
> @@ -776,7 +776,7 @@ LJLIB_CF(ffi_metatype)
>     if (!(ctype_isstruct(ct->info) || ctype_iscomplex(ct->info) ||
>   	ctype_isvector(ct->info)))
>       lj_err_arg(L, 1, LJ_ERR_FFI_INVTYPE);
> -  tv = lj_tab_setinth(L, t, -(int32_t)id);
> +  tv = lj_tab_setinth(L, t, -(int32_t)ctype_typeid(cts, ct));
>     if (!tvisnil(tv))
>       lj_err_caller(L, LJ_ERR_PROTMT);
>     settabV(L, tv, mt);
> diff --git a/test/tarantool-tests/lj-861-ctype-attributes.test.lua b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> index a407fae0..79121cdb 100644
> --- a/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> +++ b/test/tarantool-tests/lj-861-ctype-attributes.test.lua
> @@ -2,12 +2,14 @@ local tap = require('tap')
>   
>   -- Test file to demonstrate LuaJIT incorrect behaviour during
>   -- parsing and working with ctypes with attributes.
> --- See also:https://github.com/LuaJIT/LuaJIT/issues/861.
> +-- See also:
> +-- *https://github.com/LuaJIT/LuaJIT/issues/861,
> +-- *https://github.com/LuaJIT/LuaJIT/issues/1005.
>   
>   local test = tap.test('lj-861-ctype-attributes')
>   local ffi = require('ffi')
>   
> -test:plan(5)
> +test:plan(6)
>   
>   local EXPECTED_ALIGN = 4
>   
> @@ -37,8 +39,15 @@ test:is(ffi.sizeof('struct test_parsing_sizeof'), EXPECTED_ALIGN,
>   test:is(ffi.sizeof('struct test_parsing_alignof'), EXPECTED_ALIGN,
>           'correct alignof during C parsing')
>   
> -local ok, _ = pcall(ffi.metatype, 's_aligned', {})
> +local EXPECTED_TOSTR = '__tostring overloaded'
> +local ok, obj = pcall(ffi.metatype, 's_aligned', {
> +  __tostring = function()
> +    return EXPECTED_TOSTR
> +  end,
> +})
>   
>   test:ok(ok, 'ffi.metatype is called at the structure with attributes')
> +test:is(tostring(obj()), EXPECTED_TOSTR,
> +        '__tostring is overloaded for the structure with attributes')
>   
>   test:done(true)

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

Re: [Tarantool-patches] [PATCH luajit 4/4] FFI: Fix ffi.metatype() for non-raw types.

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM

Re: [Tarantool-patches] [PATCH luajit 0/4] Fixes for ctypes with attributes

[Sergey Kaplun via Tarantool-patches]

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master [1], release/3.1 [2]
and release/2.11 [3].

[1]: https://github.com/tarantool/tarantool/pull/10200
[2]: https://github.com/tarantool/tarantool/pull/10201
[3]: https://github.com/tarantool/tarantool/pull/10202

-- 
Best regards,
Sergey Kaplun