cranelift: Add stack support to the interpreter with virtual addresses #3187
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
cranelift
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "cranelift", "wasmtime:api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
afonso360
force-pushed
the
interperter-stack-vaddr
branch
2 times, most recently
from
a6f7dbd to
908d245
Compare
cfallin
reviewed
Aug 16, 2021
Member
cfallin
left a comment
cfallin
left a comment
There was a problem hiding this comment.
Some thoughts on various details here -- I just zoomed in on the most important bits but will defer to @abrown for the final LGTM. Thanks for putting this together so quickly after we discussed it; I'm looking forward to more flexible interpreter-based fuzzing using this!
afonso360
force-pushed
the
interperter-stack-vaddr
branch
2 times, most recently
from
acd3634 to
9a086eb
Compare
Member
|
@afonso360 if you rebase this on |
We also change the approach for heap loads and stores. Previously we would use the offset as the address to the heap. However, this approach does not allow using the load/store instructions to read/write from both the heap and the stack. This commit changes the addressing mechanism of the interpreter. We now return the real addresses from the addressing instructions (stack_addr/heap_addr), and instead check if the address passed into the load/store instructions points to an area in the heap or the stack.
Adds a Virtual Addressing scheme that was discussed as a better alternative to returning the real addresses. The virtual addresses are split into 4 regions (stack, heap, tables and global values), and the address itself is composed of an `entry` field and an `offset` field. In general the `entry` field corresponds to the instance of the resource (e.g. table5 is entry 5) and the `offset` field is a byte offset inside that entry. There is one exception to this which is the stack, where due to only having one stack, the whole address is an offset field. The number of bits in entry vs offset fields is variable with respect to the `region` and the address size (32bits vs 64bits). This is done because with 32 bit addresses we would have to compromise on heap size, or have a small number of global values / tables. With 64 bit addresses we do not have to compromise on this, but we need to support 32 bit addresses.
afonso360
force-pushed
the
interperter-stack-vaddr
branch
from
9a086eb to
84ed9b2
Compare
32 tasks
uweigand
added a commit
to uweigand/wasmtime
that referenced
this pull request
Sep 11, 2021
PR bytecodealliance#3187 introduced a change to the write_to_slice and read_from_slice routines in data_value.rs that switched byte order on big-endian systems: the code used to use native byte order, and now hard-codes little-endian byte order. Fix by using native byte order again.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey,
This PR is #3164 with an additional Virtual Addressing scheme that was discussed as a better alternative to returning the real addresses.
The virtual addresses are split into 4 regions (stack, heap, tables and global values), and the address itself is composed of an
entryfield and anoffsetfield. In general theentryfield corresponds to the instance of the resource (e.g. table5 is entry 5) and theoffsetfield is a byte offset inside that entry.There is one exception to this which is the stack, where due to only having one stack, the whole address is an offset field.
The number of bits in entry vs offset fields is variable with respect to the region and the address size (32bits vs 64bits). This is done because with 32 bit addresses we would have to compromise on heap size, or have a small number of global values / tables. With 64 bit addresses we do not have to compromise on this, but we need to support 32 bit addresses.
cc: @abrown @cfallin