[PATCH] D42790: [ELF] Ensure that Elf_Rela addends are always written with -r

Simon Atanasyan via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 1 08:33:17 PST 2018


On Thu, Feb 1, 2018 at 6:52 PM, George Rimar via Phabricator
<reviews at reviews.llvm.org> wrote:
> grimar added a comment.
>
> In https://reviews.llvm.org/D42790#994846, @atanasyan wrote:
>
>> In https://reviews.llvm.org/D42790#994808, @grimar wrote:
>>
>> > In https://reviews.llvm.org/D42790#994774, @arichardson wrote:
>> >
>> > > I originally discovered this issue because in our CHERI fork of LLD we set `Config->IsRela = false` when linking  MIPS64 code. We do this because the FreeBSD MIPS rtld only understands Elf_Rel relocations and does not process Elf_Rela.
>> >
>> >
>> > It sounds like FreeBSD MIPS rtld bug honestly and probably should be fixed on their side.
>>
>>
>> MIPS O32 ABI uses the Elf_Rel format in object files, while N32 and N64 ABIs uses the Elf_Rela. But originally in case of all ABIs output relocations in executable files and DSO are written in the Elf_Rel format. I think the goal was to reduce a file size. In case of embedded systems with small amount of memory it can be a benefit. Maybe there are some other reasons. When I implement MIPS LLD backend I used  the same format for input and output files, i.e. Elf_Rel for O32 ABI and Elf_Rela for N32 and N64 ABIs. It works for Linux because the dynamic linker accepts all formats. It looks like FreeBSD linker is more stricter.
>
>
> So should/can we switch LLD to always emit Elf_Rel for all MIPS outputs then ?

I think so. The attached patch solves the problem. Addition of the new
command line args is a separate question.

-- 
Simon Atanasyan
-------------- next part --------------
diff --git a/ELF/Driver.cpp b/ELF/Driver.cpp
index 3de270dd3..6db6eccb7 100644
--- a/ELF/Driver.cpp
+++ b/ELF/Driver.cpp
@@ -808,7 +808,7 @@ static void setConfigs() {
       Config->IsLE ? support::endianness::little : support::endianness::big;
   Config->IsMips64EL = (Kind == ELF64LEKind && Machine == EM_MIPS);
   Config->IsRela =
-      Config->Is64 || IsX32 || Config->MipsN32Abi || Machine == EM_PPC;
+      (Config->Is64 || IsX32 || Machine == EM_PPC) && Machine != EM_MIPS;
   Config->Pic = Config->Pie || Config->Shared;
   Config->Wordsize = Config->Is64 ? 8 : 4;
 }
diff --git a/ELF/InputSection.cpp b/ELF/InputSection.cpp
index b2577abb8..7c9a4ace9 100644
--- a/ELF/InputSection.cpp
+++ b/ELF/InputSection.cpp
@@ -339,7 +339,7 @@ void InputSection::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
     auto *P = reinterpret_cast<typename ELFT::Rela *>(Buf);
     Buf += sizeof(RelTy);
 
-    if (Config->IsRela)
+    if (RelTy::IsRela)
       P->r_addend = getAddend<ELFT>(Rel);
 
     // Output section VA is zero for -r, so r_offset is an offset within the
@@ -369,7 +369,7 @@ void InputSection::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
         continue;
       }
 
