Wasmtime GitHub notifications bot (Oct 16 2026 at 19:05): Wasmtime GitHub notifications bot (Oct 16 2026 at 19:05):ilyas-mallah opened PR #14788 from ilyas-mallah:cranelift-jit-reloc-checks to bytecodealliance:main:
CompiledBlob::perform_relocations writes each relocation through a raw pointer at its offset into the definition. The only check is a debug_assert that the offset is below the size. Release builds have no check, and the debug one ignores the 4 or 8 bytes the write covers. The offsets come from safe APIs (DataDescription::write_data_addr and write_function_addr, and the relocs of define_function_bytes), so safe code can make finalize_definitions write past a definition. In a release build, an Abs8 relocation at offset 4 of an 8-byte data object overwrote the first 4 bytes of the next object.
define_data and define_function_bytes now check each relocation's full span and return ModuleError::Backend for one that doesn't fit. The debug_assert becomes an assert, since the unsafe writes rely on it. Both new tests fail on main in a debug build and pass with this change in debug and release builds.
Would you rather have a panic here than an error? I chose an error because both functions already return ModuleResult.
Wasmtime GitHub notifications bot (Oct 16 2026 at 19:05):ilyas-mallah requested cfallin for a review on PR #14788.
Wasmtime GitHub notifications bot (Oct 16 2026 at 19:05):ilyas-mallah requested wasmtime-compiler-reviewers for a review on PR #14788.
Wasmtime GitHub notifications bot (Oct 16 2026 at 21:09):bjorn3 commented on PR #14788:
Panic makes sense to me. And I think the check could be done inside the loop that processes the relocations right before applying them instead of iterating a second time.
Wasmtime GitHub notifications bot (Oct 16 2026 at 21:11)::memo: bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Oct 16 2026 at 21:11)::speech_balloon: bjorn3 created PR review comment:
Maybe this should be a method on the
Relocenum? That way it is a bit less likely to go out of sync as new reloc kinds are added. And it should probably do an exhaustive match to mitigate the rest of the potential for going out of sync.
Wasmtime GitHub notifications bot (Oct 16 2026 at 21:11):bjorn3 edited a comment on PR #14788:
Panic makes sense to me (perhaps a debug assertion?). And I think the check could be done inside the loop that processes the relocations right before applying them instead of iterating a second time.
Wasmtime GitHub notifications bot (Oct 16 2026 at 21:12):bjorn3 edited a comment on PR #14788:
Panic makes sense to me (perhaps a debug assertion? this can't happen without a bug in cranelift-codegen, so I'm not sure how much benefit there is to unconditionally checking it). And I think the check could be done inside the loop that processes the relocations right before applying them instead of iterating a second time.
Wasmtime GitHub notifications bot (Oct 16 2026 at 21:47):ilyas-mallah updated PR #14788.
Last updated: Oct 31 2026 at 23:03 UTC