Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for weak entry points in the VM #50649

Open
mkustermann opened this issue Dec 7, 2022 · 2 comments
Open

Support for weak entry points in the VM #50649

mkustermann opened this issue Dec 7, 2022 · 2 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@mkustermann
Copy link
Member

mkustermann commented Dec 7, 2022

Currently we can mark functions (and other objects) as @pragma('vm:entry-point').

Having this annotation implies currently a number of things:

  1. The function serves as a root for our AOT tree shaker (i.e. it's code and what it calls cannot be tree-shaken)
  2. The function may be looked up by name (i.e. cannot be obfuscated)
  3. The function needs to have it's Function object preserved for AOT runtime (i.e. cannot tree-shake function object)
  4. The function's parameter types etc need to be preserved so arguments can be type-checked before invoking the function (i.e. cannot tree-shake function metadata)

There's use cases where we'd only need 2/3/4 but not 1 and as such would allow us to sometimes tree shake chunks of apps away that happen to be unused.

Case for @pragma('vm:weak-entry-point')

Flutter provides an API to map a function to a "unique" integer. When the app later on gets started (e.g. due to an OS event that starts the app) the app can look up based on that unique integer the dart function and call it. More specifically an app can

Now the interesting thing to note here is that if we have e.g.

void register() {
  if (<always-false>) {
    // Dead code, call will be removed by AOT compiler.
    PluginUtilities.getCallbackHandle(myCallback);
  }
}

void foo() {
  final function = PluginUtilities.getCallbackFromhandle(CallbackHandle.fromRawHandle(123));
  // `function` can never be `myCallback` (since we tree-shaken `PluginUtilities.getCallbackHandle(myCallback)`).
}

@pragma('vm:entry-point')
void myCallback() {
  <lots of things we do here>
}

then we (as application developer) know that myCallback will never be invoked.

To support this use case, we could add a @pramga('vm:weak-entry-point') that will have the same implications as a normal @pragma('vm:entry-point') with the difference that we allow our AOT tree shaker to tree shake it, but if it survived we have to guarantee 2/3/4 above.

/cc @alexmarkov @mraleph

@mkustermann mkustermann added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Dec 7, 2022
@alexmarkov
Copy link
Contributor

We may be able to achieve the same even without new pragma.

@pragma('vm:entry-point') takes an optional bool argument which can be used to express condition for including this entry point. So, we can probably use the same <always-false> condition in pragma:

@pragma('vm:entry-point', <always-false>).

For example, sometimes we use @pragma('vm:entry-point', !const bool.fromEnvironment("dart.vm.product")) to include something only in the flutter profile mode and exclude in release.

If this <always-false> condition is used to guard platform-dependent code, then we can introduce a series of defines for each OS and CPU to express such conditions.

@mkustermann
Copy link
Member Author

mkustermann commented Dec 7, 2022

This issue is unrelated to platform-specific tree shaking. I don't believe this can be made with compile-time expression in the annotation. The pattern described in this issue is used by some flutter plugins. So right now as long as the import graph contains a flutter plugin, it's dart code is preserved, even if the app never uses the plugin.

The precise semantics we'd like to have is for any non-tree shaken PluginUtilities.getCallbackHandle(<expr>) call (and other such methods) with <expr> being a top-level function: make us have properties 2-4 above.

An approximating to the above semantics would be a weak entrypoint annotation: if function is used, treat it as entrypoint, if it's not used, tree shake it.

copybara-service bot pushed a commit that referenced this issue Oct 22, 2024
Changes the default value of the --verify-entry-points flag
to true.

Changes the default value for the check_is_entrypoint argument to
to the Invoke/InvokeGetter/InvokeSetter flags to true. The mirrors
library implementation and calls via vm-service explicitly pass
false for this argument now.

Add annotations as needed, such as annotating classes with
annotated generative constructors. In some cases, the annotations
were more general than needed (e.g., annotating with a no-argument
entry point annotation when only the setter is needed), so make
those annotations more specific.

As this pattern is already common in downstream code, allow
Dart_Invoke on fields as long as the field is annotated for getter
access. (That is, calling Dart_Invoke for a field is equivalent to
retrieving the closure value via Dart_GetField and then calling
Dart_InvokeClosure.)

TEST=vm/cc/DartAPI_MissingEntryPoints
     vm/dart/entrypoints_verification_test

Issue: #50649
Issue: flutter/flutter#118608

Change-Id: Ibb3bf15632ab2958d8791b449af8651d47f871a5
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try
CoreLibraryReviewExempt: adding/editing vm-only pragma annotations
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363566
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Tess Strickland <[email protected]>
copybara-service bot pushed a commit that referenced this issue Oct 22, 2024
This reverts commit cb9ecbc.

Reason for revert: causes failures during Dart->Flutter roll and on Flutter HHH bots (see comments on the original CL).

Original change's description:
> [vm] Enforce that entry points must be annotated by default.
>
> Changes the default value of the --verify-entry-points flag
> to true.
>
> Changes the default value for the check_is_entrypoint argument to
> to the Invoke/InvokeGetter/InvokeSetter flags to true. The mirrors
> library implementation and calls via vm-service explicitly pass
> false for this argument now.
>
> Add annotations as needed, such as annotating classes with
> annotated generative constructors. In some cases, the annotations
> were more general than needed (e.g., annotating with a no-argument
> entry point annotation when only the setter is needed), so make
> those annotations more specific.
>
> As this pattern is already common in downstream code, allow
> Dart_Invoke on fields as long as the field is annotated for getter
> access. (That is, calling Dart_Invoke for a field is equivalent to
> retrieving the closure value via Dart_GetField and then calling
> Dart_InvokeClosure.)
>
> TEST=vm/cc/DartAPI_MissingEntryPoints
>      vm/dart/entrypoints_verification_test
>
> Issue: #50649
> Issue: flutter/flutter#118608
>
> Change-Id: Ibb3bf15632ab2958d8791b449af8651d47f871a5
> Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try
> CoreLibraryReviewExempt: adding/editing vm-only pragma annotations
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363566
> Reviewed-by: Martin Kustermann <[email protected]>
> Commit-Queue: Tess Strickland <[email protected]>

Issue: #50649
Issue: flutter/flutter#118608
Change-Id: Idba168f77b0636a50ad93309e29dc9989cc1f388
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391460
Auto-Submit: Alexander Markov <[email protected]>
Reviewed-by: Tess Strickland <[email protected]>
Commit-Queue: Tess Strickland <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
copybara-service bot pushed a commit that referenced this issue Dec 4, 2024
This is a reland of commit cb9ecbc

This reland only turns on the entry point verification flag by
default in AOT mode. After Flutter tests that use native access
in JIT mode have been appropriately updated, a followup CL will
turn this flag on by default in JIT mode as well.

Original change's description:
> [vm] Enforce that entry points must be annotated by default.
>
> Changes the default value of the --verify-entry-points flag
> to true.
>
> Changes the default value for the check_is_entrypoint argument to
> to the Invoke/InvokeGetter/InvokeSetter flags to true. The mirrors
> library implementation and calls via vm-service explicitly pass
> false for this argument now.
>
> Add annotations as needed, such as annotating classes with
> annotated generative constructors. In some cases, the annotations
> were more general than needed (e.g., annotating with a no-argument
> entry point annotation when only the setter is needed), so make
> those annotations more specific.
>
> As this pattern is already common in downstream code, allow
> Dart_Invoke on fields as long as the field is annotated for getter
> access. (That is, calling Dart_Invoke for a field is equivalent to
> retrieving the closure value via Dart_GetField and then calling
> Dart_InvokeClosure.)
>
> TEST=vm/cc/DartAPI_MissingEntryPoints
>      vm/dart/entrypoints_verification_test
>
> Issue: #50649
> Issue: flutter/flutter#118608
>
> Change-Id: Ibb3bf15632ab2958d8791b449af8651d47f871a5
> Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try
> CoreLibraryReviewExempt: adding/editing vm-only pragma annotations
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363566
> Reviewed-by: Martin Kustermann <[email protected]>
> Commit-Queue: Tess Strickland <[email protected]>

TEST=vm/cc/DartAPI_MissingEntryPoints
     vm/dart/entrypoints_verification_test

Change-Id: I24919c32ab4760c7c5435c378879791086256f02
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try,flutter-linux-try,vm-linux-debug-x64-try,vm-linux-release-x64-try,vm-appjit-linux-product-x64-try
CoreLibraryReviewExempt: adding/editing vm-only pragma annotations
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391620
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Tess Strickland <[email protected]>
github-merge-queue bot pushed a commit to flutter/flutter that referenced this issue Dec 12, 2024
This change adds entry-point annotations to methods and classes accessed
by native code during Flutter tests. Currently, entry point annotations
are not checked by the Dart VM when running in JIT mode, only in AOT
mode. In order to also enforce entry point annotations in JIT mode,
first tests in Flutter must be appropriately annotated to avoid roll
failures.

Related issues:
* #118608
* dart-lang/sdk#50649

## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [X] I signed the [CLA].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [X] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [X] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
sstrickl added a commit to flutter/engine that referenced this issue Dec 13, 2024
This change adds entry-point annotations to methods and classes accessed
by native code during engine tests. Currently, entry point annotations
are not checked by the Dart VM when running in JIT mode, only in AOT
mode. In order to also enforce entry point annotations in JIT mode,
first tests in Flutter must be appropriately annotated to avoid roll
failures.

Related issues:
* flutter/flutter#118608
* dart-lang/sdk#50649
github-merge-queue bot pushed a commit to flutter/flutter that referenced this issue Dec 17, 2024
This change adds entry-point annotations to methods and classes accessed
by native code during Flutter tests. Currently, entry point annotations
are not checked by the Dart VM when running in JIT mode, only in AOT
mode. In order to also enforce entry point annotations in JIT mode,
first tests in Flutter must be appropriately annotated to avoid roll
failures.

Related issues:
* #118608
* dart-lang/sdk#50649

## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [X] I signed the [CLA].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [X] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [X] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
copybara-service bot pushed a commit that referenced this issue Dec 18, 2024
Now that Flutter tests that access entry points from native code
have been annotated[1], we can turn on entry point checking in JIT
mode.

This CL also removes the A flag category from flag_list.h and the
AOT_FLAG_MACRO definitions and uses from flags.[cc,h], as they were
created as a temporary measure until this flag could be unconditionally
defaulted to true.

[1] See the following PRs:
* flutter/engine#57158
* flutter/flutter#160158
* flutter/flutter#160421

TEST=vm/dart/entrypoints_verification_test vm/cc/IRTest
     vm/cc/StreamingFlowGraphBuilder vm/cc/STC vm/cc/TTS

Issue: #50649
Issue: flutter/flutter#118608

Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try,vm-linux-debug-x64-try,vm-linux-release-x64-try,vm-appjit-linux-product-x64-try
Change-Id: Ibe5b21bb74f1a6fb88824b71ff87b9e555216dbf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400301
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Tess Strickland <[email protected]>
nick9822 pushed a commit to nick9822/flutter that referenced this issue Dec 18, 2024
This change adds entry-point annotations to methods and classes accessed
by native code during engine tests. Currently, entry point annotations
are not checked by the Dart VM when running in JIT mode, only in AOT
mode. In order to also enforce entry point annotations in JIT mode,
first tests in Flutter must be appropriately annotated to avoid roll
failures.

Related issues:
* flutter#118608
* dart-lang/sdk#50649
copybara-service bot pushed a commit that referenced this issue Dec 19, 2024
This reverts commit 982b9fa.

Reason for revert: b/385114574

Original change's description:
> [vm] Turn on entry point checking in JIT mode.
>
> Now that Flutter tests that access entry points from native code
> have been annotated[1], we can turn on entry point checking in JIT
> mode.
>
> This CL also removes the A flag category from flag_list.h and the
> AOT_FLAG_MACRO definitions and uses from flags.[cc,h], as they were
> created as a temporary measure until this flag could be unconditionally
> defaulted to true.
>
> [1] See the following PRs:
> * flutter/engine#57158
> * flutter/flutter#160158
> * flutter/flutter#160421
>
> TEST=vm/dart/entrypoints_verification_test vm/cc/IRTest
>      vm/cc/StreamingFlowGraphBuilder vm/cc/STC vm/cc/TTS
>
> Issue: #50649
> Issue: flutter/flutter#118608
>
> Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try,vm-linux-debug-x64-try,vm-linux-release-x64-try,vm-appjit-linux-product-x64-try
> Change-Id: Ibe5b21bb74f1a6fb88824b71ff87b9e555216dbf
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400301
> Reviewed-by: Martin Kustermann <[email protected]>
> Commit-Queue: Tess Strickland <[email protected]>

Issue: #50649
Issue: flutter/flutter#118608
Change-Id: Id403cd0832807e417202e17dac57c2224cab09e7
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try,vm-linux-debug-x64-try,vm-linux-release-x64-try,vm-appjit-linux-product-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401880
Bot-Commit: Rubber Stamper <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Ivan Inozemtsev <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jan 8, 2025
This is a reland of commit 982b9fa

Original change's description:
> [vm] Turn on entry point checking in JIT mode.
>
> Now that Flutter tests that access entry points from native code
> have been annotated[1], we can turn on entry point checking in JIT
> mode.
>
> This CL also removes the A flag category from flag_list.h and the
> AOT_FLAG_MACRO definitions and uses from flags.[cc,h], as they were
> created as a temporary measure until this flag could be unconditionally
> defaulted to true.
>
> [1] See the following PRs:
> * flutter/engine#57158
> * flutter/flutter#160158
> * flutter/flutter#160421
>
> TEST=vm/dart/entrypoints_verification_test vm/cc/IRTest
>      vm/cc/StreamingFlowGraphBuilder vm/cc/STC vm/cc/TTS
>
> Issue: #50649
> Issue: flutter/flutter#118608
>
> Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try,vm-linux-debug-x64-try,vm-linux-release-x64-try,vm-appjit-linux-product-x64-try
> Change-Id: Ibe5b21bb74f1a6fb88824b71ff87b9e555216dbf
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400301
> Reviewed-by: Martin Kustermann <[email protected]>
> Commit-Queue: Tess Strickland <[email protected]>

TEST=vm/dart/entrypoints_verification_test vm/cc/IRTest
     vm/cc/StreamingFlowGraphBuilder vm/cc/STC vm/cc/TTS

Change-Id: Ibd5f362f908b4aaa68cda870a387c081537bbc16
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try,vm-linux-debug-x64-try,vm-linux-release-x64-try,vm-appjit-linux-product-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403360
Auto-Submit: Ivan Inozemtsev <[email protected]>
Commit-Queue: Ivan Inozemtsev <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

2 participants