[vm/ffi] Fix snapshot serialization of FFI objects.
Change-Id: I78557211da79f5282339fbcf30b3f81ab56cb361
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106345
Commit-Queue: Samir Jindel <sjindel@google.com>
Auto-Submit: Samir Jindel <sjindel@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
diff --git a/runtime/vm/clustered_snapshot.cc b/runtime/vm/clustered_snapshot.cc
index 39249519..5796ec3 100644
--- a/runtime/vm/clustered_snapshot.cc
+++ b/runtime/vm/clustered_snapshot.cc
@@ -6,6 +6,7 @@
#include "platform/assert.h"
#include "vm/bootstrap.h"
+#include "vm/class_id.h"
#include "vm/compiler/backend/code_statistics.h"
#include "vm/compiler/frontend/bytecode_reader.h"
#include "vm/compiler/relocation.h"
@@ -768,6 +769,74 @@
};
#if !defined(DART_PRECOMPILED_RUNTIME)
+class FfiTrampolineDataSerializationCluster : public SerializationCluster {
+ public:
+ FfiTrampolineDataSerializationCluster()
+ : SerializationCluster("FfiTrampolineData") {}
+ ~FfiTrampolineDataSerializationCluster() {}
+
+ void Trace(Serializer* s, RawObject* object) {
+ RawFfiTrampolineData* data = FfiTrampolineData::RawCast(object);
+ objects_.Add(data);
+ PushFromTo(data);
+ }
+
+ void WriteAlloc(Serializer* s) {
+ s->WriteCid(kFfiTrampolineDataCid);
+ const intptr_t count = objects_.length();
+ s->WriteUnsigned(count);
+ for (intptr_t i = 0; i < count; i++) {
+ s->AssignRef(objects_[i]);
+ }
+ }
+
+ void WriteFill(Serializer* s) {
+ intptr_t count = objects_.length();
+ for (intptr_t i = 0; i < count; i++) {
+ RawFfiTrampolineData* const data = objects_[i];
+ AutoTraceObject(data);
+ WriteFromTo(data);
+
+ // TODO(37295): FFI callbacks shouldn't be written to a snapshot. They
+ // should only be referenced by the callback registry in Thread.
+ ASSERT(data->ptr()->callback_id_ == 0);
+ }
+ }
+
+ private:
+ GrowableArray<RawFfiTrampolineData*> objects_;
+};
+#endif // !DART_PRECOMPILED_RUNTIME
+
+class FfiTrampolineDataDeserializationCluster : public DeserializationCluster {
+ public:
+ FfiTrampolineDataDeserializationCluster() {}
+ ~FfiTrampolineDataDeserializationCluster() {}
+
+ void ReadAlloc(Deserializer* d) {
+ start_index_ = d->next_index();
+ PageSpace* old_space = d->heap()->old_space();
+ intptr_t count = d->ReadUnsigned();
+ for (intptr_t i = 0; i < count; i++) {
+ d->AssignRef(
+ AllocateUninitialized(old_space, FfiTrampolineData::InstanceSize()));
+ }
+ stop_index_ = d->next_index();
+ }
+
+ void ReadFill(Deserializer* d) {
+ for (intptr_t id = start_index_; id < stop_index_; id++) {
+ RawFfiTrampolineData* data =
+ reinterpret_cast<RawFfiTrampolineData*>(d->Ref(id));
+ Deserializer::InitializeHeader(data, kFfiTrampolineDataCid,
+ FfiTrampolineData::InstanceSize());
+ ReadFromTo(data);
+ data->ptr()->callback_id_ = 0;
+ }
+ }
+};
+
+#if !defined(DART_PRECOMPILED_RUNTIME)
class RedirectionDataSerializationCluster : public SerializationCluster {
public:
RedirectionDataSerializationCluster()
@@ -4308,6 +4377,8 @@
return new (Z) SignatureDataSerializationCluster();
case kRedirectionDataCid:
return new (Z) RedirectionDataSerializationCluster();
+ case kFfiTrampolineDataCid:
+ return new (Z) FfiTrampolineDataSerializationCluster();
case kFieldCid:
return new (Z) FieldSerializationCluster();
case kScriptCid:
@@ -4944,6 +5015,8 @@
return new (Z) SignatureDataDeserializationCluster();
case kRedirectionDataCid:
return new (Z) RedirectionDataDeserializationCluster();
+ case kFfiTrampolineDataCid:
+ return new (Z) FfiTrampolineDataDeserializationCluster();
case kFieldCid:
return new (Z) FieldDeserializationCluster();
case kScriptCid:
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 4294af9..ad74bbe 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -8271,7 +8271,7 @@
Object::Allocate(FfiTrampolineData::kClassId,
FfiTrampolineData::InstanceSize(), Heap::kOld);
RawFfiTrampolineData* data = reinterpret_cast<RawFfiTrampolineData*>(raw);
- data->ptr()->callback_id_ = -1;
+ data->ptr()->callback_id_ = 0;
return data;
}
diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h
index 91ff01e..10f1b00 100644
--- a/runtime/vm/raw_object.h
+++ b/runtime/vm/raw_object.h
@@ -1034,13 +1034,17 @@
RawFunction* callback_target_;
VISIT_TO(RawObject*, callback_target_);
+ RawObject** to_snapshot(Snapshot::Kind kind) { return to(); }
- // Callback id for callbacks, otherwise 0.
+ // Callback id for callbacks.
//
// The callbacks ids are used so that native callbacks can lookup their own
// code objects, since native code doesn't pass code objects into function
// calls. The callback id is also used to for verifying that callbacks are
// called on the correct isolate. See DLRT_VerifyCallbackIsolate for details.
+ //
+ // Will be 0 for non-callbacks. Check 'callback_target_' to determine if this
+ // is a callback or not.
uint32_t callback_id_;
};
diff --git a/runtime/vm/snapshot.cc b/runtime/vm/snapshot.cc
index 3824991..65258de 100644
--- a/runtime/vm/snapshot.cc
+++ b/runtime/vm/snapshot.cc
@@ -1134,7 +1134,12 @@
#define SNAPSHOT_WRITE(clazz) case kFfi##clazz##Cid:
- CLASS_LIST_FFI(SNAPSHOT_WRITE) { UNREACHABLE(); }
+ CLASS_LIST_FFI(SNAPSHOT_WRITE) {
+ SetWriteException(Exceptions::kArgument,
+ "Native objects (from dart:ffi) such as Pointers and "
+ "Structs cannot be passed between isolates.");
+ UNREACHABLE();
+ }
#undef SNAPSHOT_WRITE
default:
break;
diff --git a/tests/ffi/ffi.status b/tests/ffi/ffi.status
index 0e982e4..61d2ce6 100644
--- a/tests/ffi/ffi.status
+++ b/tests/ffi/ffi.status
@@ -13,14 +13,6 @@
function_structs_test: Skip
structs_test: Skip
-# dartbug.com/35934
-[ $compiler == app_jitk ]
-dynamic_library_test: Skip
-function_callbacks_test: Skip
-function_structs_test: Skip
-function_test: Skip
-negative_function_test: Skip
-
[ $arch == x64 || $arch == arm64 || $arch == simdbc64 ]
enable_structs_test: SkipByDesign # Tests that structs don't work on 32-bit systems.
@@ -44,3 +36,10 @@
[ $arch == simdbc64 ]
function_callbacks_test: Skip # Issue 37140
+
+# These tests trigger and catch an abort (intentionally) and terminate the VM
+# before it can generate a snapshot.
+[ $compiler == app_jitk ]
+function_callbacks_test/01: Skip
+function_callbacks_test/02: Skip
+function_callbacks_test/03: Skip
diff --git a/tests/ffi/snapshot_test.dart b/tests/ffi/snapshot_test.dart
new file mode 100644
index 0000000..02b4cc4
--- /dev/null
+++ b/tests/ffi/snapshot_test.dart
@@ -0,0 +1,24 @@
+// Copyright (c) 2019, 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.
+//
+// Checks that the VM throws an appropriate exception when FFI objects are
+// passed between isolates.
+
+import 'dart:async';
+import 'dart:ffi';
+import 'dart:io';
+import 'dart:isolate';
+
+import 'package:expect/expect.dart';
+
+main(args) async {
+ try {
+ await Isolate.spawn(print, fromAddress<Pointer<Void>>(1));
+ } catch (e) {
+ Expect.type<ArgumentError>(e);
+ return;
+ }
+
+ throw "Test didn't throw an exception!";
+}