Skip to content

Commit

Permalink
[vm/ffi] Fix variadic arguments on MacOS Arm64
Browse files Browse the repository at this point in the history
Non-variadic arguments on the stack on MacOS Arm64 are aligned to
the value size (which can be smaller than word size). However, for
varargs, the arguments on the stack seem to be aligned to the word
size.

This CL introduces an alignment strategy constant for primitives on
the stack in varargs and uses it in the native calling convention
calculation.

TEST=runtime/vm/compiler/ffi/native_calling_convention_test.cc with
runtime/vm/compiler/ffi/unit_tests/variadic_less_than_word/arm64_macos.expect

TEST=tests/ffi/function_varargs_generated_native_leaf_test.dart

Closes: #55471
Change-Id: I51d20c3933a2cea9b110954ddec92fb91b9c3ecd
Cq-Include-Trybots: dart/try:vm-aot-android-release-arm64c-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,vm-aot-win-debug-x64c-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-debug-x64c-try,vm-win-release-ia32-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362762
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
  • Loading branch information
dcharkes authored and Commit Queue committed Apr 17, 2024
1 parent 4bb644c commit c0b938c
Show file tree
Hide file tree
Showing 35 changed files with 685 additions and 2 deletions.
78 changes: 78 additions & 0 deletions runtime/bin/ffi_test/ffi_test_functions_generated.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22996,6 +22996,38 @@ DART_EXPORT int64_t VariadicAt1Int64x7Struct12BytesHomogeneousInt32(int64_t a0,
return result;
}

// Used for testing structs and unions by value.
// Variadic arguments test on macos_arm64.
DART_EXPORT int32_t VariadicAt1Struct12BytesHomogeneousInt32Int32x4(
Struct12BytesHomogeneousInt32 a0,
...) {
va_list var_args;
va_start(var_args, a0);
int32_t a1 = va_arg(var_args, int32_t);
int32_t a2 = va_arg(var_args, int32_t);
int32_t a3 = va_arg(var_args, int32_t);
int32_t a4 = va_arg(var_args, int32_t);
va_end(var_args);

std::cout << "VariadicAt1Struct12BytesHomogeneousInt32Int32x4" << "(("
<< a0.a0 << ", " << a0.a1 << ", " << a0.a2 << "), " << a1 << ", "
<< a2 << ", " << a3 << ", " << a4 << ")" << "\n";

int32_t result = 0;

result += a0.a0;
result += a0.a1;
result += a0.a2;
result += a1;
result += a2;
result += a3;
result += a4;

std::cout << "result = " << result << "\n";

return result;
}

// Used for testing structs and unions by value.
// Single variadic argument.
DART_EXPORT intptr_t TestVariadicAt1Int64x2(
Expand Down Expand Up @@ -23897,4 +23929,50 @@ DART_EXPORT intptr_t TestVariadicAt1Int64x7Struct12BytesHomogeneousInt32(
return 0;
}

// Used for testing structs and unions by value.
// Variadic arguments test on macos_arm64.
DART_EXPORT intptr_t TestVariadicAt1Struct12BytesHomogeneousInt32Int32x4(
// NOLINTNEXTLINE(whitespace/parens)
int32_t (*f)(Struct12BytesHomogeneousInt32 a0, ...)) {
Struct12BytesHomogeneousInt32 a0 = {};
int32_t a1;
int32_t a2;
int32_t a3;
int32_t a4;

a0.a0 = -1;
a0.a1 = 2;
a0.a2 = -3;
a1 = 4;
a2 = -5;
a3 = 6;
a4 = -7;

std::cout << "Calling TestVariadicAt1Struct12BytesHomogeneousInt32Int32x4("
<< "((" << a0.a0 << ", " << a0.a1 << ", " << a0.a2 << "), " << a1
<< ", " << a2 << ", " << a3 << ", " << a4 << ")" << ")\n";

int32_t result = f(a0, a1, a2, a3, a4);

std::cout << "result = " << result << "\n";

CHECK_EQ(-4, result);

// Pass argument that will make the Dart callback throw.
a0.a0 = 42;

result = f(a0, a1, a2, a3, a4);

CHECK_EQ(0, result);

// Pass argument that will make the Dart callback return null.
a0.a0 = 84;

result = f(a0, a1, a2, a3, a4);

CHECK_EQ(0, result);

return 0;
}

} // namespace dart
2 changes: 1 addition & 1 deletion runtime/vm/compiler/ffi/native_calling_convention.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ class ArgumentAllocator : public ValueObject {
zone_, payload_type, container_type, AllocateCpuRegister());
}
}
return AllocateStack(payload_type);
return AllocateStack(payload_type, is_vararg);
}

// Constructs a container type.
Expand Down
22 changes: 22 additions & 0 deletions runtime/vm/compiler/ffi/native_calling_convention_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,28 @@ UNIT_TEST_CASE_WITH_ZONE(
RunSignatureTest(Z, "variadic_stradle_last_register", native_signature);
}

// Struct parameter that potentially is partially allocated to a register and
// partially to the stack. Mainly interesting on ARM64 and RISC-V.
//
// See the *.expect in ./unit_tests for this behavior.
UNIT_TEST_CASE_WITH_ZONE(NativeCallingConvention_variadic_less_than_word) {
#if defined(TARGET_ARCH_IS_32_BIT)
const auto& halfptr_type = *new (Z) NativePrimitiveType(kInt16);
#elif defined(TARGET_ARCH_IS_64_BIT)
const auto& halfptr_type = *new (Z) NativePrimitiveType(kInt32);
#endif

auto& arguments = *new (Z) NativeTypes(Z, 12);
for (intptr_t i = 0; i < 12; i++) {
arguments.Add(&halfptr_type);
}

const auto& native_signature = *new (Z) NativeFunctionType(
arguments, halfptr_type, /*variadic_arguments_index=*/1);

RunSignatureTest(Z, "variadic_less_than_word", native_signature);
}

} // namespace ffi
} // namespace compiler
} // namespace dart
5 changes: 4 additions & 1 deletion runtime/vm/compiler/ffi/native_type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,10 @@ intptr_t NativePrimitiveType::SizeInBytes() const {
}

