Debugging: refactor stack frame cursor into frame handle abstraction.#12566
Debugging: refactor stack frame cursor into frame handle abstraction.#12566cfallin merged 10 commits intobytecodealliance:mainfrom
Conversation
This addresses some of the issues described bytecodealliance#12486: we need the ability to keep a handle to a stack frame as long as execution is frozen, and keep multiple of these handles around, alongside the `Store`, without any handle directly holding a borrow of the store. The frame handles work by means of an "execution version" scheme: the idea is that whenever any execution resumes in a given store, all handles to existing frames could be invalidated, but if no such execution occurs, all handles should still be valid. A tuple of (globally unique for process lifetime) store ID, and execution version within that store, should be sufficient to uniquely identify any frozen-stack period during execution. This accomplishes cheap handle invalidation without the need to track existing handles. This PR also implements a cache of parsed frame-table data. Previously this was lazily parsed by the cursor as it walked up a stack, but with multiple handles hanging around, and with handles meant to be cheap to hold and clone, and with handles being invalidated eagerly, it makes much more sense to persist this parsed metadata at the `Store` level. (It cannot persist at the `Engine` level because PCs are local per store.)
|
(I plan to add tests for various handle invalidation cases tomorrow -- sorry, missed including those here) |
github-actions
bot
added
the
wasmtime:api
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
In retrospect we probably should have had this earlier, but given all the unsafe here I think it'd be good to get Miri testing for this. Could you extend the pulley_provenance_test test in tests/all/pulley.rs (or something like that) to be able to exercise all this under Miri?
I'm getting some UB errors in miri deep in the |
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
That sounds reasonable to me yeah, and thanks!
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
fitzgen
left a comment
fitzgen
left a comment
There was a problem hiding this comment.
(realizing I forgot to hit submit on my pending review comments)
3 participants
This addresses some of the issues described #12486: we need the ability to keep a handle to a stack frame as long as execution is frozen, and keep multiple of these handles around, alongside the
Store, without any handle directly holding a borrow of the store.The frame handles work by means of an "execution version" scheme: the idea is that whenever any execution resumes in a given store, all handles to existing frames could be invalidated, but if no such execution occurs, all handles should still be valid. A tuple of (globally unique for process lifetime) store ID, and execution version within that store, should be sufficient to uniquely identify any frozen-stack period during execution. This accomplishes cheap handle invalidation without the need to track existing handles.
This PR also implements a cache of parsed frame-table data. Previously this was lazily parsed by the cursor as it walked up a stack, but with multiple handles hanging around, and with handles meant to be cheap to hold and clone, and with handles being invalidated eagerly, it makes much more sense to persist this parsed metadata at the
Storelevel. (It cannot persist at theEnginelevel because PCs are local per store.)Fixes #12486.