[PATCH] D101116: [ELF] Support .rela.eh_frame with unordered r_offset values

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Can (and if so should - perhaps not) we avoid this patch by fixing the input in the first place? Under what circumstances can the relocations appear out of order?



================
Comment at: lld/ELF/InputSection.cpp:1329
 void EhInputSection::split(ArrayRef<RelTy> rels) {
+  // getReloc expects the relocations to be sorted by r_offset. See the comment
+  // in scanRelocs.
----------------
MaskRay wrote:
> It is unfortunate that there are two pieces of code.
> `RelTy` is parametric so we cannot add a member in `EhInputSection` caching the input relocations.
> 
I might be missing something, but surely you could have some templated function that would allow you to avoid the code duplication at least?


================
Comment at: lld/ELF/InputSection.cpp:1335
+  };
+  if (!llvm::is_sorted(rels, cmp)) {
+    sorted.assign(rels.begin(), rels.end());
----------------
This sounds like this check could cost quite a bit in a large link. Have you done any performance comparisons of before and after of such a link? Given that the normal case is that things are sorted, I think we need to be somewhat careful here. Same goes below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101116