Skip to content

Commit e3e355e

Browse files
dcharkesCommit Queue
authored and
Commit Queue
committed
[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: #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 <[email protected]> Auto-Submit: Daco Harkes <[email protected]> Commit-Queue: Daco Harkes <[email protected]>
1 parent 1450896 commit e3e355e

File tree

5 files changed

+49
-5
lines changed

5 files changed

+49
-5
lines changed

runtime/lib/object.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,8 @@ DEFINE_NATIVE_ENTRY(Internal_nativeEffect, 0, 1) {
330330
}
331331

332332
DEFINE_NATIVE_ENTRY(Internal_collectAllGarbage, 0, 0) {
333-
isolate->group()->heap()->CollectAllGarbage(GCReason::kDebugging);
333+
isolate->group()->heap()->CollectAllGarbage(GCReason::kDebugging,
334+
/*compact=*/true);
334335
return Object::null();
335336
}
336337

runtime/vm/compiler/aot/precompiler.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,6 +1043,7 @@ void Precompiler::AddCalleesOfHelper(const Object& entry,
10431043
if (!callback_target.IsNull()) {
10441044
AddFunction(callback_target, RetainReasons::kFfiCallbackTarget);
10451045
}
1046+
AddTypesOf(target);
10461047
}
10471048
break;
10481049
}
@@ -1109,6 +1110,10 @@ void Precompiler::AddTypesOf(const Function& function) {
11091110
const Class& owner = Class::Handle(Z, function.Owner());
11101111
AddTypesOf(owner);
11111112

1113+
if (function.IsFfiTrampoline()) {
1114+
AddType(FunctionType::Handle(Z, function.FfiCSignature()));
1115+
}
1116+
11121117
const auto& parent_function = Function::Handle(Z, function.parent_function());
11131118
if (parent_function.IsNull()) {
11141119
return;

runtime/vm/raw_object.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,13 +1404,11 @@ class UntaggedFfiTrampolineData : public UntaggedObject {
14041404
private:
14051405
RAW_HEAP_OBJECT_IMPLEMENTATION(FfiTrampolineData);
14061406

1407-
// Not traced. We don't need this info after precompilation, and FFI
1408-
// trampolines are not supported in JIT snapshots.
1409-
COMPRESSED_POINTER_FIELD(FunctionTypePtr, c_signature)
1410-
14111407
COMPRESSED_POINTER_FIELD(TypePtr, signature_type)
14121408
VISIT_FROM(signature_type)
14131409

1410+
COMPRESSED_POINTER_FIELD(FunctionTypePtr, c_signature)
1411+
14141412
// Target Dart method for callbacks, otherwise null.
14151413
COMPRESSED_POINTER_FIELD(FunctionPtr, callback_target)
14161414

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:ffi';
6+
7+
import 'ffi_test_helpers.dart';
8+
9+
main() {
10+
// Ensure we have FfiTrampolineData in heap.
11+
final foo = DynamicLibrary.process()
12+
.lookup<NativeFunction<Pointer<Void> Function(IntPtr)>>("malloc")
13+
.asFunction<Pointer<Void> Function(int)>();
14+
print(foo);
15+
16+
triggerGc();
17+
18+
print(foo(100).address);
19+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// @dart=2.9
6+
7+
import 'dart:ffi';
8+
9+
import 'ffi_test_helpers.dart';
10+
11+
main() {
12+
// Ensure we have FfiTrampolineData in heap.
13+
final foo = DynamicLibrary.process()
14+
.lookup<NativeFunction<Pointer<Void> Function(IntPtr)>>("malloc")
15+
.asFunction<Pointer<Void> Function(int)>();
16+
print(foo);
17+
18+
triggerGc();
19+
20+
print(foo(100).address);
21+
}

0 commit comments

Comments
 (0)