Skip to content

Commit

Permalink
[vm] Align entry point verification and the precompiler.
Browse files Browse the repository at this point in the history 
For entry-point pragma annotations, most of the time they are
used with either no argument or with an argument that evaluates
to either

* false to denote the annotation should not take effect, or
* null or true to denote the annotation should take effect.

However, the user can also specify that only part of the operations
on a member should be accessed from native code by using a string
argument that is either 'call', 'set', or 'get'.

The entry point verification in Invoke/InvokeGetter/InvokeSetter
assumes that for getters and setters, the only valid string argument
is 'get' or 'set', respectively. This is because those methods are
called via `Dart_GetField`[0] and `Dart_SetField`, respectively, as if
they were the getter or setter of a defined field.

However, the precompiler previously assumed that the string
argument 'call' was the only string argument that meant the link
to a function's code object should be saved. Similarly, it assumed the
string argument 'get' for functions meant that their implicit closure
function should be saved, which ends up including getters. Furthermore,
it did not do anything with setters annotated with the string argument
'set'. This means that the code link would not be saved for getters or
setters that were annotated with the string argument expected by the
entry point verifier.

This CL aligns the precompiler to match the expectations of other
parts of the codebase. It also changes TFA to report an error
if a getter or setter is marked with the string argument 'call'.

[0] `Dart_Invoke` can be called with the name of a getter that
returns a closure, but doing so is semantically equivalent to
calling `Dart_GetField` followed by `Dart_InvokeClosure`.

TEST=vm/dart/entrypoint_verification_test

Fixes: #59920
Change-Id: Ia2768bbaf9058bb14a1cdfb331eb85fa082a0e90
Cq-Include-Trybots: luci.dart.try:vm-aot-dwarf-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-product-x64-try,vm-aot-mac-product-arm64-try,vm-aot-obfuscate-linux-release-x64-try,vm-linux-debug-x64-try,vm-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404823
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
  • Loading branch information
@sstrickl
sstrickl authored and Commit Queue committed Jan 24, 2025
1 parent 862ca5b commit 1ac77f5
Show file tree
Hide file tree
Showing 7 changed files with 378 additions and 138 deletions.
67 changes: 45 additions & 22 deletions pkg/vm/lib/transformations/type_flow/native_code.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,19 @@ class PragmaEntryPointsVisitor extends RecursiveVisitor {
return types ?? const [];
}

static const _referenceToDocumentation =
"See https://github.com/dart-lang/sdk/blob/master/runtime/docs/compiler/"
"aot/entry_point_pragma.md.";

@override
visitLibrary(Library library) {
for (final type in entryPointTypesFromPragmas(library.annotations)) {
if (type == PragmaEntryPointType.Default) {
nativeCodeOracle.addLibraryReferencedFromNativeCode(library);
} else {
throw "Error: pragma entry-point definition on a library must evaluate "
"to null. See entry_points_pragma.md.";
throw "Error: The argument to an entry-point pragma annotation "
"on a library must evaluate to null, true, or false.\n"
"$_referenceToDocumentation";
}
}
library.visitChildren(this);
Expand All @@ -101,8 +106,9 @@ class PragmaEntryPointsVisitor extends RecursiveVisitor {
entryPoints.addDynamicallyExtendableClass(klass);
nativeCodeOracle.addClassReferencedFromNativeCode(klass);
} else {
throw "Error: pragma entry-point definition on a class must evaluate "
"to null, true or false. See entry_points_pragma.md.";
throw "Error: The argument to an entry-point pragma annotation "
"on a class must evaluate to null, true, or false.\n"
"$_referenceToDocumentation";
}
}
klass.visitChildren(this);
Expand All @@ -119,34 +125,49 @@ class PragmaEntryPointsVisitor extends RecursiveVisitor {
: new DirectSelector(proc, callKind: ck));
}

final defaultCallKind = proc.isGetter
? CallKind.PropertyGet
: (proc.isSetter ? CallKind.PropertySet : CallKind.Method);

