diff --git a/CHANGELOG.md b/CHANGELOG.md index 36d142297d3..d627fa92bd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,15 +8,6 @@ daylight saving changes that are not precisely one hour. (No change on the Web which uses the JavaScript `Date` object.) -### Dart VM - -* **Breaking Change** [#45071][]: `Dart_NewWeakPersistentHandle`'s and - `Dart_NewFinalizableHandle`'s `object` parameter no longer accepts - `Pointer`s and subtypes of `Struct`. Expandos no longer accept - `Pointer`s and subtypes of `Struct`s. - -[#45071]: https://github.com/dart-lang/sdk/issues/45071 - ## 2.13.0 ### Language diff --git a/pkg/vm/lib/transformations/ffi_use_sites.dart b/pkg/vm/lib/transformations/ffi_use_sites.dart index e66eec0a2fb..52075cec57b 100644 --- a/pkg/vm/lib/transformations/ffi_use_sites.dart +++ b/pkg/vm/lib/transformations/ffi_use_sites.dart @@ -834,9 +834,7 @@ class _FfiUseSiteTransformer extends FfiTransformer { : null; } - if (!nativeTypesClasses.contains(klass) && - klass != arrayClass && - klass != arraySizeClass) { + if (!nativeTypesClasses.contains(klass) && klass != arrayClass) { for (final parent in nativeTypesClasses) { if (hierarchy.isSubtypeOf(klass, parent)) { return parent; diff --git a/runtime/include/dart_api.h b/runtime/include/dart_api.h index 3fce5e6cffc..b4415cff245 100644 --- a/runtime/include/dart_api.h +++ b/runtime/include/dart_api.h @@ -475,7 +475,7 @@ DART_EXPORT void Dart_DeletePersistentHandle(Dart_PersistentHandle object); * * Requires there to be a current isolate. * - * \param object An object with identity. + * \param object An object. * \param peer A pointer to a native object or NULL. This value is * provided to callback when it is invoked. * \param external_allocation_size The number of externally allocated @@ -531,7 +531,7 @@ DART_EXPORT void Dart_UpdateExternalSize(Dart_WeakPersistentHandle object, * * Requires there to be a current isolate. * - * \param object An object with identity. + * \param object An object. * \param peer A pointer to a native object or NULL. This value is * provided to callback when it is invoked. * \param external_allocation_size The number of externally allocated diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index a87928a224d..206aad7c013 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -991,23 +991,6 @@ DART_EXPORT void Dart_SetPersistentHandle(Dart_PersistentHandle obj1, obj1_ref->set_ptr(obj2_ref); } -// TODO(https://dartbug.com/38491): Reject Unions here as well. -static bool IsFfiStruct(Thread* T, const Object& obj) { - if (obj.IsNull()) { - return false; - } - - // CFE guarantees we can only have direct subclasses of `Struct` - // (no implementations or indirect subclasses are allowed). - const auto& klass = Class::Handle(Z, obj.clazz()); - const auto& super_klass = Class::Handle(Z, klass.SuperClass()); - if (super_klass.Name() != Symbols::Struct().ptr()) { - return false; - } - const auto& library = Library::Handle(Z, super_klass.library()); - return library.url() == Symbols::DartFfi().ptr(); -} - static Dart_WeakPersistentHandle AllocateWeakPersistentHandle( Thread* thread, const Object& ref, @@ -1017,13 +1000,6 @@ static Dart_WeakPersistentHandle AllocateWeakPersistentHandle( if (!ref.ptr()->IsHeapObject()) { return NULL; } - if (ref.IsPointer()) { - return NULL; - } - if (IsFfiStruct(thread, ref)) { - return NULL; - } - FinalizablePersistentHandle* finalizable_ref = FinalizablePersistentHandle::New(thread->isolate_group(), ref, peer, callback, external_allocation_size, @@ -1032,14 +1008,16 @@ static Dart_WeakPersistentHandle AllocateWeakPersistentHandle( } static Dart_WeakPersistentHandle AllocateWeakPersistentHandle( - Thread* T, + Thread* thread, Dart_Handle object, void* peer, intptr_t external_allocation_size, Dart_HandleFinalizer callback) { - const auto& ref = Object::Handle(Z, Api::UnwrapHandle(object)); - return AllocateWeakPersistentHandle(T, ref, peer, external_allocation_size, - callback); + REUSABLE_OBJECT_HANDLESCOPE(thread); + Object& ref = thread->ObjectHandle(); + ref = Api::UnwrapHandle(object); + return AllocateWeakPersistentHandle(thread, ref, peer, + external_allocation_size, callback); } static Dart_FinalizableHandle AllocateFinalizableHandle( @@ -1051,12 +1029,6 @@ static Dart_FinalizableHandle AllocateFinalizableHandle( if (!ref.ptr()->IsHeapObject()) { return NULL; } - if (ref.IsPointer()) { - return NULL; - } - if (IsFfiStruct(thread, ref)) { - return NULL; - } FinalizablePersistentHandle* finalizable_ref = FinalizablePersistentHandle::New(thread->isolate_group(), ref, peer, @@ -1066,13 +1038,15 @@ static Dart_FinalizableHandle AllocateFinalizableHandle( } static Dart_FinalizableHandle AllocateFinalizableHandle( - Thread* T, + Thread* thread, Dart_Handle object, void* peer, intptr_t external_allocation_size, Dart_HandleFinalizer callback) { - const auto& ref = Object::Handle(Z, Api::UnwrapHandle(object)); - return AllocateFinalizableHandle(T, ref, peer, external_allocation_size, + REUSABLE_OBJECT_HANDLESCOPE(thread); + Object& ref = thread->ObjectHandle(); + ref = Api::UnwrapHandle(object); + return AllocateFinalizableHandle(thread, ref, peer, external_allocation_size, callback); } @@ -1081,13 +1055,15 @@ Dart_NewWeakPersistentHandle(Dart_Handle object, void* peer, intptr_t external_allocation_size, Dart_HandleFinalizer callback) { - DARTSCOPE(Thread::Current()); + Thread* thread = Thread::Current(); + CHECK_ISOLATE(thread->isolate()); if (callback == NULL) { return NULL; } + TransitionNativeToVM transition(thread); - return AllocateWeakPersistentHandle(T, object, peer, external_allocation_size, - callback); + return AllocateWeakPersistentHandle(thread, object, peer, + external_allocation_size, callback); } DART_EXPORT Dart_FinalizableHandle @@ -1095,13 +1071,14 @@ Dart_NewFinalizableHandle(Dart_Handle object, void* peer, intptr_t external_allocation_size, Dart_HandleFinalizer callback) { - DARTSCOPE(Thread::Current()); + Thread* thread = Thread::Current(); + CHECK_ISOLATE(thread->isolate()); if (callback == nullptr) { return nullptr; } - - return AllocateFinalizableHandle(T, object, peer, external_allocation_size, - callback); + TransitionNativeToVM transition(thread); + return AllocateFinalizableHandle(thread, object, peer, + external_allocation_size, callback); } DART_EXPORT void Dart_UpdateExternalSize(Dart_WeakPersistentHandle object, diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index 6046c6e04b4..d77bbcf6151 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -3368,35 +3368,6 @@ TEST_CASE(DartAPI_WeakPersistentHandleErrors) { Dart_NewWeakPersistentHandle(obj2, NULL, 0, NopCallback); EXPECT_EQ(ref2, static_cast(NULL)); - // Pointer object. - Dart_Handle ffi_lib = Dart_LookupLibrary(NewString("dart:ffi")); - Dart_Handle pointer_type = - Dart_GetNonNullableType(ffi_lib, NewString("Pointer"), 0, NULL); - Dart_Handle obj3 = Dart_Allocate(pointer_type); - EXPECT_VALID(obj3); - Dart_WeakPersistentHandle ref3 = - Dart_NewWeakPersistentHandle(obj3, nullptr, 0, FinalizableHandleCallback); - EXPECT_EQ(ref3, static_cast(nullptr)); - - // Subtype of Struct object. - const char* kScriptChars = R"( - import 'dart:ffi'; - - class MyStruct extends Struct { - external Pointer notEmpty; - } - )"; - Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL); - Dart_Handle my_struct_type = - Dart_GetNonNullableType(lib, NewString("MyStruct"), 0, NULL); - Dart_Handle obj4 = Dart_Allocate(my_struct_type); - EXPECT_VALID(obj4); - Dart_WeakPersistentHandle ref4 = - Dart_NewWeakPersistentHandle(obj4, nullptr, 0, FinalizableHandleCallback); - EXPECT_EQ(ref4, static_cast(nullptr)); - - // TODO(https://dartbug.com/38491): Reject Unions here as well. - Dart_ExitScope(); } @@ -3417,33 +3388,6 @@ TEST_CASE(DartAPI_FinalizableHandleErrors) { Dart_NewFinalizableHandle(obj2, nullptr, 0, FinalizableHandleCallback); EXPECT_EQ(ref2, static_cast(nullptr)); - // Pointer object. - Dart_Handle ffi_lib = Dart_LookupLibrary(NewString("dart:ffi")); - Dart_Handle pointer_type = - Dart_GetNonNullableType(ffi_lib, NewString("Pointer"), 0, NULL); - Dart_Handle obj3 = Dart_Allocate(pointer_type); - EXPECT_VALID(obj3); - Dart_FinalizableHandle ref3 = - Dart_NewFinalizableHandle(obj3, nullptr, 0, FinalizableHandleCallback); - EXPECT_EQ(ref3, static_cast(nullptr)); - - // Subtype of Struct object. - const char* kScriptChars = R"( - import 'dart:ffi'; - - class MyStruct extends Struct { - external Pointer notEmpty; - } - )"; - Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL); - Dart_Handle my_struct_type = - Dart_GetNonNullableType(lib, NewString("MyStruct"), 0, NULL); - Dart_Handle obj4 = Dart_Allocate(my_struct_type); - EXPECT_VALID(obj4); - Dart_FinalizableHandle ref4 = - Dart_NewFinalizableHandle(obj4, nullptr, 0, FinalizableHandleCallback); - EXPECT_EQ(ref4, static_cast(nullptr)); - Dart_ExitScope(); } diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc index 18d0c462374..e5220d1f381 100644 --- a/runtime/vm/kernel_loader.cc +++ b/runtime/vm/kernel_loader.cc @@ -1367,8 +1367,7 @@ void KernelLoader::LoadLibraryImportsAndExports(Library* library, "runtime"); } if (!Api::IsFfiEnabled() && - target_library.url() == Symbols::DartFfi().ptr() && - library->url() != Symbols::DartCore().ptr()) { + target_library.url() == Symbols::DartFfi().ptr()) { H.ReportError( "import of dart:ffi is not supported in the current Dart runtime"); } diff --git a/runtime/vm/object_store.h b/runtime/vm/object_store.h index 519ea489907..e741716f51a 100644 --- a/runtime/vm/object_store.h +++ b/runtime/vm/object_store.h @@ -236,6 +236,7 @@ class ObjectPointerVisitor; RW(GrowableObjectArray, ffi_callback_functions) \ RW(Class, ffi_pointer_class) \ RW(Class, ffi_native_type_class) \ + RW(Class, ffi_struct_class) \ RW(Object, ffi_as_function_internal) \ // Please remember the last entry must be referred in the 'to' function below. diff --git a/sdk/lib/_internal/vm/lib/core_patch.dart b/sdk/lib/_internal/vm/lib/core_patch.dart index d79e07dbb76..6ffdce8bf2a 100644 --- a/sdk/lib/_internal/vm/lib/core_patch.dart +++ b/sdk/lib/_internal/vm/lib/core_patch.dart @@ -49,8 +49,6 @@ import "dart:collection" import "dart:convert" show ascii, Encoding, json, latin1, utf8; -import "dart:ffi" show Pointer, Struct; - import "dart:isolate" show Isolate; import "dart:math" show Random; diff --git a/sdk/lib/_internal/vm/lib/expando_patch.dart b/sdk/lib/_internal/vm/lib/expando_patch.dart index 868e6238e66..01cbe515749 100644 --- a/sdk/lib/_internal/vm/lib/expando_patch.dart +++ b/sdk/lib/_internal/vm/lib/expando_patch.dart @@ -140,12 +140,9 @@ class Expando { if ((object == null) || (object is bool) || (object is num) || - (object is String) || - (object is Pointer) || - (object is Struct)) { - // TODO(https://dartbug.com/38491): Reject Unions here as well. + (object is String)) { throw new ArgumentError.value(object, - "Expandos are not allowed on strings, numbers, booleans, null, Pointers or Structs."); + "Expandos are not allowed on strings, numbers, booleans or null"); } } diff --git a/sdk/lib/core/expando.dart b/sdk/lib/core/expando.dart index c081df80bc5..d623579d365 100644 --- a/sdk/lib/core/expando.dart +++ b/sdk/lib/core/expando.dart @@ -6,8 +6,7 @@ part of dart.core; /// An [Expando] allows adding new properties to objects. /// -/// Does not work on numbers, strings, booleans, `null`, `dart:ffi` pointers -/// or `dart:ffi` structs. +/// Does not work on numbers, strings, booleans or `null`. /// /// An `Expando` does not hold on to the added property value after an object /// becomes inaccessible. @@ -39,15 +38,13 @@ class Expando { /// object. If the object hasn't been expanded, the method returns /// `null`. /// - /// The object must not be a number, a string, a boolean, `null`, a - /// `dart:ffi` pointer, or a `dart:ffi` struct. + /// The object must not be a number, a string or a boolean. external T? operator [](Object object); /// Sets the value of this [Expando]'s property on the given /// object. Properties can effectively be removed again by setting /// their value to `null`. /// - /// The object must not be a number, a string, a boolean, `null`, a - /// `dart:ffi` pointer, or a `dart:ffi` struct. + /// The object must not be a number, a string or a boolean. external void operator []=(Object object, T? value); } diff --git a/tests/corelib/expando_test.dart b/tests/corelib/expando_test.dart index ccc074a2bf3..02810bc858b 100644 --- a/tests/corelib/expando_test.dart +++ b/tests/corelib/expando_test.dart @@ -1,9 +1,6 @@ // Copyright (c) 2012, 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. -// -// VMOptions=--enable-ffi=true -// VMOptions=--enable-ffi=false import "package:expect/expect.dart"; diff --git a/tests/corelib_2/expando_test.dart b/tests/corelib_2/expando_test.dart index 95ae7db61dd..e2a342b8aef 100644 --- a/tests/corelib_2/expando_test.dart +++ b/tests/corelib_2/expando_test.dart @@ -1,9 +1,6 @@ // Copyright (c) 2012, 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. -// -// VMOptions=--enable-ffi=true -// VMOptions=--enable-ffi=false import "package:expect/expect.dart"; diff --git a/tests/ffi/expando_test.dart b/tests/ffi/expando_test.dart deleted file mode 100644 index 473fc6f0293..00000000000 --- a/tests/ffi/expando_test.dart +++ /dev/null @@ -1,43 +0,0 @@ -// 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. -// -// Dart test program for testing dart:ffi primitive data pointers. -// -// SharedObjects=ffi_test_functions - -import 'dart:ffi'; - -import "package:expect/expect.dart"; - -void main() { - testPointer(); - testStruct(); -} - -Expando expando = Expando('myExpando'); - -void testPointer() { - final pointer = Pointer.fromAddress(0xdeadbeef); - Expect.throws(() { - expando[pointer]; - }); - Expect.throws(() { - expando[pointer] = 1234; - }); -} - -class MyStruct extends Struct { - external Pointer notEmpty; -} - -void testStruct() { - final pointer = Pointer.fromAddress(0xdeadbeef); - final struct = pointer.ref; - Expect.throws(() { - expando[struct]; - }); - Expect.throws(() { - expando[struct] = 1234; - }); -} diff --git a/tests/ffi_2/expando_test.dart b/tests/ffi_2/expando_test.dart deleted file mode 100644 index ac143245fbf..00000000000 --- a/tests/ffi_2/expando_test.dart +++ /dev/null @@ -1,43 +0,0 @@ -// 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. -// -// Dart test program for testing dart:ffi primitive data pointers. -// -// SharedObjects=ffi_test_functions - -import 'dart:ffi'; - -import "package:expect/expect.dart"; - -void main() { - testPointer(); - testStruct(); -} - -Expando expando = Expando('myExpando'); - -void testPointer() { - final pointer = Pointer.fromAddress(0xdeadbeef); - Expect.throws(() { - expando[pointer]; - }); - Expect.throws(() { - expando[pointer] = 1234; - }); -} - -class MyStruct extends Struct { - Pointer notEmpty; -} - -void testStruct() { - final pointer = Pointer.fromAddress(0xdeadbeef); - final struct = pointer.ref; - Expect.throws(() { - expando[struct]; - }); - Expect.throws(() { - expando[struct] = 1234; - }); -}