[llvm-objcopy] Add option to add a progbits section from a file

[jakehehrlich]

Summary

This change adds support for adding progbits sections with contents from a file.

[jhenderson]

Test case?

tools/llvm-objcopy/Object.h

129 ↗

(On Diff #126874)

I'm not convinced about the name of the class - how is it different from a Section? It's primarily because the data is owned, isn't it?

Perhaps OwnedDataSection? What do you think?

tools/llvm-objcopy/llvm-objcopy.cpp

116 ↗

(On Diff #126874)

Has this line been clang-formatted? It looks quite long.

[jakehehrlich]

tools/llvm-objcopy/Object.h

129 ↗

(On Diff #126874)

Yep the point is that it owns the data. That seems like a better name. I'll switch it to that.

[jakehehrlich]

1. Added test. I think I just didn't run git add -A before I ran git diff last time
2. Changed names
3. Fixed formatting issues.

[jhenderson]

tools/llvm-objcopy/llvm-objcopy.cpp

183–196 ↗

(On Diff #127046)

It looks like GNU objcopy does add-section after removal of sections. I think we should do the same, so this block of code should move after the removeSections call, but before finalize.

Also, a test demonstrating this point would be good.

This behaviour makes sense, because it allows you to replace the section contents of an existing section.

By the way, I also discovered that for some reason, GNU objcopy won't let you add another instance of a section that already exists. I don't think we need to match this behaviour - it's perfectly valid to add such a section.

[jakehehrlich]

I agree on the points about replacement and about not following what GNU objcopy does on duplicate sections.

1. Added test to show that you can replace a section using --remove-section and --add-section
2. Moved the AddSection code to after section removal.

[jhenderson]

LGTM.

llvm/trunk/test/tools/llvm-objcopy/add-section-remove.test

+# RUN: yaml2obj %s > %t
+# RUN: echo 0000 > %t.sec
+# RUN: llvm-objcopy -R=.test2 -add-section=.test2=%t.sec %t %t2
+# RUN: llvm-readobj -file-headers -sections -section-data %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .test1
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:        "c3c3c3c3"
+  - Name:            .test2
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:        "DEADBEEF"
+  - Name:            .test3
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:        "32323232"
+
+# CHECK: SectionHeaderCount: 7
+
+# CHECK: Name: .test1
+# CHECK: Name: .test3
+# CHECK: Name: .symtab
+# CHECK: Name: .strtab
+# CHECK: Name: .shstrtab
+# CHECK: Name: .test2
+# CHECK: SectionData (
+# CHECK-NEXT:   0000: 30303030
+# CHECK-NEXT: )

llvm/trunk/test/tools/llvm-objcopy/add-section.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy -O binary -j .test2 %t %t.sec
+# RUN: llvm-objcopy -R=.test2 %t %t2
+# RUN: llvm-objcopy -add-section=.test2=%t.sec %t2 %t3
+# RUN: llvm-readobj -file-headers -sections -section-data %t3 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .test1
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:        "c3c3c3c3"
+  - Name:            .test2
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:        "DEADBEEF"
+  - Name:            .test3
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:        "32323232"
+
+# CHECK: SectionHeaderCount: 7
+
+# CHECK: Name: .test1
+# CHECK: Name: .test3
+# CHECK: Name: .symtab
+# CHECK: Name: .strtab
+# CHECK: Name: .shstrtab
+# CHECK: Name: .test2
+# CHECK: SectionData (
+# CHECK-NEXT:   0000: DEADBEEF
+# CHECK-NEXT: )

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

private:
   ArrayRef<uint8_t> Contents;
 
 public:
   Section(ArrayRef<uint8_t> Data) : Contents(Data) {}
 
   void writeSection(FileOutputBuffer &Out) const override;
 };
 
+class OwnedDataSection : public SectionBase {
+private:
+  std::vector<uint8_t> Data;
+
+public:
+  OwnedDataSection(StringRef SecName, ArrayRef<uint8_t> Data)
+      : Data(std::begin(Data), std::end(Data)) {
+    Name = SecName;
+    Type = ELF::SHT_PROGBITS;
+    Size = Data.size();
+  }
+  void writeSection(FileOutputBuffer &Out) const override;
+};
+
 // There are two types of string tables that can exist, dynamic and not dynamic.
 // In the dynamic case the string table is allocated. Changing a dynamic string
 // table would mean altering virtual addresses and thus the memory image. So
 // dynamic string tables should not have an interface to modify them or
 // reconstruct them. This type lets us reconstruct a string table. To avoid
 // this class being used for dynamic string tables (which has happened) the
 // classof method checks that the particular instance is not allocated. This
 // then agrees with the makeSection method used to construct most sections.
public:
   bool WriteSectionHeaders = true;
 
   Object(const object::ELFObjectFile<ELFT> &Obj);
   virtual ~Object() = default;
 
   const SymbolTableSection *getSymTab() const { return SymbolTable; }
   const SectionBase *getSectionHeaderStrTab() const { return SectionNames; }
   void removeSections(std::function<bool(const SectionBase &)> ToRemove);
+  void addSection(StringRef SecName, ArrayRef<uint8_t> Data);
   virtual size_t totalSize() const = 0;
   virtual void finalize() = 0;
   virtual void write(FileOutputBuffer &Out) const = 0;
 };
 
 template <class ELFT> class ELFObject : public Object<ELFT> {
 private:
   using SecPtr = std::unique_ptr<SectionBase>;

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

 
 void Section::writeSection(FileOutputBuffer &Out) const {
   if (Type == SHT_NOBITS)
     return;
   uint8_t *Buf = Out.getBufferStart() + Offset;
   std::copy(std::begin(Contents), std::end(Contents), Buf);
 }
 
+void OwnedDataSection::writeSection(FileOutputBuffer &Out) const {
+  uint8_t *Buf = Out.getBufferStart() + Offset;
+  std::copy(std::begin(Data), std::end(Data), Buf);
+}
+
 void StringTableSection::addString(StringRef Name) {
   StrTabBuilder.add(Name);
   Size = StrTabBuilder.getSize();
 }
 
 uint32_t StringTableSection::findIndex(StringRef Name) const {
   return StrTabBuilder.getOffset(Name);
 }
for (auto &Segment : Segments)
       Segment->removeSection(RemoveSec.get());
     for (auto &KeepSec : make_range(std::begin(Sections), Iter))
       KeepSec->removeSectionReferences(RemoveSec.get());
   }
   // Now finally get rid of them all togethor.
   Sections.erase(Iter, std::end(Sections));
 }
 
+template <class ELFT>
+void Object<ELFT>::addSection(StringRef SecName, ArrayRef<uint8_t> Data) {
+  auto Sec = llvm::make_unique<OwnedDataSection>(SecName, Data);
+  Sec->OriginalOffset = ~0ULL;
+  Sections.push_back(std::move(Sec));
+}
+
 template <class ELFT> void ELFObject<ELFT>::sortSections() {
   // Put all sections in offset order. Maintain the ordering as closely as
   // possible while meeting that demand however.
   auto CompareSections = [](const SecPtr &A, const SecPtr &B) {
     return A->OriginalOffset < B->OriginalOffset;
   };
   std::stable_sort(std::begin(this->Sections), std::end(this->Sections),
                    CompareSections);

llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp

 static cl::opt<bool> ExtractDWO(
     "extract-dwo",
     cl::desc("Remove all sections that are not DWARF .dwo sections from file"));
 static cl::opt<std::string>
     SplitDWO("split-dwo",
              cl::desc("Equivalent to extract-dwo on the input file to "
                       "<dwo-file>, then strip-dwo on the input file"),
              cl::value_desc("dwo-file"));
+static cl::list<std::string> AddSection(
+    "add-section",
+    cl::desc("Make a section named <section> with the contents of <file>."),
+    cl::value_desc("section=file"));
 
 using SectionPred = std::function<bool(const SectionBase &Sec)>;
 
 bool IsDWOSection(const SectionBase &Sec) { return Sec.Name.endswith(".dwo"); }
 
 template <class ELFT>
 bool OnlyKeepDWOPred(const Object<ELFT> &Obj, const SectionBase &Sec) {
   // We can't remove the section header string table.
template <class ELFT> void CopyBinary(const ELFObjectFile<ELFT> &ObjFile) {
   if (!OutputFormat.empty() && OutputFormat != "binary")
     error("invalid output format '" + OutputFormat + "'");
   if (!OutputFormat.empty() && OutputFormat == "binary")
     Obj = llvm::make_unique<BinaryObject<ELFT>>(ObjFile);
   else
     Obj = llvm::make_unique<ELFObject<ELFT>>(ObjFile);
 
   if (!SplitDWO.empty())
-    SplitDWOToFile<ELFT>(ObjFile, SplitDWO.getValue()); 
+    SplitDWOToFile<ELFT>(ObjFile, SplitDWO.getValue());
 
   SectionPred RemovePred = [](const SectionBase &) { return false; };
 
   // Removes:
 
   if (!ToRemove.empty()) {
     RemovePred = [&](const SectionBase &Sec) {
       return std::find(std::begin(ToRemove), std::end(ToRemove), Sec.Name) !=
RemovePred = [RemovePred](const SectionBase &Sec) {
           std::end(Keep))
         return false;
       // Otherwise defer to RemovePred.
       return RemovePred(Sec);
     };
   }
 
   Obj->removeSections(RemovePred);
+
+  if (!AddSection.empty()) {
+    for (const auto &Flag : AddSection) {
+      auto SecPair = StringRef(Flag).split("=");
+      auto SecName = SecPair.first;
+      auto File = SecPair.second;
+      auto BufOrErr = MemoryBuffer::getFile(File);
+      if (!BufOrErr)
+        reportError(File, BufOrErr.getError());
+      auto Buf = std::move(*BufOrErr);
+      auto BufPtr = reinterpret_cast<const uint8_t *>(Buf->getBufferStart());
+      auto BufSize = Buf->getBufferSize();
+      Obj->addSection(SecName, ArrayRef<uint8_t>(BufPtr, BufSize));
+    }
+  }
+
   Obj->finalize();
   WriteObjectFile(*Obj, OutputFilename.getValue());
 }
 
 int main(int argc, char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);