[PATCH] D38329: [llvm-objcopy] Fix latent bug that allowed some Sections to be improperly cast to StringTableSections

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich created this revision.

If a Section had Type SHT_STRTAB (which could happen if you had a .dynstr section) it was possible to cast Section to StringTableSection and get away with any operation that was supported by SectionBase without it being noticed. This change makes this bug easier to notice and fixes it where it occurred. It also made me realize that there was some duplication of efforts in the loop that calls ::initialize. These issues are all fixed by this change.


Repository:
  rL LLVM

https://reviews.llvm.org/D38329

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


Index: tools/llvm-objcopy/Object.h
===================================================================
--- tools/llvm-objcopy/Object.h
+++ tools/llvm-objcopy/Object.h
@@ -130,6 +130,8 @@
   void finalize() override;
   void writeSection(llvm::FileOutputBuffer &Out) const override;
   static bool classof(const SectionBase *S) {
+    if (S->Flags & llvm::ELF::SHF_ALLOC)
+      return false;
     return S->Type == llvm::ELF::SHT_STRTAB;
   }
 };
@@ -246,11 +248,11 @@
 
 class SectionWithStrTab : public Section {
 private:
-  StringTableSection *StrTab = nullptr;
+  const SectionBase *StrTab = nullptr;
 
 public:
   SectionWithStrTab(llvm::ArrayRef<uint8_t> Data) : Section(Data) {}
-  void setStrTab(StringTableSection *StringTable) { StrTab = StringTable; }
+  void setStrTab(const SectionBase *StringTable) { StrTab = StringTable; }
   void removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
Index: tools/llvm-objcopy/Object.cpp
===================================================================
--- tools/llvm-objcopy/Object.cpp
+++ tools/llvm-objcopy/Object.cpp
@@ -301,11 +301,9 @@
 }
 
 void SectionWithStrTab::initialize(SectionTableRef SecTable) {
-  setStrTab(SecTable.getSectionOfType<StringTableSection>(
-      Link,
-      "Link field value " + Twine(Link) + " in section " + Name + " is invalid",
-      "Link field value " + Twine(Link) + " in section " + Name +
-          " is not a string table"));
+  setStrTab(SecTable.getSection(Link,
+                                "Link field value " + Twine(Link) +
+                                    " in section " + Name + " is invalid"));
 }
 
 void SectionWithStrTab::finalize() { this->Link = StrTab->Index; }
@@ -547,15 +545,6 @@
         initRelocations(RelSec, SymbolTable,
                         unwrapOrError(ElfFile.relas(Shdr)));
     }
-
-    if (auto Sec = dyn_cast<SectionWithStrTab>(Section.get())) {
-      Sec->setStrTab(SecTable.getSectionOfType<StringTableSection>(
-          Sec->Link,
-          "Link field value " + Twine(Sec->Link) + " in section " + Sec->Name +
-              " is invalid",
-          "Link field value " + Twine(Sec->Link) + " in section " + Sec->Name +
-              " is not a string table"));
-    }
   }
 
   return SecTable;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38329.116866.patch
Type: text/x-patch
Size: 2337 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170927/af2e0429/attachment-0001.bin>