[llvm-objcopy] Fix handling of MIPS64 little endian files

[atanasyan]

Summary

MIPS64 little endian target has a "special" encoding of r_info relocation record field. Instead of one 64-bit little endian number, it is a little endian 32-bit number followed by a 32-bit big endian number. For correct reading and writing such fields we must provide information about target machine into the corresponding routine. This patch does this for the llvm-objcopy tool and fix handling of MIPS64 little endian files.

The patch looks ugly but I could not figure out how to hide the nature of MIPS64 ABI better.

The bug was reported in the issue #52647.

[jhenderson]

Since the "Mips64EL" property is a propert of the input object, I wonder if we should just make it a member of the object, and then put a reference to that object in the relocation sections, so that we can query that, rather than needing to a) pass around the variable, and b) have to have the property in the relocation section? What do you think?

llvm/test/tools/llvm-objcopy/ELF/mips64el.test

5–7 ↗

(On Diff #393879)

I'd name the input and output objects differently for the two cases, so that we don't overwrite the first case with the second case. This is sometimes useful for test debugging.

11–14 ↗

(On Diff #393879)

We usually get rid of excessive spacing within a block, as per the suggested edit. Same below.

18–19 ↗

(On Diff #393879)

I'd be surprised if you need either of these fields.

20 ↗

(On Diff #393879)

I'd replace this with a Size field, if the size is important to the test case. If it isn't important, consider just getting rid of the field entirely.

24–25 ↗

(On Diff #393879)

These fields are redundant (either they're the default, or simply not needed).

37–42 ↗

(On Diff #393879)

Do you need this second symbol? It doesn't seem to be being used.

llvm/tools/llvm-objcopy/ELF/Object.h

800–801

[atanasyan]

Since the "Mips64EL" property is a property of the input object, I wonder if we should just make it a member of the object,
and then put a reference to that object in the relocation sections, so that we can query that, rather than needing
to a) pass around the variable, and b) have to have the property in the relocation section? What do you think?

Agreed.

[atanasyan]

• Reduce redundancy of the test case
• Add IsMips64 field to the Object class.
• Save reference to an Object in the RelocationSection class.

[nathanchance]

This resolves the Linux kernel build error, thank you for the quick fix!

[MaskRay]

LG

llvm/test/tools/llvm-objcopy/ELF/mips64.test

1

Add a comment like ## mips64el has a special encoding of the r_info relocation field. Test that we support both endianness.

[jhenderson]

Looks good, with @MaskRay's comment.

llvm/test/tools/llvm-objcopy/ELF/mips64.test

1

I assume that yaml2obj does actually generate the r_info correctly in this case? Easiest way to tell is to make sure the test fails without this fix, if you're not sure.

[atanasyan]

Thanks for review.

llvm/test/tools/llvm-objcopy/ELF/mips64.test

+# RUN: yaml2obj %s -DENDIANNESS=ELFDATA2LSB -o %t1-le
+# RUN: llvm-objcopy %t1-le %t2-le
+# RUN: llvm-readobj --relocations %t2-le | FileCheck %s
+
+# RUN: yaml2obj %s -DENDIANNESS=ELFDATA2MSB -o %t1-be
+# RUN: llvm-objcopy %t1-be %t2-be
+# RUN: llvm-readobj --relocations %t2-be | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    [[ENDIANNESS]]
+  Type:    ET_REL
+  Machine: EM_MIPS
+Sections:
+  - Name: .text
+    Type: SHT_PROGBITS
+  - Name: .rela.text
+    Type: SHT_RELA
+    Relocations:
+      - Offset: 8
+        Symbol: foo
+        Type:   R_MIPS_JALR
+Symbols:
+  - Name:    foo
+    Type:    STT_FUNC
+    Section: .text
+
+# CHECK:      Relocations [
+# CHECK-NEXT:   Section (2) .rela.text {
+# CHECK-NEXT:     0x8 R_MIPS_JALR/R_MIPS_NONE/R_MIPS_NONE foo 0x0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]

llvm/tools/llvm-objcopy/ELF/Object.h

public:
   void finalize() override;
 };
 
 class RelocationSection
     : public RelocSectionWithSymtabBase<SymbolTableSection> {
   MAKE_SEC_WRITER_FRIEND
 
   std::vector<Relocation> Relocations;
+  const Object &Obj;
 
 public:
+  RelocationSection(const Object &O) : Obj(O) {}
   void addRelocation(Relocation Rel) { Relocations.push_back(Rel); }
   Error accept(SectionVisitor &Visitor) const override;
   Error accept(MutableSectionVisitor &Visitor) override;
   Error removeSectionReferences(
       bool AllowBrokenLinks,
       function_ref<bool(const SectionBase *)> ToRemove) override;
   Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
   void markSymbols() override;
   void replaceSectionReferences(
       const DenseMap<SectionBase *, SectionBase *> &FromTo) override;
+  const Object &getObject() const { return Obj; }
 
   static bool classof(const SectionBase *S) {
     if (S->OriginalFlags & ELF::SHF_ALLOC)
       return false;
     return S->OriginalType == ELF::SHT_REL || S->OriginalType == ELF::SHT_RELA;
   }
 };
 
 // TODO: The way stripping and groups interact is complicated
private:
   Error initSymbolTable(SymbolTableSection *SymTab);
   Error readSectionHeaders();
   Error readSections(bool EnsureSymtab);
   Error findEhdrOffset();
   Expected<SectionBase &> makeSection(const Elf_Shdr &Shdr);
 
 public:
   ELFBuilder(const ELFObjectFile<ELFT> &ElfObj, Object &Obj,
-             Optional<StringRef> ExtractPartition)
-      : ElfFile(ElfObj.getELFFile()), Obj(Obj),
-        ExtractPartition(ExtractPartition) {}
+             Optional<StringRef> ExtractPartition);
 
   Error build(bool EnsureSymtab);
 };
 
 class BinaryReader : public Reader {
   MemoryBuffer *MemBuf;
   uint8_t NewSymbolVisibility;
 
public:
   uint32_t Flags;
 
   bool HadShdrs = true;
   bool MustBeRelocatable = false;
   StringTableSection *SectionNames = nullptr;
   SymbolTableSection *SymbolTable = nullptr;
   SectionIndexSection *SectionIndexTable = nullptr;
 
+  bool IsMips64EL = false;
+
   SectionTableRef sections() const { return SectionTableRef(Sections); }
   iterator_range<
       filter_iterator<pointee_iterator<std::vector<SecPtr>::const_iterator>,
                       decltype(&sectionIsAlloc)>>
   allocSections() const {
     return make_filter_range(make_pointee_range(Sections), sectionIsAlloc);
   }
 

llvm/tools/llvm-objcopy/ELF/Object.cpp

 static void setAddend(Elf_Rel_Impl<ELFT, false> &, uint64_t) {}
 
 template <class ELFT>
 static void setAddend(Elf_Rel_Impl<ELFT, true> &Rela, uint64_t Addend) {
   Rela.r_addend = Addend;
 }
 
 template <class RelRange, class T>
-static void writeRel(const RelRange &Relocations, T *Buf) {
+static void writeRel(const RelRange &Relocations, T *Buf, bool IsMips64EL) {
   for (const auto &Reloc : Relocations) {
     Buf->r_offset = Reloc.Offset;
     setAddend(*Buf, Reloc.Addend);
     Buf->setSymbolAndType(Reloc.RelocSymbol ? Reloc.RelocSymbol->Index : 0,
-                          Reloc.Type, false);
+                          Reloc.Type, IsMips64EL);
     ++Buf;
   }
 }
 
 template <class ELFT>
 Error ELFSectionWriter<ELFT>::visit(const RelocationSection &Sec) {
   uint8_t *Buf = reinterpret_cast<uint8_t *>(Out.getBufferStart()) + Sec.Offset;
   if (Sec.Type == SHT_REL)
-    writeRel(Sec.Relocations, reinterpret_cast<Elf_Rel *>(Buf));
+    writeRel(Sec.Relocations, reinterpret_cast<Elf_Rel *>(Buf),
+             Sec.getObject().IsMips64EL);
   else
-    writeRel(Sec.Relocations, reinterpret_cast<Elf_Rela *>(Buf));
+    writeRel(Sec.Relocations, reinterpret_cast<Elf_Rela *>(Buf),
+             Sec.getObject().IsMips64EL);
   return Error::success();
 }
 
 Error RelocationSection::accept(SectionVisitor &Visitor) const {
   return Visitor.visit(*this);
 }
 
 Error RelocationSection::accept(MutableSectionVisitor &Visitor) {
Expected<std::unique_ptr<Object>> IHexELFBuilder::build() {
   addSymTab(StrTab);
   if (Error Err = initSections())
     return std::move(Err);
   addDataSections();
 
   return std::move(Obj);
 }
 
+template <class ELFT>
+ELFBuilder<ELFT>::ELFBuilder(const ELFObjectFile<ELFT> &ElfObj, Object &Obj,
+                             Optional<StringRef> ExtractPartition)
+    : ElfFile(ElfObj.getELFFile()), Obj(Obj),
+      ExtractPartition(ExtractPartition) {
+  Obj.IsMips64EL = ElfFile.isMips64EL();
+}
+
 template <class ELFT> void ELFBuilder<ELFT>::setParentSegment(Segment &Child) {
   for (Segment &Parent : Obj.segments()) {
     // Every segment will overlap with itself but we don't want a segment to
     // be its own parent so we avoid that situation.
     if (&Child != &Parent && segmentOverlapsSegment(Child, Parent)) {
       // We want a canonical "most parental" segment but this requires
       // inspecting the ParentSegment.
       if (compareSegmentsByOffset(&Parent, &Child))

 static void getAddend(uint64_t &, const Elf_Rel_Impl<ELFT, false> &) {}
 
 template <class ELFT>
 static void getAddend(uint64_t &ToSet, const Elf_Rel_Impl<ELFT, true> &Rela) {
   ToSet = Rela.r_addend;
 }
 
 template <class T>
-static Error initRelocations(RelocationSection *Relocs,
-                             SymbolTableSection *SymbolTable, T RelRange) {
+static Error initRelocations(RelocationSection *Relocs, T RelRange) {
   for (const auto &Rel : RelRange) {
     Relocation ToAdd;
     ToAdd.Offset = Rel.r_offset;
     getAddend(ToAdd.Addend, Rel);
-    ToAdd.Type = Rel.getType(false);
+    ToAdd.Type = Rel.getType(Relocs->getObject().IsMips64EL);
 
-    if (uint32_t Sym = Rel.getSymbol(false)) {
-      if (!SymbolTable)
+    if (uint32_t Sym = Rel.getSymbol(Relocs->getObject().IsMips64EL)) {
+      if (!Relocs->getObject().SymbolTable)
         return createStringError(
             errc::invalid_argument,
             "'" + Relocs->Name + "': relocation references symbol with index " +
                 Twine(Sym) + ", but there is no symbol table");
-      Expected<Symbol *> SymByIndex = SymbolTable->getSymbolByIndex(Sym);
+      Expected<Symbol *> SymByIndex =
+          Relocs->getObject().SymbolTable->getSymbolByIndex(Sym);
       if (!SymByIndex)
         return SymByIndex.takeError();
 
       ToAdd.RelocSymbol = *SymByIndex;
     }
 
     Relocs->addRelocation(ToAdd);
   }
Expected<SectionBase &> ELFBuilder<ELFT>::makeSection(const Elf_Shdr &Shdr) {
   case SHT_REL:
   case SHT_RELA:
     if (Shdr.sh_flags & SHF_ALLOC) {
       if (Expected<ArrayRef<uint8_t>> Data = ElfFile.getSectionContents(Shdr))
         return Obj.addSection<DynamicRelocationSection>(*Data);
       else
         return Data.takeError();
     }
-    return Obj.addSection<RelocationSection>();
+    return Obj.addSection<RelocationSection>(Obj);
   case SHT_STRTAB:
     // If a string table is allocated we don't want to mess with it. That would
     // mean altering the memory image. There are no special link types or
     // anything so we can just use a Section.
     if (Shdr.sh_flags & SHF_ALLOC) {
       if (Expected<ArrayRef<uint8_t>> Data = ElfFile.getSectionContents(Shdr))
         return Obj.addSection<Section>(*Data);
       else
if (auto RelSec = dyn_cast<RelocationSection>(&Sec)) {
       const typename ELFFile<ELFT>::Elf_Shdr *Shdr =
           Sections->begin() + RelSec->Index;
       if (RelSec->Type == SHT_REL) {
         Expected<typename ELFFile<ELFT>::Elf_Rel_Range> Rels =
             ElfFile.rels(*Shdr);
         if (!Rels)
           return Rels.takeError();
 
-        if (Error Err = initRelocations(RelSec, Obj.SymbolTable, *Rels))
+        if (Error Err = initRelocations(RelSec, *Rels))
           return Err;
       } else {
         Expected<typename ELFFile<ELFT>::Elf_Rela_Range> Relas =
             ElfFile.relas(*Shdr);
         if (!Relas)
           return Relas.takeError();
 
-        if (Error Err = initRelocations(RelSec, Obj.SymbolTable, *Relas))
+        if (Error Err = initRelocations(RelSec, *Relas))
           return Err;
       }
     } else if (auto GroupSec = dyn_cast<GroupSection>(&Sec)) {
       if (Error Err = initGroupSection(GroupSec))
         return Err;
     }
   }