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

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:183-196
+  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)
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D41212