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

VirtualMemory::DuplicateRX doesn't work on Fuchsia #52579

Open
liamappelbe opened this issue Jun 1, 2023 · 0 comments
Open

VirtualMemory::DuplicateRX doesn't work on Fuchsia #52579

liamappelbe opened this issue Jun 1, 2023 · 0 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. os-fuchsia P3 A lower priority bug or feature request triaged Issue has been triaged by sub team

Comments

@liamappelbe
Copy link
Contributor

Specifically the call to Protect(..., kReadExecute) (which calls zx_vmar_protect) fails. This is why the FfiCallbackMetadata migration had to be reverted.

To fix this, VirtualMemory::DuplicateRX needs to be special cased, like it is for MacOS. In fact the implementation will be similar to the MacOS version. We'll need a call to zx_vmar_map, analogous to the vm_remap call, which remaps our executable memory over an existing writable memory. This requires special permissions that the ordinary VMO used by the rest of virtual_memory_fuchsia.cc doesn't have.

So we need the VMO that is created when the embedder loads the app's .so file in executable mode. At the moment the flutter embedder just discards this VMO, but we need to plumb it though Dart_Initialize to VirtualMemory.

cc @rmacnak-google

@lrhn lrhn added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Jun 1, 2023
copybara-service bot pushed a commit that referenced this issue Jun 6, 2023
This reverts https://dart-review.googlesource.com/c/sdk/+/306674

Patchset 1 is a pure rollback of the rollback.
Patchset 2 is https://dart-review.googlesource.com/c/sdk/+/306316
Patchset 4+ is the forward fix for the Fuchsia issues.

The Fuchsia bug that we're fixing (or working around), is that
VirtualMemory::DuplicateRX doesn't work on Fuchsia. A proper fix will
require special casing it, like on MacOS. In the mean time we can avoid
using this function by only allowing one page of trampolines on Fuchsia.
Unfortunately, when I removed the BSS stuff from the original CL, it
was necessary to duplicate even the first page, so I've had to add that
stuff back just for Fuchsia.

Change-Id: Id42de78ee5de126bcc83bfa4148f6efb4045f976
Bug: #52579
Bug: https://buganizer.corp.google.com/issues/284959841
Fixes: #52581
TEST=CI, especially vm-fuchsia-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306676
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
@a-siva a-siva added P3 A lower priority bug or feature request triaged Issue has been triaged by sub team os-fuchsia labels Nov 30, 2023
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. os-fuchsia P3 A lower priority bug or feature request triaged Issue has been triaged by sub team
Projects
None yet
Development

No branches or pull requests

4 participants