[PATCH] D38260: [llvm-objcopy] Add support for removing sections

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 26 18:37:50 PDT 2017


jakehehrlich added inline comments.


================
Comment at: tools/llvm-objcopy/Object.cpp:299
 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) +
----------------
jhenderson wrote:
> Why has this changed?
Oh whoops. I forgot to mention this. It was wrong before. I was using StringTableSection for what under the hood was actually a Section. This was fine because I wasn't using anything that wasn't in SectionBase here. Additionally dynamic string tables have SHT_STRTAB so the cast went though just fine. I think to avoid this mistake in the future I should probably add a check that StringTableSection is not allocated.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:59
                                "\n\tbinary"));
+cl::opt<std::string> ToRemove("R", cl::desc("Remove a specific section"));
 
----------------
jhenderson wrote:
> objcopy also allows --remove-section as an alias. It also allows specifying the option multiple times to strip multiple sections.
I'll add --remove-section as an alias. Would you take  making ToRemove a cl::list as a todo?


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:72
+    Obj->removeSections(
+        [&](const SectionBase &Sec) -> bool { return ToRemove == Sec.Name; });
+  }
----------------
jhenderson wrote:
> Why a lambda here? Also, the explicit return type is unnecessary.
What alternative are you proposing? Or are you just asking why this method takes a lambda? That's because many other conditions for removal will be added. This interface should cover all of them however.


Repository:
  rL LLVM

https://reviews.llvm.org/D38260





More information about the llvm-commits mailing list