Skip to content

Commit

Permalink
[cfe/ffi] Support missing Abis in NativeTypeCfe
Browse files Browse the repository at this point in the history
When ABI-specific integers are introduced, their mappings can be
partial. We need to account for this in the transformation and the code
we generate.

In the transformation, all sizes and offsets become nullable.
In the generated code we add `null` constants and a call to check
whether the value is not-null at runtime.

Note that with only this CL we can not generate nulls yet, because all
size and offset mappings are still complete.

TEST=pkg/front_end/testcases/nnbd/ffi*
TEST=tests/ffi*

Bug: #42563

Change-Id: I80d45f3f52001670bc0679a033f7daa22198d55e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221631
Reviewed-by: Ryan Macnak <[email protected]>
  • Loading branch information
dcharkes authored and Commit Bot committed Dec 2, 2021
1 parent eea0e48 commit 734eb8e
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 65 deletions.
39 changes: 31 additions & 8 deletions pkg/vm/lib/transformations/ffi/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ class FfiTransformer extends Transformer {
final Procedure lookupFunctionTearoff;
final Procedure getNativeFieldFunction;
final Procedure reachabilityFenceFunction;
final Procedure checkAbiSpecificIntegerMappingFunction;

late final InterfaceType nativeFieldWrapperClass1Type;
late final InterfaceType voidType;
Expand Down Expand Up @@ -417,7 +418,9 @@ class FfiTransformer extends Transformer {
getNativeFieldFunction = index.getTopLevelProcedure(
'dart:nativewrappers', '_getNativeField'),
reachabilityFenceFunction =
index.getTopLevelProcedure('dart:_internal', 'reachabilityFence') {
index.getTopLevelProcedure('dart:_internal', 'reachabilityFence'),
checkAbiSpecificIntegerMappingFunction = index.getTopLevelProcedure(
'dart:ffi', "_checkAbiSpecificIntegerMapping") {
nativeFieldWrapperClass1Type = nativeFieldWrapperClass1Class.getThisType(
coreTypes, Nullability.nonNullable);
voidType = nativeTypesClasses[NativeType.kVoid]!
Expand Down Expand Up @@ -453,7 +456,7 @@ class FfiTransformer extends Transformer {
/// [Bool] -> [bool]
/// [Void] -> [void]
/// [Pointer]<T> -> [Pointer]<T>
/// T extends [Pointer] -> T
/// T extends [Compound] -> T
/// [Handle] -> [Object]
/// [NativeFunction]<T1 Function(T2, T3) -> S1 Function(S2, S3)
/// where DartRepresentationOf(Tn) -> Sn
Expand Down Expand Up @@ -535,27 +538,42 @@ class FfiTransformer extends Transformer {
InterfaceType _listOfIntType() => InterfaceType(
listClass, Nullability.legacy, [coreTypes.intLegacyRawType]);

ConstantExpression intListConstantExpression(List<int> values) =>
ConstantExpression intListConstantExpression(List<int?> values) =>
ConstantExpression(
ListConstant(coreTypes.intLegacyRawType,
[for (var v in values) IntConstant(v)]),
ListConstant(coreTypes.intLegacyRawType, [
for (var v in values)
if (v != null) IntConstant(v) else NullConstant()
]),
_listOfIntType());

/// Expression that queries VM internals at runtime to figure out on which ABI
/// we are.
Expression runtimeBranchOnLayout(Map<Abi, int> values) {
return InstanceInvocation(
Expression runtimeBranchOnLayout(Map<Abi, int?> values) {
final result = InstanceInvocation(
InstanceAccessKind.Instance,
intListConstantExpression([
for (final abi in Abi.values) values[abi]!,
for (final abi in Abi.values) values[abi],
]),
listElementAt.name,
Arguments([StaticInvocation(abiMethod, Arguments([]))]),
interfaceTarget: listElementAt,
functionType: Substitution.fromInterfaceType(_listOfIntType())
.substituteType(listElementAt.getterType) as FunctionType);
if (values.isPartial) {
return checkAbiSpecificIntegerMapping(result);
}
return result;
}

Expression checkAbiSpecificIntegerMapping(Expression nullableExpression) =>
StaticInvocation(
checkAbiSpecificIntegerMappingFunction,
Arguments(
[nullableExpression],
types: [InterfaceType(intClass, Nullability.nonNullable)],
),
);

/// Generates an expression that returns a new `Pointer<dartType>` offset
/// by [offset] from [pointer].
///
Expand Down Expand Up @@ -819,3 +837,8 @@ bool importsFfi(Component component, List<Library> libraries) {
}
return false;
}

extension on Map<Abi, Object?> {
bool get isPartial =>
[for (final abi in Abi.values) this[abi]].contains(null);
}
18 changes: 10 additions & 8 deletions pkg/vm/lib/transformations/ffi/definitions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -448,9 +448,8 @@ class _FfiDefinitionTransformer extends FfiTransformer {
// This class is invalid, but continue reporting other errors on it.
success = false;
} else {
final DartType nativeType = InterfaceType(
nativeTypesClasses[_getFieldType(nativeTypeAnnos.first)!]!,
Nullability.legacy);
final DartType nativeType =
InterfaceType(nativeTypeAnnos.first, Nullability.legacy);
final DartType? shouldBeDartType = convertNativeTypeToDartType(
nativeType,
allowCompounds: true,
Expand Down Expand Up @@ -704,7 +703,6 @@ class _FfiDefinitionTransformer extends FfiTransformer {

static const vmFfiStructFields = "vm:ffi:struct-fields";

// return value is nullable.
InstanceConstant? _compoundAnnotatedFields(Class node) {
for (final annotation in node.annotations) {
if (annotation is ConstantExpression) {
Expand Down Expand Up @@ -774,7 +772,6 @@ class _FfiDefinitionTransformer extends FfiTransformer {
return UnionNativeTypeCfe(compoundClass, members);
}

// packing is `int?`.
void _annoteCompoundWithFields(
Class node, List<NativeTypeCfe> types, int? packing) {
List<Constant> constants =
Expand All @@ -794,8 +791,13 @@ class _FfiDefinitionTransformer extends FfiTransformer {
InterfaceType(pragmaClass, Nullability.nonNullable, [])));
}

void _generateMethodsForField(Class node, Field field, NativeTypeCfe type,
Map<Abi, int> offsets, bool unalignedAccess, IndexedClass? indexedClass) {
void _generateMethodsForField(
Class node,
Field field,
NativeTypeCfe type,
Map<Abi, int?> offsets,
bool unalignedAccess,
IndexedClass? indexedClass) {
// TODO(johnniwinther): Avoid passing [indexedClass]. When compiling
// incrementally, [field] should already carry the references from
// [indexedClass].
Expand Down Expand Up @@ -846,7 +848,7 @@ class _FfiDefinitionTransformer extends FfiTransformer {
/// If sizes are not supplied still emits a field so that the use site
/// transformer can still rewrite to it.
void _addSizeOfField(Class compound, IndexedClass? indexedClass,
[Map<Abi, int>? sizes = null]) {
[Map<Abi, int?>? sizes = null]) {
if (sizes == null) {
sizes = {for (var abi in Abi.values) abi: 0};
}
Expand Down
Loading

0 comments on commit 734eb8e

Please sign in to comment.