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

Fri Apr 23 10:35:40 PDT 2021

MaskRay marked an inline comment as done.
MaskRay added a comment.

In D101116#2711479 <https://reviews.llvm.org/D101116#2711479>, @jhenderson wrote:

> 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?

GNU ld -r can create out of order `.rela.eh_frame`, even if I *don't* use a construct like `.eh_frame : { *b.o(.eh_frame) *a.o(.eh_frame) }`.

> This sounds like this check could cost quite a bit in a large link.

I cannot observe any performance difference adding a linear scan.

================
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.
----------------
jhenderson wrote:
> 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?
Not clear the trade-off is worthwhile. The template function needs to live in one header (our usage are in two .cpp files). On the other hand, there are only few lines and allocating a SmallVector and `rels = sorted;` cannot be dropped with a common function.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101116