Skip to content

Commit

Permalink
[vm/ffi] address of operator for FFI leaf calls
Browse files Browse the repository at this point in the history
During FFI leaf calls, the Dart GC will not run. This means that we
can pass pointers into `TypedData` to FFI calls that take `Pointer`
arguments.

After this CL, we have three types of arguments that can flow into
`Pointer` argument in an FFI call:
* `Pointer`.
* `TypedData`: Any typed data including views.
* `_Compound`: A TypedData/Pointer and an offset in bytes.

The is only possible for `@Native external` functions, `asFunction`
does not support passing in `TypedData`. (See related GitHub issues
for discussion. TLDR: FFIgen should generate bindings without config.)

`.address` expressions on `TypedData` and `Array` elements do _not_
introduce bounds checks, even though `TypedData` and `Array` have
bounds information. E.g. `ffiNative(Uint8List(10)[20].address)` does
not throw.

Implementation details:

The CFE analyzes call-sites to `@Native external` functions. If the
arguments are `.address` expressions, it transforms the call site to
pass the compound or `TypedData`. If an additional offset needs to be
applied, the CFE constructs a new `_Compound` with the correct offset
in bytes.

The CFE then also creates a new `@Native external` function which have
`TypedData`s and `_Compound`s parameters. To avoid name clashes, these
functions are postfixed with `#` and `P`, `T`, or `C` for each Pointer
parameter.

TEST=pkg/vm/testcases/transformations/ffi/address_of_*

In the VM, `TypedData` arguments are passed as tagged values, and the
address is loaded inside the `FfiCallInstr`. `_Compound` arguments
turn into two IL definitions, one for the `TypedDataBase` (tagged),
and one for the offset in bytes (unboxed). The address is then loaded
inside the `FfiCallInstr` and the offset in bytes is applied.

Adding the offset in bytes required an extra temp register for ia32.
Also, it uncovered that the temp register in arm32 was conflicting
with the argument registers. However, TMP should suffice instead.

TEST=tests/ffi/address_of_array_generated_test.dart
TEST=tests/ffi/address_of_struct_generated_test.dart
TEST=tests/ffi/address_of_typeddata_generated_test.dart

Closes: #44589
Closes: #54771

CoreLibraryReviewExempt: VM only, unsupported in dart2wasm
Change-Id: I01fb428cfd6f9096a34689c2819c124a8003cb6b
Cq-Include-Trybots: dart/try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-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-x64c-try,vm-aot-win-release-x64-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-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-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/+/360882
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
  • Loading branch information
dcharkes authored and Commit Queue committed Apr 25, 2024
1 parent 8c1475c commit 4b66657
Show file tree
Hide file tree
Showing 56 changed files with 7,487 additions and 412 deletions.
22 changes: 22 additions & 0 deletions pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
Expand Up @@ -5900,6 +5900,28 @@ const MessageCode messageFfiAddressOfMustBeNative = const MessageCode(
r"""Argument to 'Native.addressOf' must be annotated with @Native.""",
);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Null> codeFfiAddressPosition = messageFfiAddressPosition;

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const MessageCode messageFfiAddressPosition = const MessageCode(
"FfiAddressPosition",
problemMessage:
r"""The '.address' expression can only be used as argument to a leaf native external call.""",
);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Null> codeFfiAddressReceiver = messageFfiAddressReceiver;

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const MessageCode messageFfiAddressReceiver = const MessageCode(
"FfiAddressReceiver",
problemMessage:
r"""The receiver of '.address' must be a concrete 'TypedData', a concrete 'TypedData' '[]', an 'Array', an 'Array' '[]', a Struct field, or a Union field.""",
correctionMessage:
r"""Change the receiver of '.address' to one of the allowed kinds.""",
);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<Message Function(String string, String name)>
templateFfiCompoundImplementsFinalizable =
Expand Down
Expand Up @@ -1743,6 +1743,12 @@ FfiCode.ABI_SPECIFIC_INTEGER_MAPPING_MISSING:
FfiCode.ABI_SPECIFIC_INTEGER_MAPPING_UNSUPPORTED:
status: noFix
since: ~2.16
FfiCode.ADDRESS_POSITION:
status: needsEvaluation
since: ~3.5
FfiCode.ADDRESS_RECEIVER:
status: needsEvaluation
since: ~3.5
FfiCode.ANNOTATION_ON_POINTER_FIELD:
status: needsFix
FfiCode.ARGUMENT_MUST_BE_A_CONSTANT:
Expand Down
17 changes: 17 additions & 0 deletions pkg/analyzer/lib/src/dart/error/ffi_code.g.dart
Expand Up @@ -65,6 +65,23 @@ class FfiCode extends AnalyzerErrorCode {
hasPublishedDocs: true,
);

/// No parameters.
static const FfiCode ADDRESS_POSITION = FfiCode(
'ADDRESS_POSITION',
"The '.address' expression can only be used as argument to a leaf native "
"external call.",
);

/// No parameters.
static const FfiCode ADDRESS_RECEIVER = FfiCode(
'ADDRESS_RECEIVER',
"The receiver of '.address' must be a concrete 'TypedData', a concrete "
"'TypedData' '[]', an 'Array', an 'Array' '[]', a Struct field, or a "
"Union field.",
correctionMessage:
"Change the receiver of '.address' to one of the allowed kinds.",
);

/// No parameters.
static const FfiCode ANNOTATION_ON_POINTER_FIELD = FfiCode(
'ANNOTATION_ON_POINTER_FIELD',
Expand Down
2 changes: 2 additions & 0 deletions pkg/analyzer/lib/src/error/error_code_values.g.dart
Expand Up @@ -590,6 +590,8 @@ const List<ErrorCode> errorCodeValues = [
FfiCode.ABI_SPECIFIC_INTEGER_MAPPING_EXTRA,
FfiCode.ABI_SPECIFIC_INTEGER_MAPPING_MISSING,
FfiCode.ABI_SPECIFIC_INTEGER_MAPPING_UNSUPPORTED,
FfiCode.ADDRESS_POSITION,
FfiCode.ADDRESS_RECEIVER,
FfiCode.ANNOTATION_ON_POINTER_FIELD,
FfiCode.ARGUMENT_MUST_BE_A_CONSTANT,
FfiCode.ARGUMENT_MUST_BE_NATIVE,
Expand Down
178 changes: 178 additions & 0 deletions pkg/analyzer/lib/src/generated/ffi_verifier.dart
Expand Up @@ -35,6 +35,37 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
static const _nativeCallable = 'NativeCallable';
static const _opaqueClassName = 'Opaque';

static const _addressOfExtensionNames = {
..._addressOfCompoundExtensionNames,
..._addressOfPrimitiveExtensionNames,
..._addressOfTypedDataExtensionNames,
};

static const _addressOfCompoundExtensionNames = {
'ArrayAddress',
'StructAddress',
'UnionAddress',
};

static const _addressOfPrimitiveExtensionNames = {
'BoolAddress',
'DoubleAddress',
'IntAddress',
};

static const _addressOfTypedDataExtensionNames = {
'Float32ListAddress',
'Float64ListAddress',
'Int16ListAddress',
'Int32ListAddress',
'Int64ListAddress',
'Int8ListAddress',
'Uint16ListAddress',
'Uint32ListAddress',
'Uint64ListAddress',
'Uint8ListAddress',
};

static const Set<String> _primitiveIntegerNativeTypesFixedSize = {
'Int8',
'Int16',
Expand Down Expand Up @@ -346,6 +377,10 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
if (element.name == 'ref') {
_validateRefPrefixedIdentifier(node);
}
} else if (enclosingElement.isAddressOfExtension) {
if (element.name == 'address') {
_validateAddressPrefixedIdentifier(node);
}
}
}
super.visitPrefixedIdentifier(node);
Expand All @@ -361,6 +396,10 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
if (element.name == 'ref') {
_validateRefPropertyAccess(node);
}
} else if (enclosingElement.isAddressOfExtension) {
if (element.name == 'address') {
_validateAddressPropertyAccess(node);
}
}
}
super.visitPropertyAccess(node);
Expand Down Expand Up @@ -986,6 +1025,84 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
}
}

