[vm, ffi] Round up checking for registers for small structs.

This fixes an edge case on all ARM64 ABIs, when there is only one argument register remaining and the next argument is a 9-16 byte struct.

TEST=ci
Bug: #52644
Change-Id: I40d962e6d1b3484dbfcf91f5d6baca0bfec76056
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330161
Reviewed-by: Daco Harkes <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

runtime/bin/ffi_test/ffi_test_functions_generated.cc

@@ -137,6 +137,12 @@ struct Struct12BytesHomogeneousFloat {
   float a2;
 };
 
+struct Struct12BytesHomogeneousInt32 {
+  int32_t a0;
+  int32_t a1;
+  int32_t a2;
+};
+
 struct Struct16BytesHomogeneousFloat {
   float a0;
   float a1;
@@ -4833,6 +4839,41 @@ DART_EXPORT wchar_t PassWCharStructInlineArrayIntUintPtrx2LongUnsigned(
   return result;
 }
 
+// Used for testing structs and unions by value.
+// Struct stradles last argument register
+DART_EXPORT int64_t
+PassInt64x7Struct12BytesHomogeneousInt32(int64_t a0,
+                                         int64_t a1,
+                                         int64_t a2,
+                                         int64_t a3,
+                                         int64_t a4,
+                                         int64_t a5,
+                                         int64_t a6,
+                                         Struct12BytesHomogeneousInt32 a7) {
+  std::cout << "PassInt64x7Struct12BytesHomogeneousInt32"
+            << "(" << a0 << ", " << a1 << ", " << a2 << ", " << a3 << ", " << a4
+            << ", " << a5 << ", " << a6 << ", (" << a7.a0 << ", " << a7.a1
+            << ", " << a7.a2 << "))"
+            << "\n";
+
+  int64_t result = 0;
+
+  result += a0;
+  result += a1;
+  result += a2;
+  result += a3;
+  result += a4;
+  result += a5;
+  result += a6;
+  result += a7.a0;
+  result += a7.a1;
+  result += a7.a2;
+
+  std::cout << "result = " << result << "\n";
+
+  return result;
+}
+
 // Used for testing structs and unions by value.
 // Smallest struct with data.
 DART_EXPORT Struct1ByteInt ReturnStruct1ByteInt(int8_t a0) {
@@ -12503,6 +12544,67 @@ DART_EXPORT intptr_t TestPassWCharStructInlineArrayIntUintPtrx2LongUnsigned(
   return 0;
 }
 
+// Used for testing structs and unions by value.
+// Struct stradles last argument register
+DART_EXPORT intptr_t TestPassInt64x7Struct12BytesHomogeneousInt32(
+    // NOLINTNEXTLINE(whitespace/parens)
+    int64_t (*f)(int64_t a0,
+                 int64_t a1,
+                 int64_t a2,
+                 int64_t a3,
+                 int64_t a4,
+                 int64_t a5,
+                 int64_t a6,
+                 Struct12BytesHomogeneousInt32 a7)) {
+  int64_t a0;
+  int64_t a1;
+  int64_t a2;
+  int64_t a3;
+  int64_t a4;
+  int64_t a5;
+  int64_t a6;
+  Struct12BytesHomogeneousInt32 a7 = {};
+
+  a0 = -1;
+  a1 = 2;
+  a2 = -3;
+  a3 = 4;
+  a4 = -5;
+  a5 = 6;
+  a6 = -7;
+  a7.a0 = 8;
+  a7.a1 = -9;
+  a7.a2 = 10;
+
+  std::cout << "Calling TestPassInt64x7Struct12BytesHomogeneousInt32("
+            << "(" << a0 << ", " << a1 << ", " << a2 << ", " << a3 << ", " << a4
+            << ", " << a5 << ", " << a6 << ", (" << a7.a0 << ", " << a7.a1
+            << ", " << a7.a2 << "))"
+            << ")\n";
+
+  int64_t result = f(a0, a1, a2, a3, a4, a5, a6, a7);
+
+  std::cout << "result = " << result << "\n";
+
+  CHECK_EQ(5, result);
+
+  // Pass argument that will make the Dart callback throw.
+  a0 = 42;
+
+  result = f(a0, a1, a2, a3, a4, a5, a6, a7);
+
+  CHECK_EQ(0, result);
+
+  // Pass argument that will make the Dart callback return null.
+  a0 = 84;
+
+  result = f(a0, a1, a2, a3, a4, a5, a6, a7);
+
+  CHECK_EQ(0, result);
+
+  return 0;
+}
+
 // Used for testing structs and unions by value.
 // Smallest struct with data.
 DART_EXPORT intptr_t TestReturnStruct1ByteInt(
@@ -21080,6 +21182,47 @@ DART_EXPORT void TestAsyncPassWCharStructInlineArrayIntUintPtrx2LongUnsigned(
   f(a0, a1, a2, a3, a4, a5);
 }
 
+// Used for testing structs and unions by value.
+// Struct stradles last argument register
+DART_EXPORT void TestAsyncPassInt64x7Struct12BytesHomogeneousInt32(
+    // NOLINTNEXTLINE(whitespace/parens)
+    void (*f)(int64_t a0,
+              int64_t a1,
+              int64_t a2,
+              int64_t a3,
+              int64_t a4,
+              int64_t a5,
+              int64_t a6,
+              Struct12BytesHomogeneousInt32 a7)) {
+  int64_t a0;
+  int64_t a1;
+  int64_t a2;
+  int64_t a3;
+  int64_t a4;
+  int64_t a5;
+  int64_t a6;
+  Struct12BytesHomogeneousInt32 a7 = {};
+
+  a0 = -1;
+  a1 = 2;
+  a2 = -3;
+  a3 = 4;
+  a4 = -5;
+  a5 = 6;
+  a6 = -7;
+  a7.a0 = 8;
+  a7.a1 = -9;
+  a7.a2 = 10;
+
+  std::cout << "Calling TestAsyncPassInt64x7Struct12BytesHomogeneousInt32("
+            << "(" << a0 << ", " << a1 << ", " << a2 << ", " << a3 << ", " << a4
+            << ", " << a5 << ", " << a6 << ", (" << a7.a0 << ", " << a7.a1
+            << ", " << a7.a2 << "))"
+            << ")\n";
+
+  f(a0, a1, a2, a3, a4, a5, a6, a7);
+}
+
 // Used for testing structs and unions by value.
 // Smallest struct with data.
 DART_EXPORT void TestAsyncReturnStruct1ByteInt(
@@ -23371,6 +23514,46 @@ DART_EXPORT double VariadicAt5Doublex5(double a0,
   return result;
 }
 
+// Used for testing structs and unions by value.
+// Struct stradles last argument register, variadic
+DART_EXPORT int64_t VariadicAt1Int64x7Struct12BytesHomogeneousInt32(int64_t a0,
+                                                                    ...) {
+  va_list var_args;
+  va_start(var_args, a0);
+  int64_t a1 = va_arg(var_args, int64_t);
+  int64_t a2 = va_arg(var_args, int64_t);
+  int64_t a3 = va_arg(var_args, int64_t);
+  int64_t a4 = va_arg(var_args, int64_t);
+  int64_t a5 = va_arg(var_args, int64_t);
+  int64_t a6 = va_arg(var_args, int64_t);
+  Struct12BytesHomogeneousInt32 a7 =
+      va_arg(var_args, Struct12BytesHomogeneousInt32);
+  va_end(var_args);
+
+  std::cout << "VariadicAt1Int64x7Struct12BytesHomogeneousInt32"
+            << "(" << a0 << ", " << a1 << ", " << a2 << ", " << a3 << ", " << a4
+            << ", " << a5 << ", " << a6 << ", (" << a7.a0 << ", " << a7.a1
+            << ", " << a7.a2 << "))"
+            << "\n";
+
+  int64_t result = 0;
+
+  result += a0;
+  result += a1;
+  result += a2;
+  result += a3;
+  result += a4;
+  result += a5;
+  result += a6;
+  result += a7.a0;
+  result += a7.a1;
+  result += a7.a2;
+
+  std::cout << "result = " << result << "\n";
+
+  return result;
+}
+
 // Used for testing structs and unions by value.
 // Single variadic argument.
 DART_EXPORT intptr_t TestVariadicAt1Int64x2(
@@ -24239,4 +24422,58 @@ DART_EXPORT intptr_t TestVariadicAt5Doublex5(
   return 0;
 }
 
+// Used for testing structs and unions by value.
+// Struct stradles last argument register, variadic
+DART_EXPORT intptr_t TestVariadicAt1Int64x7Struct12BytesHomogeneousInt32(
+    // NOLINTNEXTLINE(whitespace/parens)
+    int64_t (*f)(int64_t a0, ...)) {
+  int64_t a0;
+  int64_t a1;
+  int64_t a2;
+  int64_t a3;
+  int64_t a4;
+  int64_t a5;
+  int64_t a6;
+  Struct12BytesHomogeneousInt32 a7 = {};
+
+  a0 = -1;
+  a1 = 2;
+  a2 = -3;
+  a3 = 4;
+  a4 = -5;
+  a5 = 6;
+  a6 = -7;
+  a7.a0 = 8;
+  a7.a1 = -9;
+  a7.a2 = 10;
+
+  std::cout << "Calling TestVariadicAt1Int64x7Struct12BytesHomogeneousInt32("
+            << "(" << a0 << ", " << a1 << ", " << a2 << ", " << a3 << ", " << a4
+            << ", " << a5 << ", " << a6 << ", (" << a7.a0 << ", " << a7.a1
+            << ", " << a7.a2 << "))"
+            << ")\n";
+
+  int64_t result = f(a0, a1, a2, a3, a4, a5, a6, a7);
+
+  std::cout << "result = " << result << "\n";
+
+  CHECK_EQ(5, result);
+
+  // Pass argument that will make the Dart callback throw.
+  a0 = 42;
+
+  result = f(a0, a1, a2, a3, a4, a5, a6, a7);
+
+  CHECK_EQ(0, result);
+
+  // Pass argument that will make the Dart callback return null.
+  a0 = 84;
+
+  result = f(a0, a1, a2, a3, a4, a5, a6, a7);
+
+  CHECK_EQ(0, result);
+
+  return 0;
+}
+
 }  // namespace dart

runtime/vm/compiler/ffi/native_calling_convention.cc

@@ -433,31 +433,31 @@ class ArgumentAllocator : public ValueObject {
     }
 
     if (size <= 16) {
-      const intptr_t required_regs = size / 8;
-      const bool regs_available =
-          cpu_regs_used + required_regs <= CallingConventions::kNumArgRegs;
+      const intptr_t size_rounded = Utils::RoundUp(size, 8);
+      const intptr_t num_chunks = size_rounded / 8;
+      ASSERT((num_chunks == 1) || (num_chunks == 2));
 
-      if (regs_available) {
-        const intptr_t size_rounded =
-            Utils::RoundUp(payload_type.SizeInBytes(), 8);
-        const intptr_t num_chunks = size_rounded / 8;
-        const auto& chunk_type = *new (zone_) NativePrimitiveType(kInt64);
+      // All-or-none: block any leftover registers.
+#if defined(DART_TARGET_OS_WINDOWS)
+      if (!HasAvailableCpuRegisters(num_chunks) && !is_vararg) {
+        cpu_regs_used = CallingConventions::kNumArgRegs;
+      }
+#else
+      if (!HasAvailableCpuRegisters(num_chunks)) {
+        cpu_regs_used = CallingConventions::kNumArgRegs;
+      }
+#endif
 
-        NativeLocations& multiple_locations =
-            *new (zone_) NativeLocations(zone_, num_chunks);
-        for (int i = 0; i < num_chunks; i++) {
-          const auto& allocated_chunk =
-              &AllocateArgument(chunk_type, is_vararg);
-          multiple_locations.Add(allocated_chunk);
-        }
-        return *new (zone_)
-            MultipleNativeLocations(compound_type, multiple_locations);
+      const auto& chunk_type = *new (zone_) NativePrimitiveType(kInt64);
 
-      } else {
-        // Block all CPU registers.
-        cpu_regs_used = CallingConventions::kNumArgRegs;
-        return AllocateStack(payload_type, is_vararg);
+      NativeLocations& multiple_locations =
+          *new (zone_) NativeLocations(zone_, num_chunks);
+      for (int i = 0; i < num_chunks; i++) {
+        const auto& allocated_chunk = &AllocateArgument(chunk_type, is_vararg);
+        multiple_locations.Add(allocated_chunk);
       }
+      return *new (zone_)
+          MultipleNativeLocations(compound_type, multiple_locations);
     }
 
     const auto& pointer_location =

runtime/vm/compiler/ffi/unit_tests/stradle_last_register/arm64_android.expect

@@ -5,6 +5,6 @@ r3 int64
 r4 int64
 r5 int64
 r6 int64
-M(r7 int64, S+0 int64) Struct(size: 12)
+M(S+0 int64, S+8 int64) Struct(size: 12)
 =>
 r0 int64

runtime/vm/compiler/ffi/unit_tests/stradle_last_register/arm64_fuchsia.expect

@@ -5,6 +5,6 @@ r3 int64
 r4 int64
 r5 int64
 r6 int64
-M(r7 int64, S+0 int64) Struct(size: 12)
+M(S+0 int64, S+8 int64) Struct(size: 12)
 =>
 r0 int64

runtime/vm/compiler/ffi/unit_tests/stradle_last_register/arm64_ios.expect

@@ -5,6 +5,6 @@ r3 int64
 r4 int64
 r5 int64
 r6 int64
-M(r7 int64, S+0 int64) Struct(size: 12)
+M(S+0 int64, S+8 int64) Struct(size: 12)
 =>
 r0 int64

runtime/vm/compiler/ffi/unit_tests/stradle_last_register/arm64_linux.expect

@@ -5,6 +5,6 @@ r3 int64
 r4 int64
 r5 int64
 r6 int64
-M(r7 int64, S+0 int64) Struct(size: 12)
+M(S+0 int64, S+8 int64) Struct(size: 12)
 =>
 r0 int64

runtime/vm/compiler/ffi/unit_tests/stradle_last_register/arm64_macos.expect

@@ -5,6 +5,6 @@ r3 int64
 r4 int64
 r5 int64
 r6 int64
-M(r7 int64, S+0 int64) Struct(size: 12)
+M(S+0 int64, S+8 int64) Struct(size: 12)
 =>
 r0 int64

runtime/vm/compiler/ffi/unit_tests/stradle_last_register/arm64_win.expect

@@ -5,6 +5,6 @@ r3 int64
 r4 int64
 r5 int64
 r6 int64
-M(r7 int64, S+0 int64) Struct(size: 12)
+M(S+0 int64, S+8 int64) Struct(size: 12)
 =>
 r0 int64

runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/arm64_android.expect

@@ -6,7 +6,7 @@ M(r4 int64) Struct(size: 8)
 M(r5 int64) Struct(size: 8)
 M(r6 int64) Struct(size: 8)
 M(r7 int64) Struct(size: 8)
-S+0 Struct(size: 8)
-S+8 Struct(size: 8)
+M(S+0 int64) Struct(size: 8)
+M(S+8 int64) Struct(size: 8)
 =>
 M(r0 int64) Struct(size: 8)

runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/arm64_fuchsia.expect

@@ -6,7 +6,7 @@ M(r4 int64) Struct(size: 8)
 M(r5 int64) Struct(size: 8)
 M(r6 int64) Struct(size: 8)
 M(r7 int64) Struct(size: 8)
-S+0 Struct(size: 8)
-S+8 Struct(size: 8)
+M(S+0 int64) Struct(size: 8)
+M(S+8 int64) Struct(size: 8)
 =>
 M(r0 int64) Struct(size: 8)