Stream: git-wasmtime

Topic: wasmtime / PR #14788 cranelift-jit: assert that relocatio...


view this post on Zulip Wasmtime GitHub notifications bot (Oct 16 2026 at 21:47):

ilyas-mallah edited PR #14788.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 16 2026 at 21:48):

ilyas-mallah commented 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.

Made it a panic, checked in the relocation loop right before each write. I kept it in release builds because the offsets don't only come from cranelift-codegen. DataDescription::write_data_addr and define_function_bytes take them from the caller in safe code, so a debug assertion would still let a release build write past a definition. It's one compare per relocation.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 16 2026 at 21:48):

:memo: ilyas-mallah submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 16 2026 at 21:48):

:speech_balloon: ilyas-mallah created PR review comment:

Done, it's now Reloc::patched_len with an exhaustive match, so a new relocation kind won't compile until it states its width.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 16 2026 at 23:44):

github-actions[bot] added the label cranelift on PR #14788.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 08:01):

:memo: bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 08:01):

:speech_balloon: bjorn3 created PR review comment:

This reloc allows the linker to patch an entire sequence of instructions, not just the 4 bytes of the relative address.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 11:17):

ilyas-mallah updated PR #14788.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 11:17):

:memo: ilyas-mallah submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 11:17):

:speech_balloon: ilyas-mallah created PR review comment:

Good catch. patched_len now returns 12 for ElfX86_64TlsGd, covering the lea displacement and the call after it, and the out-of-bounds test has a case for it.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 21 2026 at 00:58):

ilyas-mallah updated PR #14788.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 21 2026 at 00:59):

:memo: ilyas-mallah submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 21 2026 at 00:59):

:speech_balloon: ilyas-mallah created PR review comment:

I also reworded the patched_len doc, which claimed to cover the whole instruction: it counts from the offset to the end of what a linker may rewrite, so the lea bytes before the displacement aren't included. The other TLS relocations each sit on the one instruction a linker rewrites, so their widths stay as they were.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 21 2026 at 08:59):

:thumbs_up: bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 23 2026 at 18:12):

:thumbs_up: cfallin submitted PR review:

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 23 2026 at 18:12):

cfallin added PR #14788 cranelift-jit: assert that relocations stay within their definition to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 23 2026 at 18:32):

github-merge-queue[bot] removed PR #14788 cranelift-jit: assert that relocations stay within their definition from the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 23 2026 at 19:02):

alexcrichton added PR #14788 cranelift-jit: assert that relocations stay within their definition to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 24 2026 at 01:44):

:check: alexcrichton merged PR #14788.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 24 2026 at 01:44):

alexcrichton removed PR #14788 cranelift-jit: assert that relocations stay within their definition from the merge queue.


Last updated: Oct 31 2026 at 23:03 UTC