[RuntimeDyld][ELF] Unwanted sign-extension when computing stub addresses. #94478
Comments
|
I tried to reproduce this issue on Arm 32 bit and couldn't so -
Makes sense here. In Arm's 32 bit ABI it notes that "Pointer arithmetic should be unsigned." (https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst). So this will be why it didn't show up on that platform. |
Casting the result of `Section.getAddressWithOffset()` goes wrong if we are on a 32-bit platform whose addresses are regarded as signed; in that case, just doing ``` (uint64_t)Section.getAddressWithOffset(...) ``` or ``` reinterpret_cast<uint64_t>(Section.getAddressWithOffset(...)) ``` will result in sign-extension. We use these expressions when constructing branch stubs, which is before we know the final load address, so we can just switch to the `Section.getLoadAddressWithOffset(...)` method instead. Doing that is also more consistent, since when calculating relative offsets for relocations, we use the load address anyway, so the code currently only works because `Section.Address` is equal to `Section.LoadAddress` at this point. Fixes llvm#94478.
Yeah, @mgorny noticed it on 32-bit x86, where they are treated as signed. In that case, it caused my new test for cross-section branches on AArch64 to fail with an assertion failure, because of the unwanted sign extension. I'm raising a PR to fix the problem at the moment. |
|
(See the comments on the end of #92245.) |
Casting the result of `Section.getAddressWithOffset()` goes wrong if we are on a 32-bit platform whose addresses are regarded as signed; in that case, just doing ``` (uint64_t)Section.getAddressWithOffset(...) ``` or ``` reinterpret_cast<uint64_t>(Section.getAddressWithOffset(...)) ``` will result in sign-extension. We use these expressions when constructing branch stubs, which is before we know the final load address, so we can just switch to the `Section.getLoadAddressWithOffset(...)` method instead. Doing that is also more consistent, since when calculating relative offsets for relocations, we use the load address anyway, so the code currently only works because `Section.Address` is equal to `Section.LoadAddress` at this point. Fixes #94478.
The RuntimeDyldELF implementation tries to make a stub, it uses an expression like
to get the address of the stub it's generating as a
uint64_t.Sadly, on 32-bit platforms where addresses are regarded as signed, because
getAddressWithOffset()returns auint8_t *pointer, this can cause unwanted sign-extension, so if (for instance) the code has been loaded at0xf1a10000, and the stub is at offset4, the resultinguint64_twill be0xfffffffff1a10004instead of the expected0xf1a10004.The text was updated successfully, but these errors were encountered: