languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-14 08:41:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

[cfe/ffi] Fix user-defined getters

Browse source 
https://dart-review.googlesource.com/c/sdk/+/198281 introduced support
for incremental compilation to the FfiTransform. This included reading
fields from already transformed structs and unions.

However, fields which already had been transformed are indistinguishable
from user-defined getters.

So instead of re-reading the information from the fields, for already
transformed structs, we read the information from the
`vm:ffi:struct-fields` pragma on the struct class.

Closes: https://github.com/dart-lang/sdk/issues/46004

TEST=tests/ffi/regress_46004_test.dart
TEST=pkg/front_end/testcases/incremental/regress_46004.yaml

Change-Id: Iebffda037913e71349bba9685dc16e2f478a0e7b
Cq-Include-Trybots: luci.dart.try:vm-kernel-win-debug-x64-try,front-end-nnbd-linux-release-x64-try,front-end-linux-release-x64-try,vm-ffi-android-debug-arm64-try,vm-precomp-ffi-qemu-linux-release-arm-try
Fixed: 46004
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200640
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
This commit is contained in:
Daco Harkes 2021-05-20 13:12:22 +00:00 committed by commit-bot@chromium.org
parent 14d5ca1721
commit f18c1bfb84
6 changed files with 330 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
pkg/front_end/testcases/incremental/regress_46004.yaml Normal file
View file

@ -0,0 +1,35 @@
# Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
# for details. All rights reserved. Use of this source code is governed by a
# BSD-style license that can be found in the LICENSE.md file.
type: newworld
worlds:
- entry: main.dart
experiments: non-nullable
sources:
main.dart: |
import 'dart:ffi';
import 'lib.dart';
class X extends Struct {
external COMObject xx;
}
lib.dart: |
import 'dart:ffi';
class COMObject extends Struct {
external Pointer<IntPtr> lpVtbl;
// This should not be interpreted as a native field.
// Neither the first nor the second compilation.
Pointer<IntPtr> get vtable => Pointer.fromAddress(lpVtbl.value);
}
expectedLibraryCount: 2
- entry: main.dart
experiments: non-nullable
worldType: updated
expectInitializeFromDill: false
invalidate:
- main.dart
expectedLibraryCount: 2
expectsRebuildBodiesOnly: false

68
pkg/front_end/testcases/incremental/regress_46004.yaml.world.1.expect Normal file
View file

@ -0,0 +1,68 @@
main = <No Member>;
library from "org-dartlang-test:///lib.dart" as lib {
import "dart:ffi";
@#C6
class COMObject extends dart.ffi::Struct {
synthetic constructor •() → lib::COMObject
: super dart.ffi::Struct::•()
;
constructor #fromTypedDataBase(dart.core::Object #typedDataBase) → lib::COMObject
: super dart.ffi::Struct::_fromTypedDataBase(#typedDataBase)
;
get lpVtbl() → dart.ffi::Pointer<dart.ffi::IntPtr>
return dart.ffi::_fromAddress<dart.ffi::IntPtr>(dart.ffi::_loadIntPtr(this.{dart.ffi::_Compound::_typedDataBase}, (#C8).{dart.core::List::[]}(dart.ffi::_abi())));
set lpVtbl(dart.ffi::Pointer<dart.ffi::IntPtr> #externalFieldValue) → void
return dart.ffi::_storeIntPtr(this.{dart.ffi::_Compound::_typedDataBase}, (#C8).{dart.core::List::[]}(dart.ffi::_abi()), #externalFieldValue.{dart.ffi::Pointer::address});
get vtable() → dart.ffi::Pointer<dart.ffi::IntPtr>
return dart.ffi::Pointer::fromAddress<dart.ffi::IntPtr>(dart.ffi::IntPtrPointer|get#value(this.{lib::COMObject::lpVtbl}));
@#C10
static get #sizeOf() → dart.core::int*
return (#C13).{dart.core::List::[]}(dart.ffi::_abi());
}
}
library from "org-dartlang-test:///main.dart" as main {
import "dart:ffi";
import "org-dartlang-test:///lib.dart";
@#C17
class X extends dart.ffi::Struct {
synthetic constructor •() → main::X
: super dart.ffi::Struct::•()
;
constructor #fromTypedDataBase(dart.core::Object #typedDataBase) → main::X
: super dart.ffi::Struct::_fromTypedDataBase(#typedDataBase)
;
get xx() → lib::COMObject
return new lib::COMObject::#fromTypedDataBase( block {
dart.core::Object #typedDataBase = this.{dart.ffi::_Compound::_typedDataBase};
dart.core::int #offset = (#C8).{dart.core::List::[]}(dart.ffi::_abi());
} =>#typedDataBase is dart.ffi::Pointer<dynamic> ?{dart.core::Object} dart.ffi::_fromAddress<lib::COMObject>(#typedDataBase.{dart.ffi::Pointer::address}.{dart.core::num::+}(#offset)) : let dart.typed_data::TypedData #typedData = dart._internal::unsafeCast<dart.typed_data::TypedData>(#typedDataBase) in #typedData.{dart.typed_data::TypedData::buffer}.{dart.typed_data::ByteBuffer::asUint8List}(#typedData.{dart.typed_data::TypedData::offsetInBytes}.{dart.core::num::+}(#offset), (#C13).{dart.core::List::[]}(dart.ffi::_abi())));
set xx(lib::COMObject #externalFieldValue) → void
return dart.ffi::_memCopy(this.{dart.ffi::_Compound::_typedDataBase}, (#C8).{dart.core::List::[]}(dart.ffi::_abi()), #externalFieldValue.{dart.ffi::_Compound::_typedDataBase}, #C7, (#C13).{dart.core::List::[]}(dart.ffi::_abi()));
@#C10
static get #sizeOf() → dart.core::int*
return (#C13).{dart.core::List::[]}(dart.ffi::_abi());
}
}
constants {
#C1 = "vm:ffi:struct-fields"
#C2 = TypeLiteralConstant(dart.ffi::Pointer<dart.ffi::NativeType>)
#C3 = <dart.core::Type>[#C2]
#C4 = null
#C5 = dart.ffi::_FfiStructLayout {fieldTypes:#C3, packing:#C4}
#C6 = dart.core::pragma {name:#C1, options:#C5}
#C7 = 0
#C8 = <dart.core::int*>[#C7, #C7, #C7]
#C9 = "vm:prefer-inline"
#C10 = dart.core::pragma {name:#C9, options:#C4}
#C11 = 8
#C12 = 4
#C13 = <dart.core::int*>[#C11, #C12, #C12]
#C14 = TypeLiteralConstant(lib::COMObject)
#C15 = <dart.core::Type>[#C14]
#C16 = dart.ffi::_FfiStructLayout {fieldTypes:#C15, packing:#C4}
#C17 = dart.core::pragma {name:#C1, options:#C16}
}

68
pkg/front_end/testcases/incremental/regress_46004.yaml.world.2.expect Normal file
View file

@ -0,0 +1,68 @@
main = <No Member>;
library from "org-dartlang-test:///lib.dart" as lib {
import "dart:ffi";
@#C6
class COMObject extends dart.ffi::Struct {
synthetic constructor •() → lib::COMObject
: super dart.ffi::Struct::•()
;
constructor #fromTypedDataBase(dart.core::Object #typedDataBase) → lib::COMObject
: super dart.ffi::Struct::_fromTypedDataBase(#typedDataBase)
;
get lpVtbl() → dart.ffi::Pointer<dart.ffi::IntPtr>
return dart.ffi::_fromAddress<dart.ffi::IntPtr>(dart.ffi::_loadIntPtr(this.{dart.ffi::_Compound::_typedDataBase}, (#C8).{dart.core::List::[]}(dart.ffi::_abi())));
set lpVtbl(dart.ffi::Pointer<dart.ffi::IntPtr> #externalFieldValue) → void
return dart.ffi::_storeIntPtr(this.{dart.ffi::_Compound::_typedDataBase}, (#C8).{dart.core::List::[]}(dart.ffi::_abi()), #externalFieldValue.{dart.ffi::Pointer::address});
get vtable() → dart.ffi::Pointer<dart.ffi::IntPtr>
return dart.ffi::Pointer::fromAddress<dart.ffi::IntPtr>(dart.ffi::IntPtrPointer|get#value(this.{lib::COMObject::lpVtbl}));
@#C10
static get #sizeOf() → dart.core::int*
return (#C13).{dart.core::List::[]}(dart.ffi::_abi());
}
}
library from "org-dartlang-test:///main.dart" as main {
import "dart:ffi";
import "org-dartlang-test:///lib.dart";
@#C17
class X extends dart.ffi::Struct {
synthetic constructor •() → main::X
: super dart.ffi::Struct::•()
;
constructor #fromTypedDataBase(dart.core::Object #typedDataBase) → main::X
: super dart.ffi::Struct::_fromTypedDataBase(#typedDataBase)
;
get xx() → lib::COMObject
return new lib::COMObject::#fromTypedDataBase( block {
dart.core::Object #typedDataBase = this.{dart.ffi::_Compound::_typedDataBase};
dart.core::int #offset = (#C8).{dart.core::List::[]}(dart.ffi::_abi());
} =>#typedDataBase is dart.ffi::Pointer<dynamic> ?{dart.core::Object} dart.ffi::_fromAddress<lib::COMObject>(#typedDataBase.{dart.ffi::Pointer::address}.{dart.core::num::+}(#offset)) : let dart.typed_data::TypedData #typedData = dart._internal::unsafeCast<dart.typed_data::TypedData>(#typedDataBase) in #typedData.{dart.typed_data::TypedData::buffer}.{dart.typed_data::ByteBuffer::asUint8List}(#typedData.{dart.typed_data::TypedData::offsetInBytes}.{dart.core::num::+}(#offset), (#C13).{dart.core::List::[]}(dart.ffi::_abi())));
set xx(lib::COMObject #externalFieldValue) → void
return dart.ffi::_memCopy(this.{dart.ffi::_Compound::_typedDataBase}, (#C8).{dart.core::List::[]}(dart.ffi::_abi()), #externalFieldValue.{dart.ffi::_Compound::_typedDataBase}, #C7, (#C13).{dart.core::List::[]}(dart.ffi::_abi()));
@#C10
static get #sizeOf() → dart.core::int*
return (#C13).{dart.core::List::[]}(dart.ffi::_abi());
}
}
constants {
#C1 = "vm:ffi:struct-fields"
#C2 = TypeLiteralConstant(dart.ffi::Pointer<dart.ffi::NativeType>)
#C3 = <dart.core::Type>[#C2]
#C4 = null
#C5 = dart.ffi::_FfiStructLayout {fieldTypes:#C3, packing:#C4}
#C6 = dart.core::pragma {name:#C1, options:#C5}
#C7 = 0
#C8 = <dart.core::int*>[#C7, #C7, #C7]
#C9 = "vm:prefer-inline"
#C10 = dart.core::pragma {name:#C9, options:#C4}
#C11 = 8
#C12 = 4
#C13 = <dart.core::int*>[#C11, #C12, #C12]
#C14 = TypeLiteralConstant(lib::COMObject)
#C15 = <dart.core::Type>[#C14]
#C16 = dart.ffi::_FfiStructLayout {fieldTypes:#C15, packing:#C4}
#C17 = dart.core::pragma {name:#C1, options:#C16}
}

125
pkg/vm/lib/transformations/ffi_definitions.dart
View file

@ -125,6 +125,13 @@ class _FfiDefinitionTransformer extends FfiTransformer {
///
/// Works both for transformed and non-transformed compound classes.
Set<Class> _compoundClassDependencies(Class node) {
final fieldTypes = _compoundAnnotatedFields(node);
if (fieldTypes != null) {
// Transformed classes.
return _compoundAnnotatedDependencies(fieldTypes);
}
// Non-tranformed classes.
final dependencies = <Class>{};
final membersWithAnnotations =
_compoundFieldMembers(node, includeSetters: false);
@ -199,20 +206,25 @@ class _FfiDefinitionTransformer extends FfiTransformer {
} else {
// Only visit the ones without cycles.
final clazz = component.single;
final compoundData = _findFields(clazz);
final compoundType = compoundData.compoundType;
compoundCache[clazz] = compoundType;
final indexedClass = indexedCompoundClasses[clazz];
if (transformCompounds.contains(clazz) &&
compoundType is! InvalidNativeTypeCfe) {
// Only visit if it has not been transformed yet, and its fields
// are valid.
_replaceFields(clazz, indexedClass, compoundData);
_addSizeOfField(clazz, indexedClass, compoundType.size);
final mustBeTransformed = (transformCompoundsInvalid.contains(clazz) ||
transformCompounds.contains(clazz));
if (!mustBeTransformed) {
compoundCache[clazz] = _compoundAnnotatedNativeTypeCfe(clazz);
} else {
final compoundData = _findFields(clazz);
final compoundType = compoundData.compoundType;
compoundCache[clazz] = compoundType;
final indexedClass = indexedCompoundClasses[clazz];
if (transformCompounds.contains(clazz) &&
compoundType is! InvalidNativeTypeCfe) {
// Only replace fields if valid.
_replaceFields(clazz, indexedClass, compoundData);
_addSizeOfField(clazz, indexedClass, compoundType.size);
} else {
// Do add a sizeOf field even if invalid.
_addSizeOfField(clazz, indexedClass);
}
changedStructureNotifier?.registerClassMemberChange(clazz);
} else if (transformCompoundsInvalid.contains(clazz)) {
_addSizeOfField(clazz, indexedClass);
}
}
});
@ -310,13 +322,12 @@ class _FfiDefinitionTransformer extends FfiTransformer {
/// Note that getters and setters that originate from an external field have
/// the same `fileOffset`, we always returns getters first.
///
/// This works for both transformed and non-transformed structs.
List<Member> _compoundFieldMembers(Class node,
{bool possiblyAlreadyTransformed = true, bool includeSetters = true}) {
/// This works only for non-transformed compounds.
List<Member> _compoundFieldMembers(Class node, {bool includeSetters = true}) {
assert(_compoundAnnotatedFields(node) == null);
final getterSetters = node.procedures.where((p) {
if (!possiblyAlreadyTransformed && !p.isExternal) {
// If a struct has not been transformed yet, we have the guarantee
// that getters and setters corresponding to native fields are external.
if (!p.isExternal) {
// Getters and setters corresponding to native fields are external.
return false;
}
if (p.isSetter && includeSetters) {
@ -350,8 +361,8 @@ class _FfiDefinitionTransformer extends FfiTransformer {
bool _checkFieldAnnotations(Class node, int packing) {
bool success = true;
final membersWithAnnotations = _compoundFieldMembers(node,
possiblyAlreadyTransformed: false, includeSetters: false);
final membersWithAnnotations =
_compoundFieldMembers(node, includeSetters: false);
for (final Member f in membersWithAnnotations) {
if (f is Field) {
if (f.initializer is! NullLiteral) {
@ -541,6 +552,7 @@ class _FfiDefinitionTransformer extends FfiTransformer {
node.addConstructor(ctor);
}
// Works only for non-transformed classes.
CompoundData _findFields(Class node) {
final types = <NativeTypeCfe>[];
final fields = <int, Field>{};
@ -672,6 +684,75 @@ 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) {
final constant = annotation.constant;
if (constant is InstanceConstant &&
constant.classNode == pragmaClass &&
constant.fieldValues[pragmaName.getterReference] ==
StringConstant(vmFfiStructFields)) {
return constant.fieldValues[pragmaOptions.getterReference];
}
}
}
return null;
}
Set<Class> _compoundAnnotatedDependencies(InstanceConstant layoutConstant) {
final fieldTypes = layoutConstant
.fieldValues[ffiStructLayoutTypesField.getterReference] as ListConstant;
final result = <Class>{};
for (final fieldType in fieldTypes.entries) {
if (fieldType is TypeLiteralConstant) {
final type = fieldType.type;
if (isCompoundSubtype(type)) {
final clazz = (type as InterfaceType).classNode;
result.add(clazz);
}
}
}
return result;
}
/// Must only be called if all the depencies are already in the cache.
CompoundNativeTypeCfe _compoundAnnotatedNativeTypeCfe(Class compoundClass) {
final layoutConstant = _compoundAnnotatedFields(compoundClass);
final fieldTypes = layoutConstant
.fieldValues[ffiStructLayoutTypesField.getterReference] as ListConstant;
final members = <NativeTypeCfe>[];
for (final fieldType in fieldTypes.entries) {
if (fieldType is TypeLiteralConstant) {
final dartType = fieldType.type;
members.add(NativeTypeCfe(this, dartType));
} else if (fieldType is InstanceConstant) {
final singleElementConstant = fieldType
.fieldValues[ffiInlineArrayElementTypeField.getterReference]
as TypeLiteralConstant;
final singleElementType =
NativeTypeCfe(this, singleElementConstant.type);
final arrayLengthConstant =
fieldType.fieldValues[ffiInlineArrayLengthField.getterReference]
as IntConstant;
final arrayLength = arrayLengthConstant.value;
members.add(ArrayNativeTypeCfe(singleElementType, arrayLength));
}
}
if (compoundClass.superclass == structClass) {
final packingConstant = layoutConstant
.fieldValues[ffiStructLayoutPackingField.getterReference];
if (packingConstant is IntConstant) {
return StructNativeTypeCfe(compoundClass, members,
packing: packingConstant.value);
}
return StructNativeTypeCfe(compoundClass, members);
}
return UnionNativeTypeCfe(compoundClass, members);
}
// packing is `int?`.
void _annoteCompoundWithFields(
Class node, List<NativeTypeCfe> types, int packing) {
@ -680,7 +761,7 @@ class _FfiDefinitionTransformer extends FfiTransformer {
node.addAnnotation(ConstantExpression(
InstanceConstant(pragmaClass.reference, [], {
pragmaName.getterReference: StringConstant("vm:ffi:struct-fields"),
pragmaName.getterReference: StringConstant(vmFfiStructFields),
pragmaOptions.getterReference:
InstanceConstant(ffiStructLayoutClass.reference, [], {
ffiStructLayoutTypesField.getterReference: ListConstant(

28
tests/ffi/regress_46004_test.dart Normal file
View file

@ -0,0 +1,28 @@
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'dart:ffi';
import "package:expect/expect.dart";
import "package:ffi/ffi.dart";
class COMObject extends Struct {
external Pointer<IntPtr> lpVtbl;
// This should not be interpreted as a native field.
Pointer<IntPtr> get vtable => Pointer.fromAddress(lpVtbl.value);
}
void main() {
Expect.equals(sizeOf<Pointer>(), sizeOf<COMObject>());
final comObjectPointer = calloc<COMObject>();
final vTablePointer = calloc<IntPtr>();
vTablePointer.value = 1234;
final comObject = comObjectPointer.ref;
comObject.lpVtbl = vTablePointer;
Expect.equals(1234, comObject.vtable.address);
calloc.free(comObjectPointer);
calloc.free(vTablePointer);
}

28
tests/ffi_2/regress_46004_test.dart Normal file
View file

@ -0,0 +1,28 @@
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'dart:ffi';
import "package:expect/expect.dart";
import "package:ffi/ffi.dart";
class COMObject extends Struct {
Pointer<IntPtr> lpVtbl;
// This should not be interpreted as a native field.
Pointer<IntPtr> get vtable => Pointer.fromAddress(lpVtbl.value);
}
void main() {
Expect.equals(sizeOf<Pointer>(), sizeOf<COMObject>());
final comObjectPointer = calloc<COMObject>();
final vTablePointer = calloc<IntPtr>();
vTablePointer.value = 1234;
final comObject = comObjectPointer.ref;
comObject.lpVtbl = vTablePointer;
Expect.equals(1234, comObject.vtable.address);
calloc.free(comObjectPointer);
calloc.free(vTablePointer);
}