intptr_t NativePrimitiveType::AlignmentInBytesStack(bool is_vararg) const {
switch (CallingConventions::kArgumentStackAlignment) {
const auto alignment =
is_vararg ? CallingConventions::kArgumentStackAlignmentVarArgs
: CallingConventions::kArgumentStackAlignment;
switch (alignment) {
case kAlignedToWordSize:
// The default is to align stack arguments to word size.
return compiler::target::kWordSize;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
r0 int32
r1 int32
r2 int32
r3 int32
r4 int32
r5 int32
r6 int32
r7 int32
S+0 int32
S+8 int32
S+16 int32
S+24 int32
=>
r0 int32
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
r0 int32
r1 int32
r2 int32
r3 int32
r4 int32
r5 int32
r6 int32
r7 int32
S+0 int32
S+8 int32
S+16 int32
S+24 int32
=>
r0 int32
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
r0 int32
S+0 int32
S+8 int32
S+16 int32
S+24 int32
S+32 int32
S+40 int32
S+48 int32
S+56 int32
S+64 int32
S+72 int32
S+80 int32
=>
r0 int32
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
r0 int32
r1 int32
r2 int32
r3 int32
r4 int32
r5 int32
r6 int32
r7 int32
S+0 int32
S+8 int32
S+16 int32
S+24 int32
=>
r0 int32
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
r0 int32
S+0 int32
S+8 int32
S+16 int32
S+24 int32
S+32 int32
S+40 int32
S+48 int32
S+56 int32
S+64 int32
S+72 int32
S+80 int32
=>
r0 int32
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
r0 int32
r1 int32
r2 int32
r3 int32
r4 int32
r5 int32
r6 int32
r7 int32
S+0 int32
S+8 int32
S+16 int32
S+24 int32
=>
r0 int32
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
r0 int32[int16]
r1 int32[int16]
r2 int32[int16]
r3 int32[int16]
S+0 int32[int16]
S+4 int32[int16]
S+8 int32[int16]
S+12 int32[int16]
S+16 int32[int16]
S+20 int32[int16]
S+24 int32[int16]
S+28 int32[int16]
=>
r0 int32[int16]
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
r0 int32[int16]
r1 int32[int16]
r2 int32[int16]
r3 int32[int16]
S+0 int32[int16]
S+4 int32[int16]
S+8 int32[int16]
S+12 int32[int16]
S+16 int32[int16]
S+20 int32[int16]
S+24 int32[int16]
S+28 int32[int16]
=>
r0 int32[int16]
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
r0 int32[int16]
r1 int32[int16]
r2 int32[int16]
r3 int32[int16]
S+0 int32[int16]
S+4 int32[int16]
S+8 int32[int16]
S+12 int32[int16]
S+16 int32[int16]
S+20 int32[int16]
S+24 int32[int16]
S+28 int32[int16]
=>
r0 int32[int16]
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
S+0 int32[int16]
S+4 int32[int16]
S+8 int32[int16]
S+12 int32[int16]
S+16 int32[int16]
S+20 int32[int16]
S+24 int32[int16]
S+28 int32[int16]
S+32 int32[int16]
S+36 int32[int16]
S+40 int32[int16]
S+44 int32[int16]
=>
eax int16
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
S+0 int32[int16]
S+4 int32[int16]
S+8 int32[int16]
S+12 int32[int16]
S+16 int32[int16]
S+20 int32[int16]
S+24 int32[int16]
S+28 int32[int16]
S+32 int32[int16]
S+36 int32[int16]
S+40 int32[int16]
S+44 int32[int16]
=>
eax int16
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
S+0 int32[int16]
S+4 int32[int16]
S+8 int32[int16]
S+12 int32[int16]
S+16 int32[int16]
S+20 int32[int16]
S+24 int32[int16]
S+28 int32[int16]
S+32 int32[int16]
S+36 int32[int16]
S+40 int32[int16]
S+44 int32[int16]
=>
eax int16
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
a0 int32[int16]
a1 int32[int16]
a2 int32[int16]
t3 int32[int16]
t4 int32[int16]
t5 int32[int16]
a6 int32[int16]
a7 int32[int16]
S+0 int32[int16]
S+4 int32[int16]
S+8 int32[int16]
S+12 int32[int16]
=>
a0 int32[int16]
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
a0 int64[int32]
a1 int64[int32]
a2 int64[int32]
t3 int64[int32]
t4 int64[int32]
t5 int64[int32]
a6 int64[int32]
a7 int64[int32]
S+0 int64[int32]
S+8 int64[int32]
S+16 int64[int32]
S+24 int64[int32]
=>
a0 int64[int32]
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
rdi int32
rsi int32
rdx int32
rcx int32
r8 int32
r9 int32
S+0 int32
S+8 int32
S+16 int32
S+24 int32
S+32 int32
S+40 int32
=>
rax int32
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
rdi int32
rsi int32
rdx int32
rcx int32
r8 int32
r9 int32
S+0 int32
S+8 int32
S+16 int32
S+24 int32
S+32 int32
S+40 int32
=>
rax int32
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
rdi int32
rsi int32
rdx int32
rcx int32
r8 int32
r9 int32
S+0 int32
S+8 int32
S+16 int32
S+24 int32
S+32 int32
S+40 int32
=>
rax int32
Loading

0 comments on commit c0b938c

Please sign in to comment.