Skip to content

Add bounds-checked unsafe intrinsics#13597

Merged
fitzgen merged 5 commits into
bytecodealliance:mainfrom
fitzgen:wasmtime-bounds-checked-unsafe-intrinsics
Jun 10, 2026
Merged

Add bounds-checked unsafe intrinsics#13597
fitzgen merged 5 commits into
bytecodealliance:mainfrom
fitzgen:wasmtime-bounds-checked-unsafe-intrinsics

Conversation

@fitzgen

@fitzgen fitzgen commented Jun 9, 2026

Copy link
Copy Markdown
Member

These intrinsics are similar to the unchecked versions, but instead of taking a raw address, take a base, offset, and length. They do a bounds check first, and trap on out-of-bounds accesses or perform the actual in-bounds access. When Spectre mitigations are enabled, their bounds check has Spectre mitigations applied.

@fitzgen fitzgen requested review from a team as code owners June 9, 2026 15:47
@fitzgen fitzgen requested review from cfallin and removed request for a team June 9, 2026 15:47

@cfallin cfallin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea; the bounds-checking logic looks right to me. Just a few comments below.


let [_callee_vmctx, _caller_vmctx, base_address, offset, length] = *self.abi_load_params()
else {
unreachable!()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More descriptive panic message here? Note that the above corresponds to a signature declared elsewhere such that mismatches will be caught earlier?

.trapz(addr, ir::TrapCode::HEAP_OUT_OF_BOUNDS);
addr
}
// Otherwise, when Specter mitigations are disabled, just conditionally

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Specter/Spectre/

///
/// See [`TrampolineCompiler::checked_native_addr`] for details; this is the
/// equivalent for the inlined translation path.
fn checked_native_addr(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deja vu -- is there any way to deduplicate the two implementations here? Especially if we add any other kind of logic to bounds-checking in the future, and especially as it's safety-critical code, it's unfortunate it has to be duplicated...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deduped. Will send a follow up to dedupe the rest of the methods as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up is here: #13601

@fitzgen fitzgen force-pushed the wasmtime-bounds-checked-unsafe-intrinsics branch from 19ed841 to 0b42dba Compare June 9, 2026 16:01
@fitzgen fitzgen enabled auto-merge June 9, 2026 16:21
@fitzgen fitzgen added this pull request to the merge queue Jun 9, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 9, 2026
@fitzgen fitzgen enabled auto-merge June 9, 2026 17:20
@fitzgen fitzgen added this pull request to the merge queue Jun 9, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 9, 2026
@fitzgen fitzgen enabled auto-merge June 9, 2026 18:06
@github-actions github-actions Bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Jun 9, 2026
fitzgen added 5 commits June 10, 2026 09:03
These intrinsics are similar to the unchecked versions, but instead of taking a
raw address, take a base, offset, and length. They do a bounds check first, and
trap on out-of-bounds accesses or perform the actual in-bounds access. When
Spectre mitigations are enabled, their bounds check has Spectre mitigations
applied.
This means we will correctly call the raise-trap libcall when signals-based
traps are disabled. However, it also means that `UnsafeIntrinsicCompiler` needed
to be refactored a bit to take a `FuncBuilder` rather than a `FuncCursor`, and
additionally take a generic `T: TranslateTrap`.

This should fix the s390x errors in CI.
@fitzgen fitzgen force-pushed the wasmtime-bounds-checked-unsafe-intrinsics branch from 12421a9 to c78b73b Compare June 10, 2026 16:16
@fitzgen

fitzgen commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

@cfallin mind taking a look at commit c78b73b ? I had to do a bit of a refactor to make this work when signals-based traps are disabled

@fitzgen fitzgen added this pull request to the merge queue Jun 10, 2026
Merged via the queue into bytecodealliance:main with commit 01094f4 Jun 10, 2026
221 of 237 checks passed
@fitzgen fitzgen deleted the wasmtime-bounds-checked-unsafe-intrinsics branch June 10, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasmtime:api Related to the API of the `wasmtime` crate itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants