smarcd opened PR #14128 from smarcd:codex/debug-function-index to bytecodealliance:main:
Adds a host-only inverse to Instance::debug_function for debugging tools that need to serialize a same-instance funcref as a Wasm function index. The lookup never exposes VM pointers and returns None when guest debugging is disabled or the function is not part of the instance.\n\nTests cover private functions, imports, an unrelated host function, and disabled guest debugging.
smarcd requested alexcrichton for a review on PR #14128.
smarcd requested wasmtime-fuzz-reviewers for a review on PR #14128.
smarcd requested wasmtime-core-reviewers for a review on PR #14128.
smarcd requested wasmtime-compiler-reviewers for a review on PR #14128.
smarcd requested wasmtime-default-reviewers for a review on PR #14128.
github-actions[bot] added the label cranelift on PR #14128.
github-actions[bot] added the label cranelift:meta on PR #14128.
github-actions[bot] added the label cranelift:module on PR #14128.
github-actions[bot] added the label wasmtime:c-api on PR #14128.
github-actions[bot] added the label wasmtime:docs on PR #14128.
github-actions[bot] added the label isle on PR #14128.
github-actions[bot] commented on PR #14128:
Subscribe to Label Action
cc @cfallin, @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:meta", "cranelift:module", "isle", "wasmtime:c-api", "wasmtime:docs"Thus the following users have been cc'd because of the following labels:
- cfallin: isle
- fitzgen: isle
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
pchickey commented on PR #14128:
It looks like a bunch of commits from the 47 release branch got included in your PR branch for some reason - can you please rebase this cleanly on main?
smarcd updated PR #14128.
github-actions[bot] added the label wasmtime:api on PR #14128.
alexcrichton commented on PR #14128:
Could you detail your use case a bit more here? This is a pretty powerful debugging capability which also sort of inherently can't be efficient (e.g. the linear search here) and may also not hold up in future possible refactorings. Given the cost of supporting such an API I'd like to better understand the intended use case.
smarcd commented on PR #14128:
This is for a host-side debugging workflow. The debugger captures an instance’s private mutable state at a breakpoint/checkpoint, then materializes that state into an isolated Store so it can inspect or continue the snapshot without mutating the original execution.
This is deliberately not production runtime functionality. The path is only enabled with guest debugging, runs at debug snapshot boundaries rather than during normal execution, and is allowed to trade efficiency for a small, well-contained API.
The specific need for debug_function_index is that function references are store-local. To restore a captured table/global funcref in the isolated debugging store, the debugger needs to serialize the same-module function identity as an index and resolve it through debug_function in the destination instance.
cfallin commented on PR #14128:
The specific need for debug_function_index is that function references are store-local. To restore a captured table/global funcref in the isolated debugging store, the debugger needs to serialize the same-module function identity as an index and resolve it through debug_function in the destination instance.
But given that the implementation here is linear in the number of functions, your whole-store snapshot is going to run in quadratic time overall, which does not seem workable for anything semi-large. I'll second Alex's point that "linear search for this function" is not something we want to support.
I could see a
Func::eqimplementation making sense (because the primitive is harder to argue against -- it may be independently useful); if we also hadFunc::hash, then you could build a hashtable of funcrefs to defining instance and index within that instance in a single linear pass, then rename through that hashtable -- asymptotically better. I am not sure if I'm missing anything that would prevent us from providingFunc::eqthough (@alexcrichton ?).
smarcd updated PR #14128.
smarcd commented on PR #14128:
Fair point, and I think it's addressable without needing
Func::eq/Func::hash— the actual complaint is the O(n) work per lookup (and thus the O(n²) whole-store cost), not the shape of the API. I pushed a rewrite that makesdebug_function_indexitself O(1) instead of scanning every function.Both an instance's imported-function table and its defined-function funcref table are contiguous, fixed-stride arrays at statically known
VMContextoffsets, so aVMFuncRefpointer's position in either array is computable directly via pointer arithmetic instead of a linear scan:
- Imported functions are indexed by
FuncIndexdirectly, so that's a direct offset computation.- Defined ("escaped") functions are indexed by a compact
FuncRefIndex. That index is not assigned inFuncIndexorder (slots are handed out in the order functions are discovered to escape during translation — e.g. export declaration order — not declaration order), so resolving it back to aFuncIndexneeds an explicit reverse table rather than arithmetic or a binary search.That reverse table only depends on compiled module metadata, not on any particular
Instance, so it's built once, lazily, and cached onModule— shared by everyInstanceand every debug snapshot of that module, rather than rebuilt per lookup or per instance. So a whole-store snapshot doing this once per captured funcref is now O(n) total (amortized), not O(n²).Added a regression test (
debug_function_index_with_non_monotonic_escape_order) that deliberately scrambles escape order via a tableelemsegment, to pin down that the reverse table is actually used correctly rather than assuming escape order tracks function-index order.Also took the opportunity to rebase cleanly on main per @pchickey's comment above.
cfallin commented on PR #14128:
I think that is still the sort of complexity that we would rather not take on if we don't have to: it makes the code more entangled (we now have a dependency on the vmctx layout; if the scheme ever needs to change we are now more restricted because we need to provide this property), and it's just a lot of delicate logic.
For reference, the "debug" variants of accessors have geneally been simple O(1) holes in the encapsulation, where under-the-hood Wasmtime already has the appropriate private accessors. This is a whole lot of new functionality instead for a niche use-case.
Is there a reason that
Func::eq/Func::hashand then an external implementation of your snapshot/clone algorithm couldn't work?
smarcd updated PR #14128.
smarcd commented on PR #14128:
That's a fair concern — coupling to
VMContext's internal layout is more entanglement than this is worth. I droppeddebug_function_indexand pushedFunc::eq/Func::hashinstead, per your suggestion.
Funcnow implementsPartialEq/Eq/Hashas pointer-identity equality (same store, same underlyingVMFuncRef). Comparing/hashing only reads theStoreIdand the raw pointer bits — no dereference, no dependency onVMContextoffsets, no assumptions aboutFuncRefIndexassignment order.With that, our debugger can build the
Func -> indexmap itself: walk the instance's function index space once with the existingInstance::debug_function, insert each(Func, index)pair into aHashMap, and invert a captured funcref through that map at snapshot time. Same result asdebug_function_index, but the O(n) work (and any layout assumptions) live in our code instead of wasmtime's.Added a regression test (
debug_function_identity_round_trips_through_a_caller_built_map) that builds exactly that map over a module whose functions are placed into a table out of index order, to make sure the identity is real pointer/store identity and not something that only happens to work when escape order matches function-index order.Much smaller diff now — just the two trait impls and a test.
:memo: cfallin submitted PR review:
Thanks -- a few comments below.
Please feel free to change the title of this PR as well -- you're really adding
EqandHashtoFunc, rather than adding any debug-specific mechanisms at all.
:speech_balloon: cfallin created PR review comment:
This unit test is fairly excessive -- again the narrative comment is unnecessary, and the test itself is testing the actual Wasmtime functionality in a fairly baroque way. All we really need to test is that
f1 == f2works when functions are fetched in different ways and/or carried through imports/exports between modules in the store, etc (and likewise for the hash value). Can you do that instead, and put it alongside other tests of theFunc-related APIs rather than indebug?
:speech_balloon: cfallin created PR review comment:
No need for this narrative paragraph -- we generally don't have comments describing very specific single use-cases like this, nor a comparison to an alternative that used to (or in this case, never did) exist in the code.
The bit about shallow equality is also self-evident and probably not needed (a reasonable user would not expect
f1 == f2to, say, prove equivalence of two different algorithms).The paragraph below about only comparing raw pointer bits is fine (a pseudo "safety comment" even though there's no literal unsafe block) I think.
Last updated: Aug 30 2026 at 09:07 UTC