alexcrichton opened PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
fitzgen submitted PR Review.
fitzgen submitted PR Review.
fitzgen created PR Review Comment:
"emit" -> "omit"?
alexcrichton updated PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton updated PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton updated PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton updated PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton updated PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton requested sunfishcode for a review on PR #1513.
sunfishcode submitted PR Review.
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
typo: a -> at
sunfishcode created PR Review Comment:
Code-quote
LinearMemory
.
sunfishcode created PR Review Comment:
Because wasm sometimes measures sizes in pages, we should clarify that this is a size in bytes.
sunfishcode created PR Review Comment:
This first-to-last property is interesting, as optimizers could be tempted to optimize memory accesses in ways that could change the access order -- for example optimizing
load.i32(x) & 0xff000000
into a one-byte load from the last byte. Could you add some text to https://github.com/bytecodealliance/wasmtime/blob/master/cranelift/docs/ir.md#memory-operation-flags that says that when an access is notnotrap
, the bytes are defined to be accessed first-to-last, so that we at least document it?
sunfishcode created PR Review Comment:
One subtlety that would be good to clarify here is that this is talking about reserving address space, not necessarily actual memory.
sunfishcode created PR Review Comment:
This could produce a value which is lower than the given
val
, but I guess that's ok here because it'd be an unusably large value in either case.
sunfishcode created PR Review Comment:
"size in bytes", and so on below.
sunfishcode created PR Review Comment:
The page size will always be a power of 2, so we can do
val & -page_size
.
alexcrichton updated PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
So thinking more about this, I think I just carried this comment over from somewhere that I didn't fully understand. I think this property isn't actually necessary (and may not be necessary to document).
The main assumption I think we can make is that all our accesses are smaller than the size of a page (e.g. the minimum size of the guard region). In that case the first byte is either:
- in wasm memory which is normal-bounds-checked and all remaining bytes are either in wasm memory or the guard-page
- in the guard page, in which case the bounds check will fail.
Does that sound right to you as well? If so I'll reword this to and probably throw in an assert that the
width
is less than 1k or something like that with a doc comment explaining.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Ah true yeah, but I was mostly just striving to do something reasonable-ish here.
alexcrichton updated PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
eust-dfinity submitted PR Review.
eust-dfinity created PR Review Comment:
Actually that example is allocating one guard page
alexcrichton updated PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I went ahead and tried to update the comment with my thinking here.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
True yeah, I was mostly just getting this test to pass. Since the JIT is configured that there might not be a guard page then the guard page allocation here shouldn't be necessary.
alexcrichton updated PR #1513 from configurable-memory-bounds
to master
:
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
New configuration options via
wasmtime::Config
are exposed to
configure the tunable limits of how memories are allocated and such.The
MemoryCreator
trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.The new
Config
methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.The
MemoryCreator
trait previously only allocated memories with a
MemoryType
, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
MemoryCreator::new_memory
to reflect this.Finally the fuzzing with
Config
turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!Closes #1501
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
sunfishcode merged PR #1513.
plicease submitted PR Review.
plicease created PR Review Comment:
After testing this from Perl, I think the
_set
suffix was accidentally omitted from this declaration. Same forwasmtime_config_dynamic_memory_guard_size
below.
Last updated: Dec 23 2024 at 12:05 UTC