[ExecutionEngine] Fix R_AARCH64_ABS{16,32} assertion

[treapster]

Summary

Previous version of assertion effectively prevented negative Result values because it didn't cast to int64_t. Fix it by adding the cast.

[treapster]

This commit has the same tests failing so it's not me:)

[treapster]

Gentle ping

[MaskRay]

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

429

The change is incorrect.

R_AARCH64_ABS16 permits either signed or unsigned narrow values. See https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst

The original assert conditions had off-by-1 issues, which I just fixed in 7b58259481417bb22d144a9c12ee8f4fb0a046e0.

[MaskRay]

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

429

Sorry, I'll fix my conditions according to lld/ELF/Target.h in a few hours...

[treapster]

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

429

Why is it incorrect? It follows the same logic as assertions in other cases, we cast to signed because otherwise negative value results in second expression being false. Your off-by-one fix still does not allow negative values.

[treapster]

@MaskRay

#include <iostream>
#include <cstdint>
#define INT32_MIN		(-2147483647-1)
#define UINT32_MAX		(4294967295U)

int main() {
    uint64_t Result = -4ull;
    std::cout << "Original: " <<
        (static_cast<int64_t>(Result) >= INT32_MIN && Result < UINT32_MAX) << "\n";

    std::cout << "Less or equal: " <<
        (static_cast<int64_t>(Result) >= INT32_MIN && Result <= UINT32_MAX) << "\n";

    std::cout << "With cast: " <<
        (static_cast<int64_t>(Result) >= INT32_MIN &&
        static_cast<int64_t>(Result) <= UINT32_MAX) << "\n";

}

Results in

Original: 0
Less or equal: 0
With cast: 1

We need both <= and cast.

[treapster]

We need both <= and cast.

But your SignExtend patch is doing the same thing, thanks.

[treapster]

Fixed at https://reviews.llvm.org/rGed92c369e5436bea9abf7eed986bae640ac13147

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

void RuntimeDyldELF::resolveAArch64Relocation(const SectionEntry &Section,
   switch (Type) {
   default:
     report_fatal_error("Relocation type not implemented yet!");
     break;
   case ELF::R_AARCH64_NONE:
     break;
   case ELF::R_AARCH64_ABS16: {
     uint64_t Result = Value + Addend;
-    assert(static_cast<int64_t>(Result) >= INT16_MIN && Result < UINT16_MAX);
+    assert(static_cast<int64_t>(Result) >= INT16_MIN &&
+           static_cast<int64_t>(Result) < UINT16_MAX);
     write(isBE, TargetPtr, static_cast<uint16_t>(Result & 0xffffU));
     break;
   }
   case ELF::R_AARCH64_ABS32: {
     uint64_t Result = Value + Addend;
-    assert(static_cast<int64_t>(Result) >= INT32_MIN && Result < UINT32_MAX);
+    assert(static_cast<int64_t>(Result) >= INT32_MIN &&
+           static_cast<int64_t>(Result) < UINT32_MAX);
     write(isBE, TargetPtr, static_cast<uint32_t>(Result & 0xffffffffU));
     break;
   }
   case ELF::R_AARCH64_ABS64:
     write(isBE, TargetPtr, Value + Addend);
     break;
   case ELF::R_AARCH64_PLT32: {
     uint64_t Result = Value + Addend - FinalAddress;