From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id C777E6EC40; Mon, 1 Jun 2026 15:27:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C777E6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1780316827; bh=iRc4zHXY1xmvCyPv7y/EtV7n3wyhbuYpXlNLeji8OeI=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=nm3UzOmBZZRnbOQS6RNInOng26+DVXegl16lEJvddzLPHsRYiT24shGdhT6UXvlDE OBQvGydJE/2i+ImeDSEyOjg9E103BmkBUqRZOzyuAjlw1wLOo39GAE1a2iJyBT7UlN bmEqyPYm6QbhU9EGDXGrCfC/hSk3qBzHRDaLV97I= Received: from send59.i.mail.ru (send59.i.mail.ru [89.221.237.154]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 5CBFC6EC40 for ; Mon, 1 Jun 2026 15:27:06 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5CBFC6EC40 Received: by exim-smtp-5b85998476-95fct with esmtpa (envelope-from ) id 1wU1jU-000000006zd-43ms; Mon, 01 Jun 2026 15:27:05 +0300 Content-Type: multipart/alternative; boundary="------------gCfzzdhcxZlTdQjXy0zBnrjv" Message-ID: Date: Mon, 1 Jun 2026 15:27:03 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun , Evgeniy Temirgaleev Cc: tarantool-patches@dev.tarantool.org References: <20260530160409.4043089-1-skaplun@tarantool.org> <20260530160409.4043089-4-skaplun@tarantool.org> In-Reply-To: <20260530160409.4043089-4-skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9402BF17F4A9A44D6B5B9AAD00C530C650B6B4D9F3374708F00894C459B0CD1B9A41FE20C938AD9242EB5D77EF37489D11B33F5E734D8CEC117E6B02B67B216808472923344694FB0 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7956F10FFCC7409BAEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB55337566EB41390B1F37380D4CAD33BD9800BE50EBDFDB182F1D5F32E771E55800CEA0A7389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0A29E2F051442AF778941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6E5E764EB5D94DBD4CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE788761406820A3098731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5B1DA85DEFE869A595002B1117B3ED696A8F4AFF3435F4CD4B91D2EB2DEE3878C823CB91A9FED034534781492E4B8EEADA3FB0D9844EF8EC5BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0AD73CAD6646DEDE191716CD42B3DD1D34CAB70F9BE574AE9C625B6776AC983F447FC0B9F89525902EE6F57B2FD27647F25E66C117BDB76D6593E8B0F142533A90A39D47551051EF1C86BA8163EA5AA3EA942613BED9C1EB61F684B997DA00E0194B8341EE9D5BE9A0A208D15A2F201AAF4147C02E3F59C1BFCBB9DA1F555F672956536EB022892E5344C41F94D744909CE2512F26BEC029E55448553D2254B8D95CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVRI2994ruhLU37oe5igr04s= X-DA7885C5: 05D25A5D3B1D20E0F255D290C0D534F92DC60D33D5E3142E822AA09EB6DED6E5B9F1F3D11C947B8C5B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F7393520AF17B8A65FDE29011874D7BFB9D2F7E90824FA2DE79497B16BB5BBF340413EF86D5F70DA33880E41E8EF7A07863ECB274557F927329BE2DDF8182D28ACDB545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 3/5] ARM64: Fix pass-by-value struct calling conventions. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------gCfzzdhcxZlTdQjXy0zBnrjv Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi, Sergey, thanks for the patch! Please see my comments. Sergey On 5/30/26 19:04, Sergey Kaplun wrote: > From: Mike Pall > > Reported by AnthonyK213. > > (cherry picked from commit c262976486e1e007b56380b6a36bfbea5f51d470) > > The FFI call to the function with the pass-by-value structure containing > the HFA arrays works incorrectly due to the `ccall_classify_struct()` > lacking the handling of the array case. > > This patch adds the corresponding branch to check the single-dimentional > array. However, the multidimensional arrays are not handled. This will > be fixed in the next commit. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#12480 > --- > src/lj_ccall.c | 18 +++++++++++---- > test/tarantool-tests/ffi-ccall/CMakeLists.txt | 1 + > test/tarantool-tests/ffi-ccall/libfficcall.c | 12 ++++++++++ > ...57-arm64-struct-array-pass-by-val.test.lua | 23 +++++++++++++++++++ > 4 files changed, 49 insertions(+), 5 deletions(-) > create mode 100644 test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua > > diff --git a/src/lj_ccall.c b/src/lj_ccall.c > index b2705de5..104c9d34 100644 > --- a/src/lj_ccall.c > +++ b/src/lj_ccall.c > @@ -781,17 +781,24 @@ static unsigned int ccall_classify_struct(CTState *cts, CType *ct) > { > CTSize sz = ct->size; > unsigned int r = 0, n = 0, isu = (ct->info & CTF_UNION); > - while (ct->sib) { > + while (ct->sib && n <= 4) { The patch adds a condition that strictly checks a number of elements with the same type (n <= 4). I would also add a test for this with the following `n`: 3/4/5. > + unsigned int m = 1; > CType *sct; > ct = ctype_get(cts, ct->sib); > if (ctype_isfield(ct->info)) { > sct = ctype_rawchild(cts, ct); > + if (ctype_isarray(sct->info)) { > + CType *cct = ctype_rawchild(cts, sct); > + if (!cct->size) continue; > + m = sct->size / cct->size; > + sct = cct; > + } > if (ctype_isfp(sct->info)) { > r |= sct->size; > - if (!isu) n++; else if (n == 0) n = 1; > + if (!isu) n += m; else if (n < m) n = m; The patch also touches a logic for unions (here and below), and it is desired to test it as well. This change was not caught by our regression tests on Apple M2: --- a/src/lj_ccall.c +++ b/src/lj_ccall.c @@ -742,7 +742,7 @@ static unsigned int ccall_classify_struct(CTState *cts, CType *ct, CType *ctf)        sct = ctype_rawchild(cts, ct);        if (ctype_isfp(sct->info)) {         r |= sct->size; -       if (!isu) n++; else if (n == 0) n = 1; +       if (!isu) n--; else if (n == 0) n = 1;        } else if (ctype_iscomplex(sct->info)) {         r |= (sct->size >> 1);         if (!isu) n += 2; else if (n < 2) n = 2; > } else if (ctype_iscomplex(sct->info)) { > r |= (sct->size >> 1); > - if (!isu) n += 2; else if (n < 2) n = 2; > + if (!isu) n += 2*m; else if (n < 2*m) n = 2*m; > } else if (ctype_isstruct(sct->info)) { > goto substruct; > } else { > @@ -803,10 +810,11 @@ static unsigned int ccall_classify_struct(CTState *cts, CType *ct) > sct = ctype_rawchild(cts, ct); > substruct: > if (sct->size > 0) { > - unsigned int s = ccall_classify_struct(cts, sct); > + unsigned int s = ccall_classify_struct(cts, sct), sn; > if (s <= 1) goto noth; > r |= (s & 255); > - if (!isu) n += (s >> 8); else if (n < (s >>8)) n = (s >> 8); > + sn = (s >> 8) * m; > + if (!isu) n += sn; else if (n < sn) n = sn; > } > } > } > diff --git a/test/tarantool-tests/ffi-ccall/CMakeLists.txt b/test/tarantool-tests/ffi-ccall/CMakeLists.txt > index 27de07ac..1d004591 100644 > --- a/test/tarantool-tests/ffi-ccall/CMakeLists.txt > +++ b/test/tarantool-tests/ffi-ccall/CMakeLists.txt > @@ -2,6 +2,7 @@ list(APPEND tests > ffi-ccall-arm64-fp-convention.test.lua > lj-205-arm64-osx-ffi-enum-arg.test.lua > lj-205-arm64-osx-ffi-small-arg.test.lua > + lj-1357-arm64-struct-array-pass-by-val.test.lua > ) > > BuildTestCLib(libfficcall libfficcall.c "${tests}") > diff --git a/test/tarantool-tests/ffi-ccall/libfficcall.c b/test/tarantool-tests/ffi-ccall/libfficcall.c > index fd2d4711..ecb21752 100644 > --- a/test/tarantool-tests/ffi-ccall/libfficcall.c > +++ b/test/tarantool-tests/ffi-ccall/libfficcall.c > @@ -77,3 +77,15 @@ float test_float_stack(float f1, float f2, float f3, float f4, float f5, > return f1 + f2 + f3 + f4 + f5 + f6 + f7 + f8 + f9 + f10 + f11; > } > > +/****************************************************************/ > +/* Homogeneous Floating-Point Aggregate (HFA) argument. */ > +/****************************************************************/ > + > +typedef struct hfa_float2 { > + float v[2]; > +} hfa_float2; > + > +float hfa_float2_sum(hfa_float2 h) > +{ > + return h.v[0] + h.v[1]; > +} > diff --git a/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua b/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua > new file mode 100644 > index 00000000..bb500de1 > --- /dev/null > +++ b/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua > @@ -0,0 +1,23 @@ > +local ffi = require('ffi') > +local tap = require('tap') > + > +-- The test file to demonstrate incorrect FFI pass-by-value > +-- structure with an array HFA member. > +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1357. > +local test = tap.test('lj-1357-arm64-struct-array-pass-by-val') > + > +test:plan(1) > + > +local ffi_ccall = ffi.load('libfficcall') > + > +ffi.cdef[[ > + typedef struct hfa_float2 { > + float v[2]; > + } hfa_float2; > + > + float hfa_float2_sum(hfa_float2 h); > +]] > + > +test:is(ffi_ccall.hfa_float2_sum({{1, 2}}), 3, 'HFA float correct') > + > +test:done(true) --------------gCfzzdhcxZlTdQjXy0zBnrjv Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi, Sergey,