for (final type in types) {
switch (type) {
case PragmaEntryPointType.CallOnly:
addSelector(defaultCallKind);
if (proc.isGetter) {
throw "Error: The argument to an entry-point pragma annotation on "
"a getter ($proc) must evaluate to null, true, false, or "
"'get'.\n$_referenceToDocumentation";
}
if (proc.isSetter) {
throw "Error: The argument to an entry-point pragma annotation on "
"a setter ($proc) must evaluate to null, true, false, or "
"'set'.\n$_referenceToDocumentation";
}
addSelector(CallKind.Method);
break;
case PragmaEntryPointType.SetterOnly:
if (!proc.isSetter) {
throw "Error: cannot generate a setter for a method or getter ($proc).";
throw "Error: cannot generate a setter for a method or getter "
"($proc).\n$_referenceToDocumentation";
}
addSelector(CallKind.PropertySet);
break;
case PragmaEntryPointType.GetterOnly:
if (proc.isSetter) {
throw "Error: cannot closurize a setter ($proc).";
throw "Error: cannot closurize a setter ($proc).\n"
"$_referenceToDocumentation";
}
if (proc.isFactory) {
throw "Error: cannot closurize a factory ($proc).";
throw "Error: cannot closurize a factory ($proc).\n"
"$_referenceToDocumentation";
}
addSelector(CallKind.PropertyGet);
break;
case PragmaEntryPointType.Default:
addSelector(defaultCallKind);
if (!proc.isSetter && !proc.isGetter && !proc.isFactory) {
if (proc.isGetter) {
addSelector(CallKind.PropertyGet);
} else if (proc.isSetter) {
addSelector(CallKind.PropertySet);
} else {
addSelector(CallKind.Method);
if (!proc.isFactory) {
addSelector(CallKind.PropertyGet);
}
}
break;
case PragmaEntryPointType.Extendable:
Expand All @@ -165,8 +186,9 @@ class PragmaEntryPointsVisitor extends RecursiveVisitor {
for (final type in entryPointTypesFromPragmas(ctor.annotations)) {
if (type != PragmaEntryPointType.Default &&
type != PragmaEntryPointType.CallOnly) {
throw "Error: pragma entry-point definition on a constructor ($ctor) must"
"evaluate to null, true, false or 'call'. See entry_points_pragma.md.";
throw "Error: The argument to an entry-point pragma annotation on a "
"constructor ($ctor) must evaluate to null, true, false or "
"'call'.\n$_referenceToDocumentation";
}
entryPoints
.addRawCall(new DirectSelector(ctor, callKind: CallKind.Method));
Expand All @@ -192,21 +214,22 @@ class PragmaEntryPointsVisitor extends RecursiveVisitor {
addSelector(CallKind.PropertyGet);
break;
case PragmaEntryPointType.SetterOnly:
if (field.isFinal) {
throw "Error: can't use 'set' in entry-point pragma for final field "
"$field";
if (!field.hasSetter) {
throw "Error: can't use 'set' in an entry-point pragma annotation "
"for a field that has no setter ($field).\n"
"$_referenceToDocumentation";
}
addSelector(CallKind.PropertySet);
break;
case PragmaEntryPointType.Default:
addSelector(CallKind.PropertyGet);
if (!field.isFinal) {
if (field.hasSetter) {
addSelector(CallKind.PropertySet);
}
break;
case PragmaEntryPointType.CallOnly:
throw "Error: can't generate invocation dispatcher for field $field"
"through @pragma('vm:entry-point')";
throw "Error: 'call' is not a valid entry-point pragma annotation "
"argument for the field $field.\n$_referenceToDocumentation";
case PragmaEntryPointType.Extendable:
throw "Error: only class can be extendable";
case PragmaEntryPointType.CanBeOverridden:
Expand Down
39 changes: 39 additions & 0 deletions runtime/bin/entrypoints_verification_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,21 @@ bool IsTreeShaken(const char* name, Dart_Handle handle, const char* error) {
Dart_Null())); \
} while (false)

#define TEST_GETTERS(target) \
do { \
FAIL("get1", Dart_GetField(target, Dart_NewStringFromCString("get1"))); \
CHECK(Dart_GetField(target, Dart_NewStringFromCString("get2"))); \
CHECK(Dart_GetField(target, Dart_NewStringFromCString("get3"))); \
} while (false)

#define TEST_SETTERS(target, value) \
do { \
FAIL("set1", \
Dart_SetField(target, Dart_NewStringFromCString("set1"), value)); \
CHECK(Dart_SetField(target, Dart_NewStringFromCString("set2"), value)); \
CHECK(Dart_SetField(target, Dart_NewStringFromCString("set3"), value)); \
} while (false)

DART_EXPORT void RunTests() {
is_dartaotruntime = Dart_IsPrecompiledRuntime();

Expand Down Expand Up @@ -261,4 +276,28 @@ DART_EXPORT void RunTests() {

fprintf(stderr, "\n\nTesting fields with instance target\n\n\n");
TEST_FIELDS(D);

//////// Test actions against getter and setter functions.

fprintf(stderr, "\n\nTesting getters with library target\n\n\n");
TEST_GETTERS(lib);

fprintf(stderr, "\n\nTesting getters with class target\n\n\n");
TEST_GETTERS(F_class);

fprintf(stderr, "\n\nTesting getters with instance target\n\n\n");
TEST_GETTERS(D);

Dart_Handle test_value =
Dart_GetField(lib, Dart_NewStringFromCString("testValue"));
CHECK(test_value);

fprintf(stderr, "\n\nTesting setters with library target\n\n\n");
TEST_SETTERS(lib, test_value);

fprintf(stderr, "\n\nTesting setters with class target\n\n\n");
TEST_SETTERS(F_class, test_value);

fprintf(stderr, "\n\nTesting setters with instance target\n\n\n");
TEST_SETTERS(D, test_value);
}
95 changes: 77 additions & 18 deletions runtime/docs/compiler/aot/entry_point_pragma.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,33 +50,92 @@ Note that `@pragma("vm:entry-point")` may be added to abstract classes -- in
this case, their name will survive obfuscation, but they won't have any
allocation stubs.

### Procedures
### Getters

Any one of the following forms may be attached to a procedure (including
getters, setters and constructors):
Any one of the following forms may be attached to getters:

```dart
@pragma("vm:entry-point")
@pragma("vm:entry-point", true/false)
@pragma("vm:entry-point", !const bool.fromEnvironment("dart.vm.product"))
@pragma("vm:entry-point", "get")
void get foo { ... }
```

The `"get"` annotation allows retrieval of the getter value via
`Dart_GetField`. `Dart_Invoke` can only be used with getters that return a
closure value, in which case it is the same as retrieving the closure via
`Dart_GetField` and then invoking the closure using `Dart_InvokeClosure`, so
the "get" annotation is also needed for such uses.

If the second parameter is missing, `null` or `true`, it behaves the same
as the `"get"` parameter.

Getters cannot be closurized.

### Setters

Any one of the following forms may be attached to setters:

```dart
@pragma("vm:entry-point")
@pragma("vm:entry-point", true/false)
@pragma("vm:entry-point", !const bool.fromEnvironment("dart.vm.product"))
@pragma("vm:entry-point", "set")
void set foo(int value) { ... }
```

The `"set"` annotation allows setting the value via `Dart_SetField`.

If the second parameter is missing, `null` or `true`, it behaves the same
as the `"set"` parameter.

Setters cannot be closurized.

### Constructors

Any one of the following forms may be attached to constructors:

```dart
@pragma("vm:entry-point")
@pragma("vm:entry-point", true/false)
@pragma("vm:entry-point", !const bool.fromEnvironment("dart.vm.product"))
@pragma("vm:entry-point", "call")
void foo() { ... }
C(this.foo) { ... }
```

If the second parameter is missing, `null` or `true`, the procedure (and its
closurized form, excluding constructors and setters) will available for lookup
and invocation directly from native or VM code.
If the annotation is `"call"`, then the procedure is available for invocation
(access via `Dart_Invoke`).

If the second parameter is missing, `null` or `true`, it behaves the same
as the `"call"` parameter.

If the constructor is _generative_, the enclosing class must also be annotated
for allocation from native or VM code.

Constructors cannot be closurized.

### Other Procedures

Any one of the following forms may be attached to other types of procedures:

```dart
@pragma("vm:entry-point")
@pragma("vm:entry-point", true/false)
@pragma("vm:entry-point", !const bool.fromEnvironment("dart.vm.product"))
@pragma("vm:entry-point", "get")
@pragma("vm:entry-point", "call")
void foo(int value) { ... }
```

If the procedure is a *generative* constructor, the enclosing class must also be
annotated for allocation from native or VM code.
If the annotation is `"get"`, then the procedure is only available for
closurization (access via `Dart_GetField`).

If the annotation is "get" or "call", the procedure will only be available for
closurization (access via `Dart_GetField`) or invocation (access via
`Dart_Invoke`).
If the annotation is `"call"`, then the procedure is only available for
invocation (access via `Dart_Invoke`).

"@pragma("vm:entry-point", "get") against constructors or setters is disallowed
since they cannot be closurized.
If the second parameter is missing, `null` or `true`, the procedure is available
for both closurization and invocation.

### Fields

Expand All @@ -95,11 +154,11 @@ int foo;
If the second parameter is missing, `null` or `true`, the field is marked for
native access and for non-static fields the corresponding getter and setter in
the interface of the enclosing class are marked for native invocation. If the
"get" or "set" parameter is used, only the getter or setter is marked. For
`"get"` or `"set"` parameter is used, only the getter or setter is marked. For
static fields, the implicit getter is always marked if the field is marked
for native access.

A field containing a closure may only be invoked using Dart_Invoke if the
A field containing a closure may only be invoked using `Dart_Invoke` if the
getter is marked, in which case it is the same as retrieving the closure from
the field using Dart_GetField and then invoking the closure using
Dart_InvokeClosure.
the field using `Dart_GetField` and then invoking the closure using
`Dart_InvokeClosure`.
Loading

0 comments on commit 1ac77f5

Please sign in to comment.