ilyas-mallah edited PR #14788.
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.
:memo: ilyas-mallah submitted PR review.
: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.
github-actions[bot] added the label cranelift on PR #14788.
:memo: bjorn3 submitted PR review.
: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.
ilyas-mallah updated PR #14788.
:memo: ilyas-mallah submitted PR review.
:speech_balloon: ilyas-mallah created PR review comment:
Good catch.
patched_lennow returns 12 forElfX86_64TlsGd, covering theleadisplacement and thecallafter it, and the out-of-bounds test has a case for it.
ilyas-mallah updated PR #14788.
:memo: ilyas-mallah submitted PR review.
: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.
:thumbs_up: bjorn3 submitted PR review.
:thumbs_up: cfallin submitted PR review:
Thanks!
cfallin added PR #14788 cranelift-jit: assert that relocations stay within their definition to the merge queue.
github-merge-queue[bot] removed PR #14788 cranelift-jit: assert that relocations stay within their definition from the merge queue.
alexcrichton added PR #14788 cranelift-jit: assert that relocations stay within their definition to the merge queue.
:check: alexcrichton merged PR #14788.
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