Fix relocation addend sign extension on 32-bit platforms#4846
Fix relocation addend sign extension on 32-bit platforms#4846sumleo wants to merge 3 commits intobytecodealliance:mainfrom
Conversation
When loading relocations on 32-bit platforms, the addend is read as uint32 and zero-extended to uint64, which corrupts negative addends. For example, -4 (0xFFFFFFFC) becomes 4294967292 instead of remaining -4. Use int32 with sign extension to int64, matching the Windows code path which already handles this correctly.
sumleo
requested review from
TianlongLiang,
lum1n0us,
no1wudi and
yamt
as code owners
core/iwasm/aot/aot_loader.c
Outdated
| else { | ||
| uint32 offset32, addend32; | ||
| uint32 offset32; | ||
| int32 addend32; |
There was a problem hiding this comment.
L3873 read_uint32() expects addend32 as an unsigned. No need to change its type to int32.
There was a problem hiding this comment.
Done. Reverted addend32 back to uint32 and now sign-extend via a double cast (int64)(int32)addend32 instead. This keeps read_uint32() happy while still preserving proper sign extension for negative addends on 32-bit platforms.
core/iwasm/aot/aot_loader.c
Outdated
| relocation->relocation_offset = (uint64)offset32; | ||
| read_uint32(buf, buf_end, addend32); | ||
| relocation->relocation_addend = (uint64)addend32; | ||
| relocation->relocation_addend = (int64)addend32; |
There was a problem hiding this comment.
Yes. there is a risk when casting uint64 to int64. But personally, I am curious that is it reported by a compiler or analysis tool?
There was a problem hiding this comment.
This was found during code review. On 32-bit platforms, the relocation addend is read as a uint32 and then cast to uint64. If the addend is negative (e.g., -4 stored as 0xFFFFFFFC), the unsigned widening produces 0x00000000FFFFFFFC instead of the correct 0xFFFFFFFFFFFFFFFC. The fix now uses (int64)(int32)addend32 to properly sign-extend while keeping addend32 as uint32 for read_uint32().
2 participants
Summary
On 32-bit platforms,
load_relocation_sectionreads the relocation addend asuint32and zero-extends it touint64. This corrupts negative addends — for example, an addend of -4 (0xFFFFFFFCas a 32-bit value) becomes4294967292instead of remaining-4(0xFFFFFFFFFFFFFFFCas a 64-bit value).The Windows code path (around line 3616) already handles this correctly with proper sign extension.
Changes
(int64)(int32)double cast when wideningaddend32to 64 bits, so negative values are sign-extended rather than zero-extendedaddend32remainsuint32as expected byread_uint32()offset32variable remainsuint32since relocation offsets are unsignedTest plan
sizeof(void *) != 8)