[vm/ffi] Support missing info in marshaller
When ABI-specific integers are introduced, their mappings can be
partial. We need to account for this in the marshaller.
This CL refactors the marshaller API to take an char** error return
parameter and changes the return type to a pointer (nullable) rather
than a reference.
Note that with only this CL we can not generate errors yet, because all
native types are still complete.
TEST=runtime/vm/compiler/ffi/native_type_vm_test.cc
TEST=tests/ffi*
Bug: https://github.com/dart-lang/sdk/issues/42816
Change-Id: Id97e73795c357e129e6280e8c5b528d18072c14d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221632
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
diff --git a/runtime/vm/compiler/ffi/marshaller.cc b/runtime/vm/compiler/ffi/marshaller.cc
index dbaff91..6cf5ece 100644
--- a/runtime/vm/compiler/ffi/marshaller.cc
+++ b/runtime/vm/compiler/ffi/marshaller.cc
@@ -25,9 +25,10 @@
const intptr_t kNativeParamsStartAt = 1;
// Representations of the arguments and return value of a C signature function.
-static const NativeFunctionType& NativeFunctionSignature(
+const NativeFunctionType* NativeFunctionTypeFromFunctionType(
Zone* zone,
- const FunctionType& c_signature) {
+ const FunctionType& c_signature,
+ const char** error) {
ASSERT(c_signature.NumOptionalParameters() == 0);
ASSERT(c_signature.NumOptionalPositionalParameters() == 0);
@@ -38,29 +39,41 @@
for (intptr_t i = 0; i < num_arguments; i++) {
AbstractType& arg_type = AbstractType::Handle(
zone, c_signature.ParameterTypeAt(i + kNativeParamsStartAt));
- const auto& rep = NativeType::FromAbstractType(zone, arg_type);
- argument_representations.Add(&rep);
+ const auto rep = NativeType::FromAbstractType(zone, arg_type, error);
+ if (*error != nullptr) {
+ return nullptr;
+ }
+ argument_representations.Add(rep);
}
const auto& result_type =
AbstractType::Handle(zone, c_signature.result_type());
- const auto& result_representation =
- NativeType::FromAbstractType(zone, result_type);
+ const auto result_representation =
+ NativeType::FromAbstractType(zone, result_type, error);
+ if (*error != nullptr) {
+ return nullptr;
+ }
- const auto& result = *new (zone) NativeFunctionType(argument_representations,
- result_representation);
+ const auto result = new (zone)
+ NativeFunctionType(argument_representations, *result_representation);
return result;
}
-BaseMarshaller::BaseMarshaller(Zone* zone, const Function& dart_signature)
- : zone_(zone),
- dart_signature_(dart_signature),
- c_signature_(
- FunctionType::ZoneHandle(zone, dart_signature.FfiCSignature())),
- native_calling_convention_(NativeCallingConvention::FromSignature(
- zone,
- NativeFunctionSignature(zone_, c_signature_))) {
- ASSERT(dart_signature_.IsZoneHandle());
+CallMarshaller* CallMarshaller::FromFunction(Zone* zone,
+ const Function& function,
+ const char** error) {
+ ASSERT(function.IsZoneHandle());
+ const auto& c_signature =
+ FunctionType::ZoneHandle(zone, function.FfiCSignature());
+ const auto native_function_signature =
+ NativeFunctionTypeFromFunctionType(zone, c_signature, error);
+ if (*error != nullptr) {
+ return nullptr;
+ }
+ const auto& native_calling_convention =
+ NativeCallingConvention::FromSignature(zone, *native_function_signature);
+ return new (zone)
+ CallMarshaller(zone, function, c_signature, native_calling_convention);
}
AbstractTypePtr BaseMarshaller::CType(intptr_t arg_index) const {
@@ -72,6 +85,27 @@
return c_signature_.ParameterTypeAt(arg_index + kNativeParamsStartAt);
}
+// Keep consistent with Function::FfiCSignatureReturnsStruct.
+bool BaseMarshaller::IsCompound(intptr_t arg_index) const {
+ const auto& type = AbstractType::Handle(zone_, CType(arg_index));
+ if (IsFfiTypeClassId(type.type_class_id())) {
+ return false;
+ }
+#ifdef DEBUG
+ const auto& cls = Class::Handle(this->zone_, type.type_class());
+ const auto& superClass = Class::Handle(this->zone_, cls.SuperClass());
+ // TODO(http://dartbug.com/42563): Implement AbiSpecificInt.
+ const bool is_struct =
+ String::Handle(this->zone_, superClass.UserVisibleName())
+ .Equals(Symbols::Struct());
+ const bool is_union =
+ String::Handle(this->zone_, superClass.UserVisibleName())
+ .Equals(Symbols::Union());
+ RELEASE_ASSERT(is_struct || is_union);
+#endif
+ return true;
+}
+
bool BaseMarshaller::ContainsHandles() const {
return dart_signature_.FfiCSignatureContainsHandles();
}
@@ -574,13 +608,26 @@
intptr_t argument_slots_required_ = 0;
};
-CallbackMarshaller::CallbackMarshaller(Zone* zone,
- const Function& dart_signature)
- : BaseMarshaller(zone, dart_signature),
- callback_locs_(CallbackArgumentTranslator::TranslateArgumentLocations(
- zone_,
- native_calling_convention_.argument_locations(),
- native_calling_convention_.return_location())) {}
+CallbackMarshaller* CallbackMarshaller::FromFunction(Zone* zone,
+ const Function& function,
+ const char** error) {
+ ASSERT(function.IsZoneHandle());
+ const auto& c_signature =
+ FunctionType::ZoneHandle(zone, function.FfiCSignature());
+ const auto native_function_signature =
+ NativeFunctionTypeFromFunctionType(zone, c_signature, error);
+ if (*error != nullptr) {
+ return nullptr;
+ }
+ const auto& native_calling_convention =
+ NativeCallingConvention::FromSignature(zone, *native_function_signature);
+ const auto& callback_locs =
+ CallbackArgumentTranslator::TranslateArgumentLocations(
+ zone, native_calling_convention.argument_locations(),
+ native_calling_convention.return_location());
+ return new (zone) CallbackMarshaller(
+ zone, function, c_signature, native_calling_convention, callback_locs);
+}
const NativeLocation& CallbackMarshaller::NativeLocationOfNativeParameter(
intptr_t def_index) const {
diff --git a/runtime/vm/compiler/ffi/marshaller.h b/runtime/vm/compiler/ffi/marshaller.h
index fc29f67..b7607e9 100644
--- a/runtime/vm/compiler/ffi/marshaller.h
+++ b/runtime/vm/compiler/ffi/marshaller.h
@@ -28,6 +28,13 @@
// Values below 0 index result (result might be multiple if composite).
const intptr_t kResultIndex = -1;
+// Inspects the function signature and transitively any class and field
+// definitions and annotations.
+const NativeFunctionType* NativeFunctionTypeFromFunctionType(
+ Zone* zone,
+ const FunctionType& c_signature,
+ const char** error);
+
// Provides the mapping from the native calling convention to the Dart calling
// convention.
//
@@ -114,11 +121,7 @@
kFfiBoolCid;
}
- bool IsCompound(intptr_t arg_index) const {
- const auto& type = AbstractType::Handle(zone_, CType(arg_index));
- const bool predefined = IsFfiTypeClassId(type.type_class_id());
- return !predefined;
- }
+ bool IsCompound(intptr_t arg_index) const;
// Treated as a null constant in Dart.
bool IsVoid(intptr_t arg_index) const {
@@ -131,7 +134,14 @@
StringPtr function_name() const { return dart_signature_.name(); }
protected:
- BaseMarshaller(Zone* zone, const Function& dart_signature);
+ BaseMarshaller(Zone* zone,
+ const Function& dart_signature,
+ const FunctionType& c_signature,
+ const NativeCallingConvention& native_calling_convention)
+ : zone_(zone),
+ dart_signature_(dart_signature),
+ c_signature_(c_signature),
+ native_calling_convention_(native_calling_convention) {}
~BaseMarshaller() {}
@@ -145,8 +155,18 @@
class CallMarshaller : public BaseMarshaller {
public:
- CallMarshaller(Zone* zone, const Function& dart_signature)
- : BaseMarshaller(zone, dart_signature) {}
+ static CallMarshaller* FromFunction(Zone* zone,
+ const Function& function,
+ const char** error);
+
+ CallMarshaller(Zone* zone,
+ const Function& dart_signature,
+ const FunctionType& c_signature,
+ const NativeCallingConvention& native_calling_convention)
+ : BaseMarshaller(zone,
+ dart_signature,
+ c_signature,
+ native_calling_convention) {}
virtual Representation RepInFfiCall(intptr_t def_index_global) const;
@@ -173,7 +193,20 @@
class CallbackMarshaller : public BaseMarshaller {
public:
- CallbackMarshaller(Zone* zone, const Function& dart_signature);
+ static CallbackMarshaller* FromFunction(Zone* zone,
+ const Function& function,
+ const char** error);
+
+ CallbackMarshaller(Zone* zone,
+ const Function& dart_signature,
+ const FunctionType& c_signature,
+ const NativeCallingConvention& native_calling_convention,
+ const NativeLocations& callback_locs)
+ : BaseMarshaller(zone,
+ dart_signature,
+ c_signature,
+ native_calling_convention),
+ callback_locs_(callback_locs) {}
virtual Representation RepInFfiCall(intptr_t def_index_global) const;
diff --git a/runtime/vm/compiler/ffi/native_type.cc b/runtime/vm/compiler/ffi/native_type.cc
index 6dfaeb7..bf4d452a 100644
--- a/runtime/vm/compiler/ffi/native_type.cc
+++ b/runtime/vm/compiler/ffi/native_type.cc
@@ -7,6 +7,7 @@
#include "platform/assert.h"
#include "platform/globals.h"
#include "vm/class_id.h"
+#include "vm/compiler/ffi/abi.h"
#include "vm/constants.h"
#include "vm/zone_text_buffer.h"
@@ -386,53 +387,22 @@
}
}
-NativeType& NativeType::FromTypedDataClassId(Zone* zone, classid_t class_id) {
+const NativeType& NativeType::FromTypedDataClassId(Zone* zone,
+ classid_t class_id) {
ASSERT(IsFfiPredefinedClassId(class_id));
const auto fundamental_rep = TypeRepresentation(class_id);
return *new (zone) NativePrimitiveType(fundamental_rep);
}
#if !defined(FFI_UNIT_TESTS)
-NativeType& NativeType::FromAbstractType(Zone* zone, const AbstractType& type) {
- const classid_t class_id = type.type_class_id();
- if (IsFfiPredefinedClassId(class_id)) {
- return NativeType::FromTypedDataClassId(zone, class_id);
- }
-
- // User-defined structs.
- const auto& cls = Class::Handle(zone, type.type_class());
- const auto& superClass = Class::Handle(zone, cls.SuperClass());
- const bool is_struct = String::Handle(zone, superClass.UserVisibleName())
- .Equals(Symbols::Struct());
- ASSERT(is_struct || String::Handle(zone, superClass.UserVisibleName())
- .Equals(Symbols::Union()));
-
- auto& pragmas = Object::Handle(zone);
- Library::FindPragma(dart::Thread::Current(), /*only_core=*/false, cls,
- Symbols::vm_ffi_struct_fields(), /*multiple=*/true,
- &pragmas);
- ASSERT(!pragmas.IsNull());
- ASSERT(pragmas.IsGrowableObjectArray());
- const auto& pragmas_array = GrowableObjectArray::Cast(pragmas);
- auto& pragma = Instance::Handle(zone);
- auto& clazz = Class::Handle(zone);
- auto& library = Library::Handle(zone);
- for (intptr_t i = 0; i < pragmas_array.Length(); i++) {
- pragma ^= pragmas_array.At(i);
- clazz ^= pragma.clazz();
- library ^= clazz.library();
- if (String::Handle(zone, clazz.UserVisibleName())
- .Equals(Symbols::FfiStructLayout()) &&
- String::Handle(zone, library.url()).Equals(Symbols::DartFfi())) {
- break;
- }
- }
-
+static const NativeType* CompoundFromPragma(Zone* zone,
+ const Instance& pragma,
+ bool is_struct,
+ const char** error) {
const auto& struct_layout = pragma;
- const auto& struct_layout_class = clazz;
- ASSERT(String::Handle(zone, struct_layout_class.UserVisibleName())
+ const auto& clazz = Class::Handle(zone, struct_layout.clazz());
+ ASSERT(String::Handle(zone, clazz.UserVisibleName())
.Equals(Symbols::FfiStructLayout()));
- ASSERT(String::Handle(zone, library.url()).Equals(Symbols::DartFfi()));
const auto& struct_layout_fields = Array::Handle(zone, clazz.fields());
ASSERT(struct_layout_fields.Length() == 2);
const auto& types_field =
@@ -460,8 +430,11 @@
// Subtype of NativeType: Struct, native integer or native float.
field_type ^= field_types.At(i);
const auto& field_native_type =
- NativeType::FromAbstractType(zone, field_type);
- field_native_types.Add(&field_native_type);
+ NativeType::FromAbstractType(zone, field_type, error);
+ if (*error != nullptr) {
+ return nullptr;
+ }
+ field_native_types.Add(field_native_type);
} else {
// Inline array.
const auto& struct_layout_array_class =
@@ -482,20 +455,67 @@
.Equals(Symbols::Length()));
const auto& length = Smi::Handle(
zone, Smi::RawCast(field_instance.GetField(length_field)));
- const auto& element_type = NativeType::FromAbstractType(zone, field_type);
- const auto& field_native_type =
- *new (zone) NativeArrayType(element_type, length.AsInt64Value());
- field_native_types.Add(&field_native_type);
+ const auto element_type =
+ NativeType::FromAbstractType(zone, field_type, error);
+ if (*error != nullptr) {
+ return nullptr;
+ }
+ const auto field_native_type =
+ new (zone) NativeArrayType(*element_type, length.AsInt64Value());
+ field_native_types.Add(field_native_type);
}
}
if (is_struct) {
- return NativeStructType::FromNativeTypes(zone, field_native_types,
- member_packing);
+ return &NativeStructType::FromNativeTypes(zone, field_native_types,
+ member_packing);
} else {
- return NativeUnionType::FromNativeTypes(zone, field_native_types);
+ return &NativeUnionType::FromNativeTypes(zone, field_native_types);
}
}
+
+// TODO(http://dartbug.com/42563): Implement AbiSpecificInt.
+const NativeType* NativeType::FromAbstractType(Zone* zone,
+ const AbstractType& type,
+ const char** error) {
+ const classid_t class_id = type.type_class_id();
+ if (IsFfiPredefinedClassId(class_id)) {
+ return &NativeType::FromTypedDataClassId(zone, class_id);
+ }
+
+ // User-defined structs or unions.
+ const auto& cls = Class::Handle(zone, type.type_class());
+ const auto& superClass = Class::Handle(zone, cls.SuperClass());
+ const bool is_struct = String::Handle(zone, superClass.UserVisibleName())
+ .Equals(Symbols::Struct());
+ const bool is_union = String::Handle(zone, superClass.UserVisibleName())
+ .Equals(Symbols::Union());
+ RELEASE_ASSERT(is_struct || is_union);
+
+ auto& pragmas = Object::Handle(zone);
+ String& pragma_name = String::Handle(zone);
+ pragma_name = Symbols::vm_ffi_struct_fields().ptr();
+ Library::FindPragma(dart::Thread::Current(), /*only_core=*/false, cls,
+ pragma_name, /*multiple=*/true, &pragmas);
+ ASSERT(!pragmas.IsNull());
+ ASSERT(pragmas.IsGrowableObjectArray());
+ const auto& pragmas_array = GrowableObjectArray::Cast(pragmas);
+ auto& pragma = Instance::Handle(zone);
+ auto& clazz = Class::Handle(zone);
+ auto& library = Library::Handle(zone);
+ const String& class_symbol = Symbols::FfiStructLayout();
+ for (intptr_t i = 0; i < pragmas_array.Length(); i++) {
+ pragma ^= pragmas_array.At(i);
+ clazz ^= pragma.clazz();
+ library ^= clazz.library();
+ if (String::Handle(zone, clazz.UserVisibleName()).Equals(class_symbol) &&
+ String::Handle(zone, library.url()).Equals(Symbols::DartFfi())) {
+ break;
+ }
+ }
+
+ return CompoundFromPragma(zone, pragma, is_struct, error);
+}
#endif
#if !defined(DART_PRECOMPILED_RUNTIME) && !defined(FFI_UNIT_TESTS)
diff --git a/runtime/vm/compiler/ffi/native_type.h b/runtime/vm/compiler/ffi/native_type.h
index 2ee284a..f60fea6 100644
--- a/runtime/vm/compiler/ffi/native_type.h
+++ b/runtime/vm/compiler/ffi/native_type.h
@@ -57,9 +57,11 @@
class NativeType : public ZoneAllocated {
public:
#if !defined(FFI_UNIT_TESTS)
- static NativeType& FromAbstractType(Zone* zone, const AbstractType& type);
+ static const NativeType* FromAbstractType(Zone* zone,
+ const AbstractType& type,
+ const char** error);
#endif
- static NativeType& FromTypedDataClassId(Zone* zone, classid_t class_id);
+ static const NativeType& FromTypedDataClassId(Zone* zone, classid_t class_id);
#if !defined(DART_PRECOMPILED_RUNTIME) && !defined(FFI_UNIT_TESTS)
static NativePrimitiveType& FromUnboxedRepresentation(Zone* zone,
diff --git a/runtime/vm/compiler/ffi/native_type_vm_test.cc b/runtime/vm/compiler/ffi/native_type_vm_test.cc
index 196ae4c..50a1e07 100644
--- a/runtime/vm/compiler/ffi/native_type_vm_test.cc
+++ b/runtime/vm/compiler/ffi/native_type_vm_test.cc
@@ -17,7 +17,9 @@
const auto& ffi_library = Library::Handle(Library::FfiLibrary());
const auto& int8_class = Class::Handle(GetClass(ffi_library, "Int8"));
const auto& int8_type = Type::Handle(int8_class.DeclarationType());
- const auto& native_type = NativeType::FromAbstractType(Z, int8_type);
+ const char* error = nullptr;
+ const auto& native_type = *NativeType::FromAbstractType(Z, int8_type, &error);
+ EXPECT_NULLPTR(error);
EXPECT_EQ(1, native_type.SizeInBytes());
EXPECT_STREQ("int8", native_type.ToCString());
@@ -36,7 +38,10 @@
const auto& ffi_library = Library::Handle(Library::FfiLibrary());
const auto& bool_class = Class::Handle(GetClass(ffi_library, "Bool"));
const auto& bool_type = Type::Handle(bool_class.DeclarationType());
- const auto& bool_native_type = NativeType::FromAbstractType(Z, bool_type);
+ const char* error = nullptr;
+ const auto& bool_native_type =
+ *NativeType::FromAbstractType(Z, bool_type, &error);
+ EXPECT_NULLPTR(error);
const auto& uint8_native_type = *new (Z) NativePrimitiveType(kUint8);
@@ -67,8 +72,10 @@
const auto& struct_class = Class::Handle(GetClass(root_library, "MyStruct"));
const auto& struct_type = Type::Handle(struct_class.DeclarationType());
+ const char* error = nullptr;
const auto& native_type =
- NativeType::FromAbstractType(Z, struct_type).AsCompound();
+ NativeType::FromAbstractType(Z, struct_type, &error)->AsCompound();
+ EXPECT_NULLPTR(error);
EXPECT_EQ(2, native_type.members().length());
diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc
index 981ca96..2b7346a 100644
--- a/runtime/vm/compiler/frontend/kernel_to_il.cc
+++ b/runtime/vm/compiler/frontend/kernel_to_il.cc
@@ -4488,7 +4488,12 @@
Fragment function_body(instruction_cursor);
function_body += CheckStackOverflowInPrologue(function.token_pos());
- const auto& marshaller = *new (Z) compiler::ffi::CallMarshaller(Z, function);
+ const char* error = nullptr;
+ const auto marshaller_ptr =
+ compiler::ffi::CallMarshaller::FromFunction(Z, function, &error);
+ RELEASE_ASSERT(error == nullptr);
+ RELEASE_ASSERT(marshaller_ptr != nullptr);
+ const auto& marshaller = *marshaller_ptr;
const bool signature_contains_handles = marshaller.ContainsHandles();
@@ -4634,8 +4639,12 @@
}
FlowGraph* FlowGraphBuilder::BuildGraphOfFfiCallback(const Function& function) {
- const auto& marshaller =
- *new (Z) compiler::ffi::CallbackMarshaller(Z, function);
+ const char* error = nullptr;
+ const auto marshaller_ptr =
+ compiler::ffi::CallbackMarshaller::FromFunction(Z, function, &error);
+ RELEASE_ASSERT(error == nullptr);
+ RELEASE_ASSERT(marshaller_ptr != nullptr);
+ const auto& marshaller = *marshaller_ptr;
graph_entry_ =
new (Z) GraphEntryInstr(*parsed_function_, Compiler::kNoOSRDeoptId);
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 2513eb4..df10c58 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -7553,12 +7553,26 @@
kFfiHandleCid;
}
+// Keep consistent with BaseMarshaller::IsCompound.
bool Function::FfiCSignatureReturnsStruct() const {
ASSERT(IsFfiTrampoline());
- const FunctionType& c_signature = FunctionType::Handle(FfiCSignature());
- const auto& return_type = AbstractType::Handle(c_signature.result_type());
- const bool predefined = IsFfiTypeClassId(return_type.type_class_id());
- return !predefined;
+ Zone* zone = Thread::Current()->zone();
+ const auto& c_signature = FunctionType::Handle(zone, FfiCSignature());
+ const auto& type = AbstractType::Handle(zone, c_signature.result_type());
+ if (IsFfiTypeClassId(type.type_class_id())) {
+ return false;
+ }
+ // TODO(http://dartbug.com/42563): Implement AbiSpecificInt.
+#ifdef DEBUG
+ const auto& cls = Class::Handle(zone, type.type_class());
+ const auto& superClass = Class::Handle(zone, cls.SuperClass());
+ const bool is_struct = String::Handle(zone, superClass.UserVisibleName())
+ .Equals(Symbols::Struct());
+ const bool is_union = String::Handle(zone, superClass.UserVisibleName())
+ .Equals(Symbols::Union());
+ RELEASE_ASSERT(is_struct || is_union);
+#endif
+ return true;
}
int32_t Function::FfiCallbackId() const {
