[PATCH] D146618: [ExecutionEngine] Fix R_AARCH64_ABS{16, 32} assertion

[Denis Revunov via Phabricator via llvm-commits]

treapster added inline comments.


================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:429
     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);
----------------
MaskRay wrote:
> MaskRay wrote:
> > 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.
> Sorry, I'll fix my conditions according to lld/ELF/Target.h in a few hours...
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146618/new/

https://reviews.llvm.org/D146618