Skip to content

Commit b759d09

Browse files
aamCommit Queue
authored andcommitted
[vm/shared] Enforce pragma('vm:shared') annotations for captured local vars.
Fixes #61287 TEST=ffi/isolate_group_bound_captured_local_test Change-Id: I9dc1e8aaf9e99d8ec9ad730cf1cd89ae3b4ed148 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444923 Commit-Queue: Alexander Aprelev <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent 94894b1 commit b759d09

File tree

10 files changed

+171
-5
lines changed

10 files changed

+171
-5
lines changed

runtime/vm/ffi_callback_metadata.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,16 @@ void FfiCallbackMetadata::EnsureOnlyTriviallyImmutableValuesInClosure(
398398
for (intptr_t i = 0; i < context.num_variables(); i++) {
399399
ValidateTriviallyImmutabilityOfAnObject(zone, &obj, context.At(i));
400400
}
401+
402+
if (!function.does_close_over_only_final_and_shared_vars()) {
403+
const String& error = String::Handle(
404+
zone,
405+
String::New(
406+
"Only final and 'vm:shared' variables can be captured by isolate "
407+
"group callbacks."));
408+
Exceptions::ThrowArgumentError(error);
409+
UNREACHABLE();
410+
}
401411
}
402412
}
403413

runtime/vm/object.cc

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8586,6 +8586,27 @@ void Function::set_awaiter_link(Function::AwaiterLink link) const {
85868586
UNREACHABLE();
85878587
}
85888588

