[PATCH] D112063: [lld][ELF] Add first bits to support relocation relaxations for AArch64

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexander-shaposhnikov added inline comments.


================
Comment at: lld/ELF/Arch/AArch64.cpp:578
+  // always appear in pairs.
+  for (size_t i = 0, size = relocs.size(); i + 1 < size; ++i) {
+    if (relocs[i].type == R_AARCH64_LD64_GOT_LO12_NC) {
----------------
alexander-shaposhnikov wrote:
> MaskRay wrote:
> > `i + 1 < size` has a bug that the last relocation is not checked.
> > 
> > If `i` is moved outside the loop one single `safeToRelaxAdrpLdr = i == size;` can be used.
> > 
> > That said, the same check has been repeated in `tryRelaxAdrpLdr`. Maybe `AArch64Relaxer` is not needed.
> that was intentional (potentially not checking the last relocation), but to be rigorous we can do that.
as Wilco pointed out, for the extra safety we check if these relocations show up in pairs (R_AARCH64_ADR_GOT_PAGE and R_AARCH64_LD64_GOT_LO12_NC), this needs to happen in advance before any changes/writes happen.
Rather than keeping the state in boolean variables / exposing these details / complexity to higher level code it seems to be better to encapsulate this logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112063