/// Check that .address is only used in argument lists passed to native leaf
/// calls.
void _validateAddressPosition(Expression node, AstNode errorNode) {
final parent = node.parent;
final grandParent = parent?.parent;
if (parent is! ArgumentList ||
grandParent is! MethodInvocation ||
!grandParent.isNativeLeafInvocation) {
_errorReporter.atNode(
errorNode,
FfiCode.ADDRESS_POSITION,
);
}
}

void _validateAddressPrefixedIdentifier(PrefixedIdentifier node) {
final errorNode = node.identifier;
_validateAddressPosition(node, errorNode);
final extensionName = node.staticElement?.enclosingElement?.name;
final receiver = node.prefix;
_validateAddressReceiver(node, extensionName, receiver, errorNode);
}

void _validateAddressPropertyAccess(PropertyAccess node) {
final errorNode = node.propertyName;
_validateAddressPosition(node, errorNode);
final extensionName =
node.propertyName.staticElement?.enclosingElement?.name;
final receiver = node.target;
_validateAddressReceiver(node, extensionName, receiver, errorNode);
}

void _validateAddressReceiver(
Expression node,
String? extensionName,
Expression? receiver,
AstNode errorNode,
) {
if (_addressOfCompoundExtensionNames.contains(extensionName) ||
_addressOfTypedDataExtensionNames.contains(extensionName)) {
return; // Only primitives need their reciever checked.
}
if (receiver == null) {
return;
}
switch (receiver) {
case IndexExpression _:
// Array or TypedData element.
final arrayOrTypedData = receiver.target;
final type = arrayOrTypedData?.staticType;
if (type?.isArray ?? false) {
return;
}
if (type?.isTypedData ?? false) {
return;
}
case PrefixedIdentifier _:
// Struct or Union field.
final compound = receiver.prefix;
final type = compound.staticType;
if (type?.isCompoundSubtype ?? false) {
return;
}
case PropertyAccess _:
// Struct or Union field.
final compound = receiver.target;
final type = compound?.staticType;
if (type?.isCompoundSubtype ?? false) {
return;
}
default:
}
_errorReporter.atNode(
errorNode,
FfiCode.ADDRESS_RECEIVER,
);
}

