[vm/ffi] Fix FfiTrampolineData GC bug
This lets the GC visit FfiTrampolineData::c_signature again.
https://dart-review.googlesource.com/c/sdk/+/272201 stopped adding
FfiTrampolineData::c_signature to snapshots. However, instead of
skipping it manually in app_shapshot.cc, we skipped it in
raw_object.h, which also caused the GC to skip it.
This CL adds it back in as we need it in JIT snapshots. This way we
keep it consistent between AOT/JIT snapshots.
TEST=tests/ffi/regress_b_261224444_test.dart
The c signatures of FFI trampolines were not properly traced in the
precompiler, causing us to hit an assert when the classes mentioned in
those types where only referenced from a signature and not retained
for any other reason.
TEST=tests/ffi/native_assets/process_test.dart (dartkp)
Closes: https://github.com/dart-lang/sdk/issues/50678
Bug: b/261224444
Change-Id: I84fc880744c2045ea3e2ef4f37df454b80b2faeb
Cq-Include-Trybots: luci.dart.try:vm-precomp-ffi-qemu-linux-release-arm-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try,app-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/274387
Reviewed-by: Martin Kustermann <kustermann@google.com>
Auto-Submit: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
diff --git a/runtime/lib/object.cc b/runtime/lib/object.cc
index c16ddb2..768d8af 100644
--- a/runtime/lib/object.cc
+++ b/runtime/lib/object.cc
@@ -330,7 +330,8 @@
}
DEFINE_NATIVE_ENTRY(Internal_collectAllGarbage, 0, 0) {
- isolate->group()->heap()->CollectAllGarbage(GCReason::kDebugging);
+ isolate->group()->heap()->CollectAllGarbage(GCReason::kDebugging,
+ /*compact=*/true);
return Object::null();
}
diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc
index 10f4dc3..c6a80e0 100644
--- a/runtime/vm/compiler/aot/precompiler.cc
+++ b/runtime/vm/compiler/aot/precompiler.cc
@@ -1043,6 +1043,7 @@
if (!callback_target.IsNull()) {
AddFunction(callback_target, RetainReasons::kFfiCallbackTarget);
}
+ AddTypesOf(target);
}
break;
}
@@ -1109,6 +1110,10 @@
const Class& owner = Class::Handle(Z, function.Owner());
AddTypesOf(owner);
+ if (function.IsFfiTrampoline()) {
+ AddType(FunctionType::Handle(Z, function.FfiCSignature()));
+ }
+
const auto& parent_function = Function::Handle(Z, function.parent_function());
if (parent_function.IsNull()) {
return;
diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h
index 42e3969..c4f3ef3 100644
--- a/runtime/vm/raw_object.h
+++ b/runtime/vm/raw_object.h
@@ -1404,13 +1404,11 @@
private:
RAW_HEAP_OBJECT_IMPLEMENTATION(FfiTrampolineData);
- // Not traced. We don't need this info after precompilation, and FFI
- // trampolines are not supported in JIT snapshots.
- COMPRESSED_POINTER_FIELD(FunctionTypePtr, c_signature)
-
COMPRESSED_POINTER_FIELD(TypePtr, signature_type)
VISIT_FROM(signature_type)
+ COMPRESSED_POINTER_FIELD(FunctionTypePtr, c_signature)
+
// Target Dart method for callbacks, otherwise null.
COMPRESSED_POINTER_FIELD(FunctionPtr, callback_target)
diff --git a/tests/ffi/regress_b_261224444_test.dart b/tests/ffi/regress_b_261224444_test.dart
new file mode 100644
index 0000000..7f2c5de
--- /dev/null
+++ b/tests/ffi/regress_b_261224444_test.dart
@@ -0,0 +1,19 @@
+// Copyright (c) 2022, 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 'ffi_test_helpers.dart';
+
+main() {
+ // Ensure we have FfiTrampolineData in heap.
+ final foo = DynamicLibrary.process()
+ .lookup<NativeFunction<Pointer<Void> Function(IntPtr)>>("malloc")
+ .asFunction<Pointer<Void> Function(int)>();
+ print(foo);
+
+ triggerGc();
+
+ print(foo(100).address);
+}
diff --git a/tests/ffi_2/regress_b_261224444_test.dart b/tests/ffi_2/regress_b_261224444_test.dart
new file mode 100644
index 0000000..430418e
--- /dev/null
+++ b/tests/ffi_2/regress_b_261224444_test.dart
@@ -0,0 +1,21 @@
+// Copyright (c) 2022, 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=2.9
+
+import 'dart:ffi';
+
+import 'ffi_test_helpers.dart';
+
+main() {
+ // Ensure we have FfiTrampolineData in heap.
+ final foo = DynamicLibrary.process()
+ .lookup<NativeFunction<Pointer<Void> Function(IntPtr)>>("malloc")
+ .asFunction<Pointer<Void> Function(int)>();
+ print(foo);
+
+ triggerGc();
+
+ print(foo(100).address);
+}