8589+
bool Function::does_close_over_only_final_and_shared_vars() const {
8590+
if (IsClosureFunction()) {
8591+
const Object& obj = Object::Handle(untag()->data());
8592+
ASSERT(!obj.IsNull());
8593+
return ClosureData::Cast(obj).does_close_over_only_final_and_shared_vars();
8594+
}
8595+
UNREACHABLE();
8596+
}
8597+
8598+
void Function::set_does_close_over_only_final_and_shared_vars(
8599+
bool value) const {
8600+
if (IsClosureFunction()) {
8601+
const Object& obj = Object::Handle(untag()->data());
8602+
ASSERT(!obj.IsNull());
8603+
ClosureData::Cast(obj).set_does_close_over_only_final_and_shared_vars(
8604+
value);
8605+
return;
8606+
}
8607+
UNREACHABLE();
8608+
}
8609+
85898610
ClosurePtr Function::implicit_static_closure() const {
85908611
if (IsImplicitStaticClosureFunction()) {
85918612
const Object& obj = Object::Handle(untag()->data());
@@ -12128,6 +12149,19 @@ void ClosureData::set_awaiter_link(Function::AwaiterLink link) const {
1212812149
link.index);
1212912150
}
1213012151

12152+
bool ClosureData::does_close_over_only_final_and_shared_vars() const {
12153+
return untag()
12154+
->packed_fields_
12155+
.Read<UntaggedClosureData::DoesCloseOverOnlySharedFields>();
12156+
}
12157+
12158+
void ClosureData::set_does_close_over_only_final_and_shared_vars(
12159+
bool value) const {
12160+
untag()
12161+
->packed_fields_
12162+
.Update<UntaggedClosureData::DoesCloseOverOnlySharedFields>(value);
12163+
}
12164+
1213112165
ClosureDataPtr ClosureData::New() {
1213212166
ASSERT(Object::closure_data_class() != Class::null());
1213312167
return Object::Allocate<ClosureData>(Heap::kOld);

runtime/vm/object.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3302,6 +3302,9 @@ class Function : public Object {
33023302
(awaiter_link().depth != UntaggedClosureData::kNoAwaiterLinkDepth);
33033303
}
33043304

3305+
void set_does_close_over_only_final_and_shared_vars(bool value) const;
3306+
bool does_close_over_only_final_and_shared_vars() const;
3307+
33053308
// Enclosing function of this local function.
33063309
FunctionPtr parent_function() const;
33073310

@@ -4393,6 +4396,9 @@ class ClosureData : public Object {
43934396
Function::AwaiterLink awaiter_link() const;
43944397
void set_awaiter_link(Function::AwaiterLink link) const;
43954398

4399+
bool does_close_over_only_final_and_shared_vars() const;
4400+
void set_does_close_over_only_final_and_shared_vars(bool value) const;
4401+
43964402
// Enclosing function of this local function.
43974403
PRECOMPILER_WSR_FIELD_DECLARATION(Function, parent_function)
43984404

runtime/vm/raw_object.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,7 +1521,10 @@ class UntaggedClosureData : public UntaggedObject {
15211521
using PackedAwaiterLinkIndex = BitField<decltype(packed_fields_),
15221522
uint8_t,
15231523
PackedAwaiterLinkDepth::kNextBit>;
1524-
1524+
using DoesCloseOverOnlySharedFields =
1525+
BitField<decltype(packed_fields_),
1526+
bool,
1527+
PackedAwaiterLinkIndex::kNextBit>;
15251528
friend class Function;
15261529
friend class UnitDeserializationRoots;
15271530
};
@@ -2470,7 +2473,8 @@ class UntaggedContext : public UntaggedObject {
24702473
V(Late) \
24712474
V(Nullable) \
24722475
V(Invisible) \
2473-
V(AwaiterLink)
2476+
V(AwaiterLink) \
2477+
V(Shared)
24742478

24752479
class UntaggedContextScope : public UntaggedObject {
24762480
RAW_HEAP_OBJECT_IMPLEMENTATION(ContextScope);

runtime/vm/scopes.cc

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,8 @@ ContextScopePtr LocalScope::PreserveOuterScope(
462462

463463
LocalVariable* awaiter_link = nullptr;
464464

465+
bool does_capture_only_final_and_shared_vars = true;
466+
465467
// Create a descriptor for each referenced captured variable of enclosing
466468
// functions to preserve its name and its context allocation information.
467469
int captured_idx = 0;
@@ -500,6 +502,13 @@ ContextScopePtr LocalScope::PreserveOuterScope(
500502
if (is_awaiter_link) {
501503
awaiter_link = variable;
502504
}
505+
506+
bool is_shared = variable->ComputeIfShared(library);
507+
context_scope.SetIsSharedAt(captured_idx, is_shared);
508+
if (!is_shared && !variable->is_final()) {
509+
does_capture_only_final_and_shared_vars = false;
510+
}
511+
503512
captured_idx++;
504513
}
505514
}
@@ -523,6 +532,11 @@ ContextScopePtr LocalScope::PreserveOuterScope(
523532
}
524533
}
525534

535+
if (!function.IsNull()) {
536+
function.set_does_close_over_only_final_and_shared_vars(
537+
does_capture_only_final_and_shared_vars);
538+
}
539+
526540
return context_scope.ptr();
527541
}
528542

@@ -542,6 +556,7 @@ LocalScope* LocalScope::RestoreOuterScope(const ContextScope& context_scope) {
542556
String::ZoneHandle(context_scope.NameAt(i)), static_type,
543557
context_scope.KernelOffsetAt(i), inferred_type);
544558
variable->set_is_awaiter_link(context_scope.IsAwaiterLinkAt(i));
559+
variable->set_is_shared(context_scope.IsSharedAt(i));
545560
variable->set_is_captured();
546561
variable->set_index(VariableIndex(context_scope.ContextIndexAt(i)));
547562
if (context_scope.IsFinalAt(i)) {
@@ -693,6 +708,19 @@ bool LocalVariable::ComputeIfIsAwaiterLink(const Library& library) {
693708
return is_awaiter_link_ == IsAwaiterLink::kLink;
694709
}
695710

711+
bool LocalVariable::ComputeIfShared(const Library& library) {
712+
if (is_shared_ == IsShared::kUnknown) {
713+
RELEASE_ASSERT(annotations_offset_ != kNoKernelOffset);
714+
Thread* T = Thread::Current();
715+
Zone* Z = T->zone();
716+
const auto& metadata = Object::Handle(
717+
Z, kernel::EvaluateMetadata(library, annotations_offset_,
718+
/* is_annotations_offset = */ true));
719+
set_is_shared(FindPragmaInMetadata(T, metadata, Symbols::vm_shared()));
720+
}
721+
return is_shared_ == IsShared::kShared;
722+
}
723+
696724
bool LocalVariable::Equals(const LocalVariable& other) const {
697725
if (HasIndex() && other.HasIndex() && (index() == other.index())) {
698726
if (is_captured() == other.is_captured()) {

runtime/vm/scopes.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ class LocalVariable : public ZoneAllocated {
104104
late_init_offset_(0),
105105
type_check_mode_(kDoTypeCheck),
106106
index_(),
107-
is_awaiter_link_(IsAwaiterLink::kNotLink) {
107+
is_awaiter_link_(IsAwaiterLink::kNotLink),
108+
is_shared_(IsShared::kNotShared) {
108109
DEBUG_ASSERT(static_type.IsNotTemporaryScopedHandle());
109110
ASSERT(static_type.IsFinalized());
110111
ASSERT(inferred_type != nullptr);
@@ -129,6 +130,8 @@ class LocalVariable : public ZoneAllocated {
129130
annotations_offset_ = offset;
130131
is_awaiter_link_ = (offset == kNoKernelOffset) ? IsAwaiterLink::kNotLink
131132
: IsAwaiterLink::kUnknown;
133+
is_shared_ =
134+
(offset == kNoKernelOffset) ? IsShared::kNotShared : IsShared::kUnknown;
132135
}
133136

134137
const AbstractType& static_type() const { return static_type_; }
@@ -151,6 +154,11 @@ class LocalVariable : public ZoneAllocated {
151154
bool is_late() const { return IsLateBit::decode(bitfield_); }
152155
void set_is_late() { bitfield_ = IsLateBit::update(true, bitfield_); }
153156

157+
bool ComputeIfShared(const Library& library);
158+
void set_is_shared(bool value) {
159+
is_shared_ = value ? IsShared::kShared : IsShared::kNotShared;
160+
}
161+
154162
intptr_t late_init_offset() const { return late_init_offset_; }
155163
void set_late_init_offset(intptr_t late_init_offset) {
156164
late_init_offset_ = late_init_offset;
@@ -264,6 +272,13 @@ class LocalVariable : public ZoneAllocated {
264272
};
265273
IsAwaiterLink is_awaiter_link_;
266274

275+
enum class IsShared {
276+
kUnknown,
277+
kNotShared,
278+
kShared,
279+
};
280+
IsShared is_shared_;
281+
267282
friend class LocalScope;
268283
DISALLOW_COPY_AND_ASSIGN(LocalVariable);
269284
};

runtime/vm/symbols.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,7 @@ class ObjectPointerVisitor;
577577
V(vm_notify_debugger_on_exception, "vm:notify-debugger-on-exception") \
578578
V(vm_prefer_inline, "vm:prefer-inline") \
579579
V(vm_recognized, "vm:recognized") \
580+
V(vm_shared, "vm:shared") \
580581
V(vm_testing_print_flow_graph, "vm:testing:print-flow-graph") \
581582
V(vm_trace_entrypoints, "vm:testing.unsafe.trace-entrypoints-fn") \
582583
V(vm_unsafe_no_interrupts, "vm:unsafe:no-interrupts") \
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Copyright (c) 2025, 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+
// Test that exception is thrown when captured variables doesn't have
6+
// pragma('vm:shared') annotation.
7+
//
8+
// VMOptions=--experimental-shared-data
9+
10+
import 'dart:async';
11+
import 'dart:ffi';
12+
import 'dart:isolate';
13+
14+
import 'package:dart_internal/isolate_group.dart' show IsolateGroup;
15+
16+
import "package:expect/async_helper.dart";
17+
import "package:expect/expect.dart";
18+
19+
typedef CallbackNativeType = Void Function(Int64, Int32);
20+
21+
Future<void> testCapturedLocalVarNoDecoration() async {
22+
int foo_result = 42;
23+
Expect.throws(() {
24+
final callback = NativeCallable<CallbackNativeType>.isolateGroupBound((
25+
int a,
26+
int b,
27+
) {
28+
foo_result += (a * b);
29+
});
30+
}, (e) => e.toString().contains("variables can be captured"));
31+
}
32+
33+
Future<void> testCapturedLocalVarPragmaVmShared() async {
34+
@pragma('vm:shared')
35+
int foo_result = 42;
36+
final callback = NativeCallable<CallbackNativeType>.isolateGroupBound((
37+
int a,
38+
int b,
39+
) {
40+
foo_result += (a * b);
41+
});
42+
callback.close();
43+
}
44+
45+
Future<void> testCapturedLocalVarFinal() async {
46+
final int foo_result = 42;
47+
final callback = NativeCallable<CallbackNativeType>.isolateGroupBound((
48+
int a,
49+
int b,
50+
) {
51+
foo_result + (a * b);
52+
});
53+
callback.close();
54+
}
55+
56+
main(args, message) async {
57+
asyncStart();
58+
await testCapturedLocalVarNoDecoration();
59+
await testCapturedLocalVarPragmaVmShared();
60+
await testCapturedLocalVarFinal();
61+
asyncEnd();
62+
print("All tests completed :)");
63+
}

tests/ffi/isolate_group_bound_init_test.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,10 @@ testInitStrings() async {
6262
for (int i = 0; i < nWorkers; i++) {
6363
Isolate.spawn(
6464
(sendPort) {
65+
@pragma('vm:shared')
66+
final sp = sendPort;
6567
IsolateGroup.runSync(() {
66-
sendPort.send(shared_late_final_string);
68+
sp.send(shared_late_final_string);
6769
});
6870
},
6971
rp.sendPort,
@@ -100,9 +102,11 @@ testInitThrows() async {
100102
for (int i = 0; i < nWorkers; i++) {
101103
Isolate.spawn(
102104
(sendPort) {
105+
@pragma('vm:shared')
106+
final sp = sendPort;
103107
IsolateGroup.runSync(() {
104108
try {
105-
sendPort.send(shared_late_final_throw);
109+
sp.send(shared_late_final_throw);
106110
} catch (e) {
107111
Expect.equals("165", e);
108112
rethrow;

tests/ffi/run_isolate_group_run_test.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ String string_foo = "";
5454
SendPort? sp;
5555

5656
StringMethodTearoffTest() {
57+
@pragma('vm:shared')
5758
final stringTearoff = "abc".toString;
5859
IsolateGroup.runSync(() {
5960
stringTearoff;

0 commit comments

Comments
 (0)