thanks for the patch! Please see my comments.

Sergey

On 5/30/26 19:04, Sergey Kaplun wrote:
From: Mike Pall <mike>

Reported by AnthonyK213.

(cherry picked from commit c262976486e1e007b56380b6a36bfbea5f51d470)

The FFI call to the function with the pass-by-value structure containing
the HFA arrays works incorrectly due to the `ccall_classify_struct()`
lacking the handling of the array case.

This patch adds the corresponding branch to check the single-dimentional
array. However, the multidimensional arrays are not handled. This will
be fixed in the next commit.

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

Part of tarantool/tarantool#12480
---
 src/lj_ccall.c                                | 18 +++++++++++----
 test/tarantool-tests/ffi-ccall/CMakeLists.txt |  1 +
 test/tarantool-tests/ffi-ccall/libfficcall.c  | 12 ++++++++++
 ...57-arm64-struct-array-pass-by-val.test.lua | 23 +++++++++++++++++++
 4 files changed, 49 insertions(+), 5 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua

diff --git a/src/lj_ccall.c b/src/lj_ccall.c
index b2705de5..104c9d34 100644
--- a/src/lj_ccall.c
+++ b/src/lj_ccall.c
@@ -781,17 +781,24 @@ static unsigned int ccall_classify_struct(CTState *cts, CType *ct)
 {
   CTSize sz = ct->size;
   unsigned int r = 0, n = 0, isu = (ct->info & CTF_UNION);
-  while (ct->sib) {
+  while (ct->sib && n <= 4) {

The patch adds a condition that strictly checks a number of elements with the same type (n <= 4).

I would also add a test for this with the following `n`: 3/4/5.

+    unsigned int m = 1;
     CType *sct;
     ct = ctype_get(cts, ct->sib);
     if (ctype_isfield(ct->info)) {
       sct = ctype_rawchild(cts, ct);
+      if (ctype_isarray(sct->info)) {
+	CType *cct = ctype_rawchild(cts, sct);
+	if (!cct->size) continue;
+	m = sct->size / cct->size;
+	sct = cct;
+      }
       if (ctype_isfp(sct->info)) {
 	r |= sct->size;
-	if (!isu) n++; else if (n == 0) n = 1;
+	if (!isu) n += m; else if (n < m) n = m;

The patch also touches a logic for unions (here and below), and it is desired to test it as well.

This change was not caught by our regression tests on Apple M2:

--- a/src/lj_ccall.c
+++ b/src/lj_ccall.c
@@ -742,7 +742,7 @@ static unsigned int ccall_classify_struct(CTState *cts, CType *ct, CType *ctf)
       sct = ctype_rawchild(cts, ct);
       if (ctype_isfp(sct->info)) {
        r |= sct->size;
-       if (!isu) n++; else if (n == 0) n = 1;
+       if (!isu) n--; else if (n == 0) n = 1;
       } else if (ctype_iscomplex(sct->info)) {
        r |= (sct->size >> 1);
        if (!isu) n += 2; else if (n < 2) n = 2;

       } else if (ctype_iscomplex(sct->info)) {
 	r |= (sct->size >> 1);
-	if (!isu) n += 2; else if (n < 2) n = 2;
+	if (!isu) n += 2*m; else if (n < 2*m) n = 2*m;
       } else if (ctype_isstruct(sct->info)) {
 	goto substruct;
       } else {
@@ -803,10 +810,11 @@ static unsigned int ccall_classify_struct(CTState *cts, CType *ct)
       sct = ctype_rawchild(cts, ct);
     substruct:
       if (sct->size > 0) {
-	unsigned int s = ccall_classify_struct(cts, sct);
+	unsigned int s = ccall_classify_struct(cts, sct), sn;
 	if (s <= 1) goto noth;
 	r |= (s & 255);
-	if (!isu) n += (s >> 8); else if (n < (s >>8)) n = (s >> 8);
+	sn = (s >> 8) * m;
+	if (!isu) n += sn; else if (n < sn) n = sn;
       }
     }
   }
diff --git a/test/tarantool-tests/ffi-ccall/CMakeLists.txt b/test/tarantool-tests/ffi-ccall/CMakeLists.txt
index 27de07ac..1d004591 100644
--- a/test/tarantool-tests/ffi-ccall/CMakeLists.txt
+++ b/test/tarantool-tests/ffi-ccall/CMakeLists.txt
@@ -2,6 +2,7 @@ list(APPEND tests
   ffi-ccall-arm64-fp-convention.test.lua
   lj-205-arm64-osx-ffi-enum-arg.test.lua
   lj-205-arm64-osx-ffi-small-arg.test.lua
+  lj-1357-arm64-struct-array-pass-by-val.test.lua
 )
 
 BuildTestCLib(libfficcall libfficcall.c "${tests}")
diff --git a/test/tarantool-tests/ffi-ccall/libfficcall.c b/test/tarantool-tests/ffi-ccall/libfficcall.c
index fd2d4711..ecb21752 100644
--- a/test/tarantool-tests/ffi-ccall/libfficcall.c
+++ b/test/tarantool-tests/ffi-ccall/libfficcall.c
@@ -77,3 +77,15 @@ float test_float_stack(float f1, float f2, float f3, float f4, float f5,
 	return f1 + f2 + f3 + f4 + f5 + f6 + f7 + f8 + f9 + f10 + f11;
 }
 
+/****************************************************************/
+/*       Homogeneous Floating-Point Aggregate (HFA) argument.   */
+/****************************************************************/
+
+typedef struct hfa_float2 {
+	float v[2];
+} hfa_float2;
+
+float hfa_float2_sum(hfa_float2 h)
+{
+	return h.v[0] + h.v[1];
+}
diff --git a/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua b/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua
new file mode 100644
index 00000000..bb500de1
--- /dev/null
+++ b/test/tarantool-tests/lj-1357-arm64-struct-array-pass-by-val.test.lua
@@ -0,0 +1,23 @@
+local ffi = require('ffi')
+local tap = require('tap')
+
+-- The test file to demonstrate incorrect FFI pass-by-value
+-- structure with an array HFA member.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1357.
+local test = tap.test('lj-1357-arm64-struct-array-pass-by-val')
+
+test:plan(1)
+
+local ffi_ccall = ffi.load('libfficcall')
+
+ffi.cdef[[
+  typedef struct hfa_float2 {
+    float v[2];
+  } hfa_float2;
+
+  float hfa_float2_sum(hfa_float2 h);
+]]
+
+test:is(ffi_ccall.hfa_float2_sum({{1, 2}}), 3, 'HFA float correct')
+
+test:done(true)
--------------gCfzzdhcxZlTdQjXy0zBnrjv--