void _validateAllocate(FunctionExpressionInvocation node) {
var typeArgumentTypes = node.typeArgumentTypes;
if (typeArgumentTypes == null || typeArgumentTypes.length != 1) {
Expand Down Expand Up @@ -1959,6 +2076,22 @@ extension on ElementAnnotation {
// forwarding factory instead of the forwarded constructor.
}

/// @Native(isLeaf: true)
bool get isNativeLeaf {
final annotationValue = computeConstantValue();
final annotationType = annotationValue?.type; // Native<T>
if (annotationValue == null || annotationType is! InterfaceType) {
return false;
}
if (!annotationValue.isNative) {
return false;
}
return annotationValue
.getField(FfiVerifier._isLeafParamName)
?.toBoolValue() ??
false;
}

bool get isPacked {
final element = this.element;
return element is ConstructorElement &&
Expand All @@ -1973,6 +2106,44 @@ extension on ElementAnnotation {
}
}

extension on FunctionElement {
/// @Native(isLeaf: true) external function.
bool get isNativeLeaf {
for (final annotation in metadata) {
if (annotation.isNativeLeaf) {
return true;
}
}
return false;
}
}

extension on MethodElement {
/// @Native(isLeaf: true) external function.
bool get isNativeLeaf {
for (final annotation in metadata) {
if (annotation.isNativeLeaf) {
return true;
}
}
return false;
}
}

extension on MethodInvocation {
/// Calls @Native(isLeaf: true) external function.
bool get isNativeLeafInvocation {
final element = methodName.staticElement;
if (element is FunctionElement) {
return element.isNativeLeaf;
}
if (element is MethodElement) {
return element.isNativeLeaf;
}
return false;
}
}

extension on DartObject {
bool get isDefaultAsset {
return switch (type) {
Expand Down Expand Up @@ -2017,6 +2188,13 @@ extension on Element? {
return element is ClassElement && element.supertype.isAbiSpecificInteger;
}

bool get isAddressOfExtension {
final element = this;
return element is ExtensionElement &&
element.isFfiExtension &&
FfiVerifier._addressOfExtensionNames.contains(element.name);
}

/// Return `true` if this represents the extension `AllocatorAlloc`.
bool get isAllocatorExtension {
var element = this;
Expand Down
9 changes: 9 additions & 0 deletions pkg/analyzer/messages.yaml
Expand Up @@ -18697,6 +18697,15 @@ FfiCode:
const C();
}
```
ADDRESS_POSITION:
problemMessage: "The '.address' expression can only be used as argument to a leaf native external call."
hasPublishedDocs: false
comment: No parameters.
ADDRESS_RECEIVER:
problemMessage: "The receiver of '.address' must be a concrete 'TypedData', a concrete 'TypedData' '[]', an 'Array', an 'Array' '[]', a Struct field, or a Union field."
correctionMessage: Change the receiver of '.address' to one of the allowed kinds.
hasPublishedDocs: false
comment: No parameters.
ANNOTATION_ON_POINTER_FIELD:
problemMessage: "Fields in a struct class whose type is 'Pointer' shouldn't have any annotations."
correctionMessage: Try removing the annotation.
Expand Down
2 changes: 2 additions & 0 deletions pkg/front_end/messages.status
Expand Up @@ -380,6 +380,8 @@ FastaUsageShort/analyzerCode: Fail
FastaUsageShort/example: Fail
FfiAbiSpecificIntegerInvalid/analyzerCode: Fail
FfiAbiSpecificIntegerMappingInvalid/analyzerCode: Fail
FfiAddressPosition/analyzerCode: Fail
FfiAddressReceiver/analyzerCode: Fail
FfiCompoundImplementsFinalizable/analyzerCode: Fail
FfiCreateOfStructOrUnion/analyzerCode: Fail
FfiDartTypeMismatch/analyzerCode: Fail
Expand Down
11 changes: 11 additions & 0 deletions pkg/front_end/messages.yaml
Expand Up @@ -5039,6 +5039,17 @@ FfiAbiSpecificIntegerMappingInvalid:
problemMessage: "Classes extending 'AbiSpecificInteger' must have exactly one 'AbiSpecificIntegerMapping' annotation specifying the mapping from ABI to a NativeType integer with a fixed size."
external: test/ffi_test.dart

FfiAddressPosition:
# Used by dart:ffi
problemMessage: "The '.address' expression can only be used as argument to a leaf native external call."
external: test/ffi_test.dart

FfiAddressReceiver:
# Used by dart:ffi
problemMessage: "The receiver of '.address' must be a concrete 'TypedData', a concrete 'TypedData' '[]', an 'Array', an 'Array' '[]', a Struct field, or a Union field."
correctionMessage: "Change the receiver of '.address' to one of the allowed kinds."
external: test/ffi_test.dart

FfiCreateOfStructOrUnion:
# Used by dart:ffi
problemMessage: "Subclasses of 'Struct' and 'Union' are backed by native memory, and can't be instantiated by a generative constructor. Try allocating it via allocation, or load from a 'Pointer'."
Expand Down
1 change: 1 addition & 0 deletions pkg/front_end/test/spell_checking_list_messages.txt
Expand Up @@ -143,6 +143,7 @@ tojs
trusttypes
type's
type3.#name
typeddata
typeof
u
unavailable
Expand Down

0 comments on commit 4b66657

Please sign in to comment.