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.

Sign up for GitHub

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

Jump to bottom

[vm/ffi] Native asset resolution via embedder #55523

Closed
dcharkes opened this issue Apr 19, 2024 · 1 comment
Closed

[vm/ffi] Native asset resolution via embedder #55523

dcharkes opened this issue Apr 19, 2024 · 1 comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi

Comments

@dcharkes
Copy link
Contributor

We should call out to the embedder to resolve native assets and symbols.

This would enable the embedder resolving symlinks:

Also it would enable the embedder not having the files on disk at all or having the as part of a larger file and using dlopen_with_offset.

The embedder would provide an API for dlopen(), dlclose(), dlsym(), dladdr().

The embedder API would be tied to the new native assets feature. So dlopen would take a native assets mapping such as ['relative', 'some/file/path.so'] or ['process']. (As such it would not be used for DynamicLibrary.open and DynamicLibrary.lookup. #37771)

Having a dlclose embedder callback is only useful when we unload dynamic libraries on isolate group shutdown:

Having a dladdr embedder callback is only if we add some functionality that uses this. We currently don't have such functionality.
@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi labels Apr 19, 2024
@dcharkes
Copy link
Contributor Author

dcharkes commented May 9, 2024

I've created a prototype here: https://dart-review.googlesource.com/c/sdk/+/361881

cc @a-siva @rmacnak-google @aam @alexmarkov @mraleph in case anyone has concerns on adding these embedder APIs. Feedback welcome!

@dcharkes dcharkes mentioned this issue Jun 11, 2024
Merged
8 tasks
copybara-service bot pushed a commit that referenced this issue Jun 12, 2024
@dcharkes
[vm] Move ResolveUri to platform
4b2906c 
This will enable the standalone embedder to resolve uris in a follow
up CL:
https://dart-review.googlesource.com/c/sdk/+/361881

The implementation was using zones for allocation, this CL switches
that to using `malloc`. The caller is responsible for freeing the
memory if resolving succeeds.

This CL removes `Dart_DefaultCanonicalizeUrl`.

We don't have a run_platform_tests, so this CL keeps the tests in
run_vm_tests.

TEST=vm/cc/ParseUri
TEST=vm/cc/ResolveUri
TEST=tests/ffi/native_assets/asset_relative_test.dart

Bug: #55523
Change-Id: Ifb300d8164eb50506f22ce619fad0811f74ef34c
Cq-Include-Trybots: luci.dart.try:vm-aot-asan-linux-release-x64-try,vm-aot-msan-linux-release-x64-try,vm-asan-linux-release-arm64-try,vm-asan-linux-release-x64-try,vm-msan-linux-release-arm64-try,vm-msan-linux-release-x64-try,pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-linux-release-try,pkg-mac-release-try,pkg-mac-release-arm64-try,pkg-win-release-arm64-try,pkg-win-release-try,vm-aot-linux-debug-x64-try,vm-linux-debug-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-aot-win-debug-x64-try,vm-win-debug-arm64-try,vm-mac-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/368423
Reviewed-by: Martin Kustermann <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jun 12, 2024
@dcharkes
[vm] Native asset relative path resolution with symlinks
9588927 
This CL moves native assets resolution to the embedder.

The Dart VM looks up the asset path (for example
`['relative', 'foo.so']`) with the asset id. The embedder defines
callbacks for asset loading, and returns a handle to the dylib.
Then the VM calls the embedder again with `dlsym` to lookup the symbol.

The Dart VM & kernel compiler are responsible for the asset id to
asset path mapping. The kernel compiler compiles it into the snapshot
and the VM looks it up in the snapshot.

Relative paths are resolved relative to the isolate script uri (kernel
snapshot, jit snapshot, aot snapshot, or `dart compile exe` result).
The embedder is responsible for remembering the script uri it set when
spawning the isolate group.

This CL does not add `dlclose` or `dladdr` embedder callbacks yet.
Bug: #55521
Bug: #55966

TEST=pkg/dartdev/test/native_assets/build_test.dart
TEST=tests/ffi/native_assets/asset_relative_test.dart

Bug: #55410
Bug: #55523
Bug: #55207
Change-Id: I75ec4a368c5fb3d2f76b03771e796ff56bcac941
Cq-Include-Trybots: dart/try:vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,vm-aot-win-debug-x64c-try,pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-mac-release-try,pkg-mac-release-arm64-try,pkg-win-release-try,pkg-win-release-arm64-try,vm-aot-asan-linux-release-x64-try,vm-asan-linux-release-x64-try,vm-aot-msan-linux-release-x64-try,vm-msan-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361881
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Daco Harkes <[email protected]>
auto-submit bot pushed a commit to flutter/engine that referenced this issue Jun 18, 2024
@dcharkes
Setup NativeAssetsApi during isolate group creation (#53329)
2d5c28e 
This initializes the `NativeAssetsApi` for native assets resolution in the isolate group creation callback.

* dart-lang/sdk#55523

## Implementation considerations

The DartIO initialization lives in its own GN target. This doesn't work for the native assets initialization due to it having to look up the `script_uri` in one of the callbacks. Since the callbacks are function pointers, we can't have a lambda that captures the script uri. So instead, the native assets initialization lives in the flutter/runtime target.

The import from dart should probably be `runtime/include/bin/native_assets_api.h` to mirror what we're doing with dart IO, rather than directly importing from `runtime/bin/native_assets.h`.

## Testing

All native asset testing is in flutter_tools, so those tests will only run once this rolls into flutter/flutter. I have done manual testing locally with a Dart branch that removes the fallback: https://dart-review.googlesource.com/c/sdk/+/370740

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
copybara-service bot pushed a commit that referenced this issue Jun 20, 2024
@dcharkes
Revert "[vm] Move ResolveUri to bin"
a616e73 
This reverts commit 1a614b1.

Reason for revert: Breaks embedders in g3.

Original change's description:
> [vm] Move `ResolveUri` to bin
>
> This CL removes the native assets fallback from the VM.
>
> TEST=pkg/dartdev/test/native_assets/build_test.dart
> TEST=tests/ffi/native_assets/asset_relative_test.dart
>
> Closes: #55523
> Closes: #55925
> Change-Id: Ifc7e491300ae1b65bb5a9e5279cbcb45ad91e165
> Cq-Include-Trybots: dart/try:vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,vm-aot-win-debug-x64c-try,pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-mac-release-try,pkg-mac-release-arm64-try,pkg-win-release-try,pkg-win-release-arm64-try,vm-aot-asan-linux-release-x64-try,vm-asan-linux-release-x64-try,vm-aot-msan-linux-release-x64-try,vm-msan-linux-release-x64-try
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370740
> Reviewed-by: Martin Kustermann <[email protected]>
> Commit-Queue: Daco Harkes <[email protected]>

Change-Id: Ib11db566b857089126cbfdf5af2d01648c348b84
Cq-Include-Trybots: dart/try:vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,vm-aot-win-debug-x64c-try,pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-mac-release-try,pkg-mac-release-arm64-try,pkg-win-release-try,pkg-win-release-arm64-try,vm-aot-asan-linux-release-x64-try,vm-asan-linux-release-x64-try,vm-aot-msan-linux-release-x64-try,vm-msan-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/372441
Commit-Queue: Daco Harkes <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jun 26, 2024
@dcharkes
Reland "[vm] Move ResolveUri to bin"
8f962bd 
This is a reland of commit 1a614b1
Bug: b/348315159

This CL removes the native assets fallback from the VM.

TEST=pkg/dartdev/test/native_assets/build_test.dart
TEST=tests/ffi/native_assets/asset_relative_test.dart

Closes: #55523
Closes: #55925

Change-Id: Ib40d3a8c7d4b7c6226b6e4db4b3016cc32591239
Cq-Include-Trybots: dart/try:vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,vm-aot-win-debug-x64c-try,pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-mac-release-try,pkg-mac-release-arm64-try,pkg-win-release-try,pkg-win-release-arm64-try,vm-aot-asan-linux-release-x64-try,vm-asan-linux-release-x64-try,vm-aot-msan-linux-release-x64-try,vm-msan-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/372500
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Daco Harkes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dcharkes