Stream: git-wasmtime

Topic: wasmtime / PR #10865 Extend CustomCodeMemory interface al...

Wasmtime GitHub notifications bot (May 29 2025 at 19:34):

Milek7 requested dicej for a review on PR #10865.

Wasmtime GitHub notifications bot (May 29 2025 at 19:34):

Milek7 opened PR #10865 from Milek7:customcodememory-dso to bytecodealliance:main:

Module::deserialize_raw combined with CustomCodeMemory were introduced with aim of supporting no_std platforms. With small tweaks they can also be used for using Wasmtime on platforms that have full-blown virtual memory capabilities, but doesn't allow for directly mapping executable pages from user code instead limiting that capability for system loader. Adding features necessary for such platforms was previously attempted by #8245.

There are currently two issues with using Module::deserialize_raw for using images either statically linked into embedder executable or dynamically loaded shared objects:

• CodeMemory::publish will initially make entire image read-only, destroying executable permissions that cannot be restored by user code. This will happen even if CustomCodeMemory is provided.
• CodeMemory::publish will attempt to unconditionally register unwind information. As these are already properly registered by the system loader this is superfluous at best, or could fail module loading if system decides to return error on attempt for double-registration.

This commit solves these issues by:

• Moving responsibility for making image RO to CustomCodeMemory hook.
• Making CustomCodeMemory publishing hook return enum that tells what steps (only mapping, or mapping with registartion), using default implementation only for actions not reported by the hook.
• Additionally CustomCodeMemory hooks also receive pointers to the entire image, not only executable section. This allows the embedder to keep track of the images and unload them in case they were loaded as dynamic shared object.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (May 29 2025 at 19:34):

Milek7 requested wasmtime-core-reviewers for a review on PR #10865.

Wasmtime GitHub notifications bot (May 29 2025 at 19:34):

Milek7 edited PR #10865.

Wasmtime GitHub notifications bot (May 29 2025 at 19:39):

Milek7 edited PR #10865:

Module::deserialize_raw and CustomCodeMemory was introduced with aim of supporting no_std platforms. With small tweaks they can also be used for using Wasmtime on platforms that have full-blown virtual memory capabilities, but doesn't allow for directly mapping executable pages from user code instead limiting that capability for system loader. Adding features necessary for such platforms was previously attempted by #8245.

There are currently two issues with using Module::deserialize_raw for using images either statically linked into embedder executable or dynamically loaded shared objects:

• CodeMemory::publish will initially make entire image read-only, destroying executable permissions that cannot be restored by user code. This will happen even if CustomCodeMemory is provided.
• CodeMemory::publish will attempt to unconditionally register unwind information. As these are already properly registered by the system loader this is superfluous at best, or could fail module loading if system decides to return error on attempt for double-registration.

This commit solves these issues by:

• Moving responsibility for making image RO to CustomCodeMemory hook.
• Making CustomCodeMemory publishing hook return enum that tells what steps (only mapping, or mapping with registration) were performed, using default implementation only for actions not reported by the hook.
• Additionally CustomCodeMemory hooks also receive pointers to the entire image, not only executable section. This allows the embedder to keep track of the images and unload them in case they were loaded as dynamic shared object.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (May 29 2025 at 19:43):

Milek7 edited PR #10865:

Module::deserialize_raw and CustomCodeMemory were introduced with aim of supporting no_std platforms. With small tweaks they can also be used for using Wasmtime on platforms that have full-blown virtual memory capabilities, but doesn't allow for directly mapping executable pages from user code instead limiting that capability for system loader. Adding features necessary for such platforms was previously attempted by #8245.

There are currently two issues with using Module::deserialize_raw for using images either statically linked into embedder executable or dynamically loaded shared objects:

• CodeMemory::publish will initially make entire image read-only, destroying executable permissions that cannot be restored by user code. This will happen even if CustomCodeMemory is provided.
• CodeMemory::publish will attempt to unconditionally register unwind information. As these are already properly registered by the system loader this is superfluous at best, or could fail module loading if system decides to return error on attempt for double-registration.

This commit solves these issues by:

• Moving responsibility for making image RO to CustomCodeMemory hook.
• Making CustomCodeMemory publishing hook return enum that tells what steps (only mapping, or mapping with registration) were performed, using default implementation only for actions not reported by the hook.
• Additionally CustomCodeMemory hooks also receive pointers to the entire image, not only executable section. This allows the embedder to keep track of the images and unload them in case they were loaded as dynamic shared object.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (May 29 2025 at 19:47):

Milek7 updated PR #10865.

Wasmtime GitHub notifications bot (May 29 2025 at 20:44):

Milek7 commented on PR #10865:

Okay, I got a little ahead of myself.

make_readonly won't be actually get called for modules created by Module::deserialize_raw. So first issue is not true.
However double-registration is still a concern.
Providing address of entire image to hooks is still useful for keeping track for unloading purposes (and maybe for no virtual memory target if they actually want to make that RO mapping).

So I still think this make sense.

Wasmtime GitHub notifications bot (May 29 2025 at 20:44):

Milek7 has marked PR #10865 as ready for review.

Wasmtime GitHub notifications bot (May 29 2025 at 20:44):

Milek7 edited a comment on PR #10865:

Okay, I got a little ahead of myself.

make_readonly won't be actually get called for modules created by Module::deserialize_raw. So first issue is not true.
However double-registration is still a concern.
Providing address of entire image to hooks is still useful for keeping track for unloading purposes (and maybe for no virtual memory target if they actually want to make that RO mapping).

So I still think this makes sense.

Wasmtime GitHub notifications bot (May 29 2025 at 20:45):

Milek7 edited PR #10865:

Module::deserialize_raw and CustomCodeMemory were introduced with aim of supporting no_std platforms. With small tweaks they can also be used for using Wasmtime on platforms that have full-blown virtual memory capabilities, but doesn't allow for directly mapping executable pages from user code instead limiting that capability for system loader. Adding features necessary for such platforms was previously attempted by #8245.

There are currently two issues with using Module::deserialize_raw for using images either statically linked into embedder executable or dynamically loaded shared objects:

• CodeMemory::publish will initially make entire image read-only, destroying executable permissions that cannot be restored by user code. This will happen even if CustomCodeMemory is provided.
• CodeMemory::publish will attempt to unconditionally register unwind information. As these are already properly registered by the system loader this is superfluous at best, or could fail module loading if system decides to return error on attempt for double-registration.

This commit solves these issues by:

• Moving responsibility for making image RO to CustomCodeMemory hook.
• Making CustomCodeMemory publishing hook return enum that tells what steps (only mapping, or mapping with registration) were performed, using default implementation only for actions not reported by the hook.
• Additionally CustomCodeMemory hooks also receive pointers to the entire image, not only executable section. This allows the embedder to keep track of the images and unload them in case they were loaded as dynamic shared object.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (May 29 2025 at 20:46):

Milek7 edited a comment on PR #10865:

Okay, I got a little ahead of myself.

make_readonly don't actually get called for modules created by Module::deserialize_raw. So first issue is not true.
However double-registration is still a concern.
Providing address of entire image to hooks is still useful for keeping track for unloading purposes (and maybe for no virtual memory target if they actually want to make that RO mapping).

So I still think this makes sense.

Wasmtime GitHub notifications bot (May 31 2025 at 02:24):

alexcrichton commented on PR #10865:

This seems reasonable to me to support, but what do you think of modeling this a bit differently? This is a pretty sensitive piece of the codebase which we want to ensure doesn't accidentally regress and having a lot of intertwined "if this else that" style logic can be relatively confusing to follow. Do you think it would be possible to internally model this as "assume there's always a custom memory publisher"? That way adding more hooks would mean adding more trait methods, and the "driver" would still be relatively straightforward of "do this, then that, then that".

A custom trait could then override any one particular step to a noop and that wouldn't involve a need for custom enums/etc which are pretty tightly bound to the exact structure of what we have today as well.

Wasmtime GitHub notifications bot (Jun 02 2025 at 13:53):

Milek7 commented on PR #10865:

Do you think about trait methods that would return bool indicating whether default implementation should be called, or calling only trait methods with default implementation provided as default CustomCodeMemory? In second case should MmapVec become part of public API?

Wasmtime GitHub notifications bot (Jun 04 2025 at 14:52):

alexcrichton commented on PR #10865:

Ideally we'd model everything with the trait to allow embedders to customize all the hooks necessary, but that's also not easy as there's internal types, like MmapVec, which we don't want to expose. This came up in https://github.com/bytecodealliance/wasmtime/issues/10740 where we have an internal trait with private types and a public trait with less functionality and that became a problem as well. Unfortunately I'm not sure how best to thread this needle...

Last updated: Dec 06 2025 at 07:03 UTC