[PATCH] D111181: llvm-objcopy][ELF] Don't assume RELA sections are dynamic if they carry the SHF_ALLOC flag

[Ard Biesheuvel via Phabricator via llvm-commits]

ardb created this revision.
ardb added reviewers: jakehehrlich, nickdesaulniers.
Herald added subscribers: abrachet, emaste.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
ardb requested review of this revision.
Herald added subscribers: llvm-commits, MaskRay.
Herald added a project: LLVM.

Currently, llvm-objcopy assumes that any SHT_RELA type section with the SHF_ALLOC attribute is a dynamic relocation section, and therefore refers to symbol and string tables of the dynamic variety, which deviates from the ordinary static ones.

However, the ELF spec permits the use of SHF_ALLOC for ordinary SHT_RELA type sections as well, and so this heuristic is not entirely accurate, and may produce incorrect results, resulting in cryptic error messages such as

llvm-strip: error: Link field value 48 in section .rela.text is not a symbol table

where the file in question does in fact have a SYMTAB section at #48.

So let's replace this with a check against the ELF file type, which is more appropriate: partially linked binaries of type ET_REL cannot have dynamic relocation sections, and ET_EXEC/ET_DYN fully linked cannot have the static ones. Note that this requires that reading the section headers is deferred until after we figure out the file type, which also means that finding the executable header offset


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111181

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


Index: llvm/tools/llvm-objcopy/ELF/Object.cpp
===================================================================
--- llvm/tools/llvm-objcopy/ELF/Object.cpp
+++ llvm/tools/llvm-objcopy/ELF/Object.cpp
@@ -1415,12 +1415,26 @@
 }
 
 template <class ELFT> Error ELFBuilder<ELFT>::findEhdrOffset() {
+  uint32_t Index = 0;
   if (!ExtractPartition)
     return Error::success();
 
-  for (const SectionBase &Sec : Obj.sections()) {
-    if (Sec.Type == SHT_LLVM_PART_EHDR && Sec.Name == *ExtractPartition) {
-      EhdrOffset = Sec.Offset;
+  Expected<typename ELFFile<ELFT>::Elf_Shdr_Range> Sections =
+      ElfFile.sections();
+  if (!Sections)
+    return Sections.takeError();
+
+  for (const typename ELFFile<ELFT>::Elf_Shdr &Shdr : *Sections) {
+    if (Index == 0) {
+      ++Index;
+      continue;
+    }
+    Expected<StringRef> SecName = ElfFile.getSectionName(Shdr);
+    if (!SecName)
+      return SecName.takeError();
+    if (Shdr.sh_type == SHT_LLVM_PART_EHDR &&
+        SecName->str() == *ExtractPartition) {
+      EhdrOffset = Shdr.sh_offset;
       return Error::success();
     }
   }
@@ -1692,7 +1706,7 @@
   switch (Shdr.sh_type) {
   case SHT_REL:
   case SHT_RELA:
-    if (Shdr.sh_flags & SHF_ALLOC) {
+    if (Obj.Type != ELF::ET_REL) {
       if (Expected<ArrayRef<uint8_t>> Data = ElfFile.getSectionContents(Shdr))
         return Obj.addSection<DynamicRelocationSection>(*Data);
       else
@@ -1900,8 +1914,6 @@
 }
 
 template <class ELFT> Error ELFBuilder<ELFT>::build(bool EnsureSymtab) {
-  if (Error E = readSectionHeaders())
-    return E;
   if (Error E = findEhdrOffset())
     return E;
 
@@ -1922,6 +1934,8 @@
   Obj.Entry = Ehdr.e_entry;
   Obj.Flags = Ehdr.e_flags;
 
+  if (Error E = readSectionHeaders())
+    return E;
   if (Error E = readSections(EnsureSymtab))
     return E;
   return readProgramHeaders(*HeadersFile);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111181.377339.patch
Type: text/x-patch
Size: 1866 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211005/244501da/attachment.bin>