-      if (Config->IsRela) {
+      if (RelTy::IsRela) {
         P->r_addend =
             Sym.getVA(getAddend<ELFT>(Rel)) - Section->getOutputSection()->Addr;
       } else if (Config->Relocatable) {
diff --git a/test/ELF/mips-64.s b/test/ELF/mips-64.s
index dd8a58d60..501988ddb 100644
--- a/test/ELF/mips-64.s
+++ b/test/ELF/mips-64.s
@@ -30,17 +30,16 @@ v2:
 # SYM: 00020008 g       .data           00000008 v2
 
 # CHECK:      Relocations [
-# CHECK-NEXT:   Section (7) .rela.dyn {
-# CHECK-NEXT:     0x20010 R_MIPS_REL32/R_MIPS_64/R_MIPS_NONE - 0x20000
-#                                                             ^-- v1
-# CHECK-NEXT:     0x20008 R_MIPS_REL32/R_MIPS_64/R_MIPS_NONE v2 0x8
+# CHECK-NEXT:   Section (7) .rel.dyn {
+# CHECK-NEXT:     0x20010 R_MIPS_REL32/R_MIPS_64/R_MIPS_NONE - 0x0
+# CHECK-NEXT:     0x20008 R_MIPS_REL32/R_MIPS_64/R_MIPS_NONE v2 0x0
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 
 # CHECK: DynamicSection [
 # CHECK:   Tag        Type     Name/Value
-# CHECK:   0x0000000000000008 RELASZ    48 (bytes)
-# CHECK:   0x0000000000000009 RELAENT   24 (bytes)
+# CHECK:   0x0000000000000012 RELSZ    32 (bytes)
+# CHECK:   0x0000000000000013 RELENT   16 (bytes)
 
 # CHECK:      Primary GOT {
 # CHECK-NEXT:   Canonical gp value:
diff --git a/test/ELF/mips-tls-64.s b/test/ELF/mips-tls-64.s
index 06785ae23..7d191f963 100644
--- a/test/ELF/mips-tls-64.s
+++ b/test/ELF/mips-tls-64.s
@@ -34,7 +34,7 @@
 # DIS: 0000000000000000 g       *UND*           00000000 foo
 
 # CHECK:      Relocations [
-# CHECK-NEXT:   Section (7) .rela.dyn {
+# CHECK-NEXT:   Section (7) .rel.dyn {
 # CHECK-NEXT:     0x30020 R_MIPS_TLS_TPREL64/R_MIPS_NONE/R_MIPS_NONE foo 0x0
 # CHECK-NEXT:     0x30030 R_MIPS_TLS_DTPMOD64/R_MIPS_NONE/R_MIPS_NONE foo 0x0
 # CHECK-NEXT:     0x30038 R_MIPS_TLS_DTPREL64/R_MIPS_NONE/R_MIPS_NONE foo 0x0
@@ -66,7 +66,7 @@
 # DIS-SO-NEXT:  20040 00000000 00000000 00000000 00000000
 
 # SO:      Relocations [
-# SO-NEXT:   Section (7) .rela.dyn {
+# SO-NEXT:   Section (7) .rel.dyn {
 # SO-NEXT:     0x20030 R_MIPS_TLS_DTPMOD64/R_MIPS_NONE/R_MIPS_NONE - 0x0
 # SO-NEXT:     0x20010 R_MIPS_TLS_TPREL64/R_MIPS_NONE/R_MIPS_NONE foo 0x0
 # SO-NEXT:     0x20020 R_MIPS_TLS_DTPMOD64/R_MIPS_NONE/R_MIPS_NONE foo 0x0
diff --git a/test/ELF/mips64-eh-abs-reloc.s b/test/ELF/mips64-eh-abs-reloc.s
index 7bc500137..aa21a3769 100644
--- a/test/ELF/mips64-eh-abs-reloc.s
+++ b/test/ELF/mips64-eh-abs-reloc.s
@@ -20,8 +20,8 @@
 # OBJ-NEXT:  }
 
 # PIC-RELOCS: Relocations [
-# PIC-RELOCS-NEXT:  Section (7) .rela.dyn {
-# PIC-RELOCS-NEXT:    {{0x.+}} R_MIPS_REL32/R_MIPS_64/R_MIPS_NONE - 0x10000
+# PIC-RELOCS-NEXT:  Section (7) .rel.dyn {
+# PIC-RELOCS-NEXT:    {{0x.+}} R_MIPS_REL32/R_MIPS_64/R_MIPS_NONE - 0x0
 # PIC-RELOCS-NEXT:  }
 # PIC-RELOCS-NEXT:]
 


More information about the llvm-commits mailing list