Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect handling of ARM64_RELOC_PAGEOFF12 by RuntimeDyldMachOAArch64::resolveRelocation #125850

Open
JohnReppy opened this issue Feb 5, 2025 · 1 comment

Comments

@JohnReppy
Copy link

Summary

In certain situations, the RuntimeDyldMachOAArch64::resolveRelocation method patches an add instruction with an incorrect immediate operand.

Details

The instruction being patched is an add with a 12-bit immediate that is paired with an adrp instruction to compute the address of a label. The calculation use to compute the immediate is

(Value + RE.Addend) & 0xFFF

The value of (Value + RE.Addend) is the memory
The variable Value holds the base address of the memory allocated by the loader's memory manager to contain the Section, and (Value + RE.Addend) is the address of the label in
that memory. The problem is that Value is not guaranteed to be 12-bit aligned and so the computed immediate may be incorrect.

My code generator is based on LLVM 18.1.8, but the implementation of RuntimeDyldMachOAArch64::resolveRelocation does not appear to have changed in 19.1.7.

Proposed Fix

In debugging this issue, I've noticed that RE.Addend appears to always hold the correct patch value, so think the correct computation should either be

RE.Addend & 0xFFF

or

(Value - Section.getLoadAddress() + RE.Addend) & 0xFFF

With the caveat that in my code, I've only seen the add instruction patched, but I think that this also works for patching load/store instruction (the other use of ARM64_RELOC_PAGEOFF12).

An alternative fix might be to set the alignment requirement for the code section to $2^12$ in the call to MemManager::reserveAllocationSpace, but that seems more fragile. I also noticed that the MCJIT execution engine avoids this bug because it page-aligns the memory allocated for sections.

@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/issue-subscribers-backend-aarch64

Author: John Reppy (JohnReppy)

## Summary In certain situations, the `RuntimeDyldMachOAArch64::resolveRelocation` method patches an `add` instruction with an incorrect immediate operand.

Details

The instruction being patched is an add with a 12-bit immediate that is paired with an adrp instruction to compute the address of a label. The calculation use to compute the immediate is

(Value + RE.Addend) & 0xFFF

The value of (Value + RE.Addend) is the memory
The variable Value holds the base address of the memory allocated by the loader's memory manager to contain the Section, and (Value + RE.Addend) is the address of the label in
that memory. The problem is that Value is not guaranteed to be 12-bit aligned and so the computed immediate may be incorrect.

My code generator is based on LLVM 18.1.8, but the implementation of RuntimeDyldMachOAArch64::resolveRelocation does not appear to have changed in 19.1.7.

Proposed Fix

In debugging this issue, I've noticed that RE.Addend appears to always hold the correct patch value, so think the correct computation should either be

RE.Addend & 0xFFF

or

(Value - Section.getLoadAddress() + RE.Addend) & 0xFFF

With the caveat that in my code, I've only seen the add instruction patched, but I think that this also works for patching load/store instruction (the other use of ARM64_RELOC_PAGEOFF12).

An alternative fix might be to set the alignment requirement for the code section to $2^12$ in the call to MemManager::reserveAllocationSpace, but that seems more fragile. I also noticed that the MCJIT execution engine avoids this bug because it page-aligns the memory allocated for sections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants