Hi, Sergey,

thanks for the patch! Please see my comments.

Sergey

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)