[ELF] Fix sorting in combrelocs mode and add DT_REL(A)COUNT to .dynamic

[evgeny777]

Summary

Our dynamic linker depends on this feature, so it would be nice to have.

[grimar]

ELF/OutputSections.cpp

364 ↗

(On Diff #68529)

This breaks the optimization implemented in D19528 I think.
We found that sorting by symbol makes sence. So that should be done even for
relative relocations. I would suggest something like:

bool ARel = A.Type == Target->RelativeRel;
bool BRel = B.Type == Target->RelativeRel;
if (ARel && !BRel)
  return true;
else if (!ARel && BRel)
  return false;

return A.getSymbol(Config->Mips64EL) < B.getSymbol(Config->Mips64EL);

test/ELF/aarch64-tls-gdie.s

27 ↗

(On Diff #68529)

0x0B0 != 192

test/ELF/aarch64-tls-ie.s

34 ↗

(On Diff #68529)

Ditto.

test/ELF/combrelocs.s

43 ↗

(On Diff #68529)

You probably want to check that RELACOUNT is present instead of removing it.
Also checking of its value may be useful.

test/ELF/gotpc-relax-nopic.s

19 ↗

(On Diff #68529)

Excessive new lines.

[ruiu]

Did you have comments on George's comments?

[evgeny777]

Sorry, I missed the comments, because notifications are not working for me today for some reason.
Review updated.

[evgeny777]

Any comments?

[evgeny777]

Diff updated

a) Fix bug in calculating number of relative relocs
b) When number of relative relocs is zero DT_REL(A)COUNT is not emitted. This greatly decreased number of patched unit tests
c) For all modified tests, I've added addition check that DET_REL(A)COUNT matches actual number of relative relocs

[ruiu]

ELF/OutputSections.cpp

672–674 ↗

(On Diff #69491)

I'm not sure if it is the right thing to not emit DT_REL[A]COUNT if there are no relative relocations. If the number of relative relocations is zero, shouldn't we write zero?

[evgeny777]

ELF/OutputSections.cpp

672–674 ↗

(On Diff #69491)

This doc:
https://docs.oracle.com/cd/E19957-01/806-0641/6j9vuqujs/index.html

Says that DT_RELACOUNT is an optional field. Our dynamic linker assumes that number of relative relocs is zero, unless there is a DT_REL(A)COUNT tag. That said, I don't see any reason for writing it always.

[ruiu]

LGTM. If an absence of DT_RELCOUNT doesn't cause any trouble, this is fine by me. By the way, by "our dynamic loader", did you mean a dynamic loader of your operating system or something? Just curious.

[evgeny777]

Yes,it's dynamic loader of our OS.

lld/trunk/ELF/OutputSections.h

 public:
   RelocationSection(StringRef Name, bool Sort);
   void addReloc(const DynamicReloc<ELFT> &Reloc);
   unsigned getRelocOffset();
   void finalize() override;
   void writeTo(uint8_t *Buf) override;
   bool hasRelocs() const { return !Relocs.empty(); }
   typename Base::Kind getKind() const override { return Base::Reloc; }
+  size_t getRelativeRelocCount() const { return NumRelativeRelocs; }
   static bool classof(const Base *B) { return B->getKind() == Base::Reloc; }
 
 private:
   bool Sort;
+  size_t NumRelativeRelocs = 0;
   std::vector<DynamicReloc<ELFT>> Relocs;
 };
 
 template <class ELFT>
 class OutputSection final : public OutputSectionBase<ELFT> {
   typedef OutputSectionBase<ELFT> Base;
 
 public:

lld/trunk/ELF/OutputSections.cpp

: OutputSectionBase<ELFT>(Name, Config->Rela ? SHT_RELA : SHT_REL,
                               SHF_ALLOC),
       Sort(Sort) {
   this->Header.sh_entsize = Config->Rela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
   this->Header.sh_addralign = sizeof(uintX_t);
 }
 
 template <class ELFT>
 void RelocationSection<ELFT>::addReloc(const DynamicReloc<ELFT> &Reloc) {
+  if (Reloc.Type == Target->RelativeRel)
+    ++NumRelativeRelocs;
   Relocs.push_back(Reloc);
 }
 
 template <class ELFT, class RelTy>
 static bool compRelocations(const RelTy &A, const RelTy &B) {
+  bool AIsRel = A.getType(Config->Mips64EL) == Target->RelativeRel;
+  bool BIsRel = B.getType(Config->Mips64EL) == Target->RelativeRel;
+  if (AIsRel != BIsRel)
+    return AIsRel;
+
   return A.getSymbol(Config->Mips64EL) < B.getSymbol(Config->Mips64EL);
 }
 
 template <class ELFT> void RelocationSection<ELFT>::writeTo(uint8_t *Buf) {
   uint8_t *BufBegin = Buf;
   for (const DynamicReloc<ELFT> &Rel : Relocs) {
     auto *P = reinterpret_cast<Elf_Rela *>(Buf);
     Buf += Config->Rela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
template <class ELFT> void DynamicSection<ELFT>::finalize() {
   Out<ELFT>::DynStrTab->finalize();
 
   if (Out<ELFT>::RelaDyn->hasRelocs()) {
     bool IsRela = Config->Rela;
     Add({IsRela ? DT_RELA : DT_REL, Out<ELFT>::RelaDyn});
     Add({IsRela ? DT_RELASZ : DT_RELSZ, Out<ELFT>::RelaDyn->getSize()});
     Add({IsRela ? DT_RELAENT : DT_RELENT,
          uintX_t(IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel))});
+
+    size_t NumRelativeRels = Out<ELFT>::RelaDyn->getRelativeRelocCount();
+    if (Config->ZCombreloc && NumRelativeRels)
+      Add({IsRela ? DT_RELACOUNT : DT_RELCOUNT, NumRelativeRels});
   }
   if (Out<ELFT>::RelaPlt && Out<ELFT>::RelaPlt->hasRelocs()) {
     Add({DT_JMPREL, Out<ELFT>::RelaPlt});
     Add({DT_PLTRELSZ, Out<ELFT>::RelaPlt->getSize()});
     Add({Config->EMachine == EM_MIPS ? DT_MIPS_PLTGOT : DT_PLTGOT,
          Out<ELFT>::GotPlt});
     Add({DT_PLTREL, uint64_t(Config->Rela ? DT_RELA : DT_REL)});
   }

lld/trunk/test/ELF/gotpc-relax-nopic.s

 # DISASM-NEXT:    11015:  48 81 fa 00 20 01 00  cmpq  $73728, %rdx
 # DISASM-NEXT:    1101c:  48 81 cf 00 20 01 00  orq   $73728, %rdi
 # DISASM-NEXT:    11023:  48 81 de 00 20 01 00  sbbq  $73728, %rsi
 # DISASM-NEXT:    1102a:  48 81 ed 00 20 01 00  subq  $73728, %rbp
 # DISASM-NEXT:    11031:  49 81 f0 00 20 01 00  xorq  $73728, %r8
 # DISASM-NEXT:    11038:  49 f7 c7 00 20 01 00  testq $73728, %r15
 
 # RUN: ld.lld -shared %t.o -o %t2
-# RUN: llvm-readobj -s %t2 | FileCheck --check-prefix=SEC-PIC    %s
+# RUN: llvm-readobj -s -r -d %t2 | FileCheck --check-prefix=SEC-PIC    %s
 # RUN: llvm-objdump -d %t2 | FileCheck --check-prefix=DISASM-PIC %s
 # SEC-PIC:      Section {
 # SEC-PIC:        Index:
 # SEC-PIC:        Name: .got
 # SEC-PIC-NEXT:   Type: SHT_PROGBITS
 # SEC-PIC-NEXT:   Flags [
 # SEC-PIC-NEXT:     SHF_ALLOC
 # SEC-PIC-NEXT:     SHF_WRITE
 # SEC-PIC-NEXT:   ]
-# SEC-PIC-NEXT:   Address: 0x2090
-# SEC-PIC-NEXT:   Offset: 0x2090
+# SEC-PIC-NEXT:   Address: 0x20A0
+# SEC-PIC-NEXT:   Offset: 0x20A0
 # SEC-PIC-NEXT:   Size: 8
 # SEC-PIC-NEXT:   Link:
 # SEC-PIC-NEXT:   Info:
 # SEC-PIC-NEXT:   AddressAlignment:
 # SEC-PIC-NEXT:   EntrySize:
 # SEC-PIC-NEXT: }
+# SEC-PIC:      Relocations [
+# SEC-PIC-NEXT:   Section ({{.*}}) .rela.dyn {
+# SEC-PIC-NEXT:     0x20A0 R_X86_64_RELATIVE - 0x3000
+# SEC-PIC-NEXT:   }
+# SEC-PIC-NEXT: ]
+# SEC-PIC:      0x000000006FFFFFF9 RELACOUNT            1 
 
 ## Check that there was no relaxation performed. All values refer to got entry.
-## Ex: 0x1000 + 4233 + 7 = 0x2090
-##     0x102a + 4191 + 7 = 0x2090
+## Ex: 0x1000 + 4249 + 7 = 0x20A0
+##     0x102a + 4207 + 7 = 0x20A0
 # DISASM-PIC:      Disassembly of section .text:
 # DISASM-PIC-NEXT: _start:
-# DISASM-PIC-NEXT:  1000: 48 13 05 89 10 00 00 adcq  4233(%rip), %rax
-# DISASM-PIC-NEXT:  1007: 48 03 1d 82 10 00 00 addq  4226(%rip), %rbx
-# DISASM-PIC-NEXT:  100e: 48 23 0d 7b 10 00 00 andq  4219(%rip), %rcx
-# DISASM-PIC-NEXT:  1015: 48 3b 15 74 10 00 00 cmpq  4212(%rip), %rdx
-# DISASM-PIC-NEXT:  101c: 48 0b 3d 6d 10 00 00 orq   4205(%rip), %rdi
-# DISASM-PIC-NEXT:  1023: 48 1b 35 66 10 00 00 sbbq  4198(%rip), %rsi
-# DISASM-PIC-NEXT:  102a: 48 2b 2d 5f 10 00 00 subq  4191(%rip), %rbp
-# DISASM-PIC-NEXT:  1031: 4c 33 05 58 10 00 00 xorq  4184(%rip), %r8
-# DISASM-PIC-NEXT:  1038: 4c 85 3d 51 10 00 00 testq 4177(%rip), %r15
+# DISASM-PIC-NEXT: 1000: 48 13 05 99 10 00 00  adcq  4249(%rip), %rax
+# DISASM-PIC-NEXT: 1007: 48 03 1d 92 10 00 00  addq  4242(%rip), %rbx
+# DISASM-PIC-NEXT: 100e: 48 23 0d 8b 10 00 00  andq  4235(%rip), %rcx
+# DISASM-PIC-NEXT: 1015: 48 3b 15 84 10 00 00  cmpq  4228(%rip), %rdx
+# DISASM-PIC-NEXT: 101c: 48 0b 3d 7d 10 00 00  orq 4221(%rip), %rdi
+# DISASM-PIC-NEXT: 1023: 48 1b 35 76 10 00 00  sbbq  4214(%rip), %rsi
+# DISASM-PIC-NEXT: 102a: 48 2b 2d 6f 10 00 00  subq  4207(%rip), %rbp
+# DISASM-PIC-NEXT: 1031: 4c 33 05 68 10 00 00  xorq  4200(%rip), %r8
+# DISASM-PIC-NEXT: 1038: 4c 85 3d 61 10 00 00  testq 4193(%rip), %r15
 
 .data
 .type   bar, @object
 bar:
  .byte   1
  .size   bar, .-bar
 
 .text

lld/trunk/test/ELF/i386-tls-ie-shared.s

 // RUN: llvm-mc -filetype=obj -triple=i686-pc-linux %s -o %t.o
 // RUN: llvm-mc -filetype=obj -triple=i686-pc-linux %p/Inputs/tls-opt-iele-i686-nopic.s -o %tso.o
 // RUN: ld.lld -shared %tso.o -o %tso
 // RUN: ld.lld -shared %t.o %tso -o %t1
-// RUN: llvm-readobj -s -r %t1 | FileCheck --check-prefix=GOTRELSHARED %s
+// RUN: llvm-readobj -s -r -d %t1 | FileCheck --check-prefix=GOTRELSHARED %s
 // RUN: llvm-objdump -d %t1 | FileCheck --check-prefix=DISASMSHARED %s
 
 // GOTRELSHARED:     Section {
 // GOTRELSHARED:      Index: 8
 // GOTRELSHARED:      Name: .got
 // GOTRELSHARED-NEXT:   Type: SHT_PROGBITS
 // GOTRELSHARED-NEXT:   Flags [
 // GOTRELSHARED-NEXT:     SHF_ALLOC
 // GOTRELSHARED-NEXT:     SHF_WRITE
 // GOTRELSHARED-NEXT:   ]
-// GOTRELSHARED-NEXT:   Address: 0x1050
-// GOTRELSHARED-NEXT:   Offset: 0x1050
+// GOTRELSHARED-NEXT:   Address: 0x1058
+// GOTRELSHARED-NEXT:   Offset: 0x1058
 // GOTRELSHARED-NEXT:   Size: 16
 // GOTRELSHARED-NEXT:   Link: 0
 // GOTRELSHARED-NEXT:   Info: 0
 // GOTRELSHARED-NEXT:   AddressAlignment: 4
 // GOTRELSHARED-NEXT:   EntrySize: 0
 // GOTRELSHARED-NEXT: }
 // GOTRELSHARED:      Relocations [
 // GOTRELSHARED-NEXT:   Section ({{.*}}) .rel.dyn {
 // GOTRELSHARED-NEXT:     0x2002 R_386_RELATIVE - 0x0
 // GOTRELSHARED-NEXT:     0x200A R_386_RELATIVE - 0x0
 // GOTRELSHARED-NEXT:     0x2013 R_386_RELATIVE - 0x0
 // GOTRELSHARED-NEXT:     0x201C R_386_RELATIVE - 0x0
 // GOTRELSHARED-NEXT:     0x2024 R_386_RELATIVE - 0x0
 // GOTRELSHARED-NEXT:     0x202D R_386_RELATIVE - 0x0
 // GOTRELSHARED-NEXT:     0x2036 R_386_RELATIVE - 0x0
 // GOTRELSHARED-NEXT:     0x203F R_386_RELATIVE - 0x0
-// GOTRELSHARED-NEXT:     0x1050 R_386_TLS_TPOFF tlslocal0 0x0
-// GOTRELSHARED-NEXT:     0x1054 R_386_TLS_TPOFF tlslocal1 0x0
-// GOTRELSHARED-NEXT:     0x1058 R_386_TLS_TPOFF tlsshared0 0x0
-// GOTRELSHARED-NEXT:     0x105C R_386_TLS_TPOFF tlsshared1 0x0
+// GOTRELSHARED-NEXT:     0x1058 R_386_TLS_TPOFF tlslocal0 0x0
+// GOTRELSHARED-NEXT:     0x105C R_386_TLS_TPOFF tlslocal1 0x0
+// GOTRELSHARED-NEXT:     0x1060 R_386_TLS_TPOFF tlsshared0 0x0
+// GOTRELSHARED-NEXT:     0x1064 R_386_TLS_TPOFF tlsshared1 0x0
 // GOTRELSHARED-NEXT:   }
 // GOTRELSHARED-NEXT: ]
+// GOTRELSHARED:      0x6FFFFFFA RELCOUNT             8
 
 // DISASMSHARED:       Disassembly of section test:
 // DISASMSHARED-NEXT:  _start:
 // (.got)[0] = 0x2050 = 8272
 // (.got)[1] = 0x2054 = 8276
 // (.got)[2] = 0x2058 = 8280
 // (.got)[3] = 0x205C = 8284
-// DISASMSHARED-NEXT:  2000:    8b 0d 50 10 00 00       movl    4176, %ecx
-// DISASMSHARED-NEXT:  2006:    65 8b 01        movl    %gs:(%ecx), %eax
-// DISASMSHARED-NEXT:  2009:    a1 50 10 00 00  movl    4176, %eax
-// DISASMSHARED-NEXT:  200e:    65 8b 00        movl    %gs:(%eax), %eax
-// DISASMSHARED-NEXT:  2011:    03 0d 50 10 00 00       addl    4176, %ecx
-// DISASMSHARED-NEXT:  2017:    65 8b 01        movl    %gs:(%ecx), %eax
-// DISASMSHARED-NEXT:  201a:    8b 0d 54 10 00 00       movl    4180, %ecx
-// DISASMSHARED-NEXT:  2020:    65 8b 01        movl    %gs:(%ecx), %eax
-// DISASMSHARED-NEXT:  2023:    a1 54 10 00 00  movl    4180, %eax
-// DISASMSHARED-NEXT:  2028:    65 8b 00        movl    %gs:(%eax), %eax
-// DISASMSHARED-NEXT:  202b:    03 0d 54 10 00 00       addl    4180, %ecx
-// DISASMSHARED-NEXT:  2031:    65 8b 01        movl    %gs:(%ecx), %eax
-// DISASMSHARED-NEXT:  2034:    8b 0d 58 10 00 00       movl    4184, %ecx
-// DISASMSHARED-NEXT:  203a:    65 8b 01        movl    %gs:(%ecx), %eax
-// DISASMSHARED-NEXT:  203d:    03 0d 5c 10 00 00       addl    4188, %ecx
-// DISASMSHARED-NEXT:  2043:    65 8b 01        movl    %gs:(%ecx), %eax
+// DISASMSHARED-NEXT:  2000: 8b 0d 58 10 00 00   movl  4184, %ecx
+// DISASMSHARED-NEXT:  2006: 65 8b 01  movl  %gs:(%ecx), %eax
+// DISASMSHARED-NEXT:  2009: a1 58 10 00 00  movl  4184, %eax
+// DISASMSHARED-NEXT:  200e: 65 8b 00  movl  %gs:(%eax), %eax
+// DISASMSHARED-NEXT:  2011: 03 0d 58 10 00 00   addl  4184, %ecx
+// DISASMSHARED-NEXT:  2017: 65 8b 01  movl  %gs:(%ecx), %eax
+// DISASMSHARED-NEXT:  201a: 8b 0d 5c 10 00 00   movl  4188, %ecx
+// DISASMSHARED-NEXT:  2020: 65 8b 01  movl  %gs:(%ecx), %eax
+// DISASMSHARED-NEXT:  2023: a1 5c 10 00 00  movl  4188, %eax
+// DISASMSHARED-NEXT:  2028: 65 8b 00  movl  %gs:(%eax), %eax
+// DISASMSHARED-NEXT:  202b: 03 0d 5c 10 00 00   addl  4188, %ecx
+// DISASMSHARED-NEXT:  2031: 65 8b 01  movl  %gs:(%ecx), %eax
+// DISASMSHARED-NEXT:  2034: 8b 0d 60 10 00 00   movl  4192, %ecx
+// DISASMSHARED-NEXT:  203a: 65 8b 01  movl  %gs:(%ecx), %eax
+// DISASMSHARED-NEXT:  203d: 03 0d 64 10 00 00   addl  4196, %ecx
+// DISASMSHARED-NEXT:  2043: 65 8b 01  movl  %gs:(%ecx), %eax
 
 .type tlslocal0,@object
 .section .tbss,"awT",@nobits
 .globl tlslocal0
 .align 4
 tlslocal0:
  .long 0
  .size tlslocal0, 4

lld/trunk/test/ELF/local-got-pie.s

 // RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 // RUN: ld.lld %t.o -o %t -pie
-// RUN: llvm-readobj -s -r %t | FileCheck %s
+// RUN: llvm-readobj -s -r -d %t | FileCheck %s
 // RUN: llvm-objdump -d %t | FileCheck --check-prefix=DISASM %s
 
 .globl _start
 _start:
  call foo@gotpcrel
 
  .hidden foo
  .global foo
 foo:
  nop
 
-// 0x20A0 - 1001 - 5 = 4250
+// 0x20B0 - 1001 - 5 = 4266
 // DISASM:      Disassembly of section .text:
 // DISASM-NEXT: _start:
-// DISASM-NEXT:   1000: {{.*}} callq 4251
+// DISASM-NEXT:   1000: {{.*}} callq 4267
 // DISASM:      foo:
 // DISASM-NEXT:   1005: {{.*}} nop
 
 // CHECK:      Name: .got
 // CHECK-NEXT: Type: SHT_PROGBITS
 // CHECK-NEXT: Flags [
 // CHECK-NEXT:   SHF_ALLOC
 // CHECK-NEXT:   SHF_WRITE
 // CHECK-NEXT: ]
-// CHECK-NEXT: Address: 0x20A0
+// CHECK-NEXT: Address: 0x20B0
 // CHECK-NEXT: Offset:
 // CHECK-NEXT: Size: 8
 
 // CHECK:      Relocations [
 // CHECK-NEXT:   Section ({{.*}}) .rela.dyn {
-// CHECK-NEXT:     0x20A0 R_X86_64_RELATIVE - 0x1005
+// CHECK-NEXT:     0x20B0 R_X86_64_RELATIVE - 0x1005
 // CHECK-NEXT:   }
 // CHECK-NEXT: ]
+// CHECK:      0x000000006FFFFFF9 RELACOUNT            1

lld/trunk/test/ELF/local-got-shared.s

 // RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 // RUN: ld.lld %t.o -o %t -shared
-// RUN: llvm-readobj -s -r %t | FileCheck %s
+// RUN: llvm-readobj -s -r -d %t | FileCheck %s
 // RUN: llvm-objdump -d %t | FileCheck --check-prefix=DISASM %s
 
 bar:
   call foo@gotpcrel
 
         .hidden foo
         .global foo
 foo:
         nop
 
-// 0x2090 - 0x1000 - 5 = 4235
+// 0x20A0 - 0x1000 - 5 = 4251
 // DISASM:      bar:
-// DISASM-NEXT:   1000: {{.*}} callq 4235
+// DISASM-NEXT:   1000: {{.*}} callq 4251
 
 // DISASM:      foo:
 // DISASM-NEXT:   1005: {{.*}} nop
 
 // CHECK:      Name: .got
 // CHECK-NEXT: Type: SHT_PROGBITS
 // CHECK-NEXT: Flags [
 // CHECK-NEXT:   SHF_ALLOC
 // CHECK-NEXT:   SHF_WRITE
 // CHECK-NEXT: ]
-// CHECK-NEXT: Address: 0x2090
+// CHECK-NEXT: Address: 0x20A0
 // CHECK-NEXT: Offset:
 // CHECK-NEXT: Size: 8
 
 // CHECK:      Relocations [
 // CHECK-NEXT:   Section ({{.*}}) .rela.dyn {
-// CHECK-NEXT:     0x2090 R_X86_64_RELATIVE - 0x1005
+// CHECK-NEXT:     0x20A0 R_X86_64_RELATIVE - 0x1005
 // CHECK-NEXT:   }
 // CHECK-NEXT: ]
+// CHECK:      0x000000006FFFFFF9 RELACOUNT            1