[PATCH] D62365: ELF: Don't reuse a thunk in a different partition.

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I think this is the right thing to do. I've left a suggestion to abstract the names a bit as I think that there are some other cases where we don't want to reuse Thunks across OutputSection boundaries.

A theoretical test case

          .section .sec1, "ax", %progbits
          bl far
  
          .section .sec2, "ax", %progbits
          bl far
  
          .section .sec3, "ax", %progbits
          .globl far
          .type far, %function
  far:    bx lr

  SECTIONS {
    .sec1 0x1000 (OVERLAY) : AT (0x1000) { *(.sec1) }
    .sec2 0x1000 (OVERLAY) : AT (0x2000) { *(.sec2) }
    .far  0x10000000 : { *(.sec3) }
  }

ld.bfd correctly produces a Thunk in both of .sec1 and .sec2 as .sec1 and .sec2 will not be in memory at the same time. It looks like LLD isn't handling this test case well at the moment. I get relocation out of range errors and .sec1 and .sec2 being given a VMA and LMA of 0. I think that this is because the OVERLAY also has the property of removing SHF_ALLOC from the OutputSection which seems to be having some other side-effects. I'll raise a PR.



================
Comment at: lld/ELF/Relocations.cpp:1626
   for (Thunk *T : *ThunkVec)
-    if (T->isCompatibleWith(Type) &&
+    if (T->getThunkTargetSym()->Section->Partition == IS->Partition &&
+        T->isCompatibleWith(Type) &&
----------------
There are some other cases where we won't want to reuse across an OutputSection boundary, for example when the OutputSections are overlays. Would it be worth extracting into something like if (CompatibleOutputSections(T->getThunkTargetSym()->Section, IS)) where we could add to these conditions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62365