llvm-objdump --section, -j section filtering

[colinl]

Summary

This patch adds the --section and -j options to llvm-objdump to specify a section to dump.

One thing that needs to be addressed is the --section flag is in conflict with the MachO flag of the same name. We either need to change the name of the filtering flag which might be unexpected behavior for people coming from other toolchains or change the MachO flag which would change existing behavior.

My recommendation would be changing the machO flag to --macho-section.

[colinl]

Using the same option is a possibility, as long as there's not a case where we'd want to use both at the same time. Looking in DumpInput it looks like if -macho is set it'll follow the ParseInputMachO path whereas otherwise it will dump it as an archive or object file so this seems like they're currently mutually exclusive.

Agreed on the test case, I'll prepare something and modify the patch.

[colinl]

After discussing with Kevin Enderby it seemed like merging the usage of --section seemed feasible. This is the merged result as well as a test showing sections being filtered.

[filcab]

Just some nitpicky details.
I also prefer to have empty lines before class definitions, but that might just be due to the way I use my editor.

Thank you,
Filipe

tools/llvm-objdump/llvm-objdump.cpp

204

We're *not* keeping the ones where the predicate returns true?
I'd prefer to call it FilterOut, or reverse this condition.

237

Is this an appropriate value to return?

243

return std::find(Sections.begin(), Sections.end(), String) == Sections.end();

(or != Sections.end() if you switch the predicate around)

tools/llvm-objdump/llvm-objdump.h

28

Please split this in two commits: Renaming (and moving between .cpp files) DumpSections to Sections, and the the functional change.

29

Are you changing whitespace in line 29? Adding \r? (The diff on phabricator looks weird)
If you're changing WS, please split that to another commit. If you're adding \r, please don't. :-)

[colinl]

For the time being I added this with just the -j flag. One concern I had with merging the --section flag was it looks like some MachODump operations iterate over all sections and people may want to filter those down which wouldn't be available since --section would mean something else when MachO dumping.

printMachOUnwindInfo, SegInfo, printObjc2_32bit_MetaData are things I was thinking of.

[colinl]

Submitted in r243526 with just the "j" flag and without MachO changes.

Still working on if we should do anything with the conflicting --section flag.

[colinl]

tools/llvm-objdump/llvm-objdump.cpp

237

The only other alternative I considered was returning a bool, otherwise the return types match exactly, int. Is there another possibility to consider?

243

True, I'll make that change.

[colinl]

Merged MachO DumpSections in to FilterSections with 243556. Also simplifying the filtering logic as suggested. Leaving open though if more changes are wanted.

tools/llvm-objdump/llvm-objdump.cpp

204

I could see how that could be better, let me take a look at which way would make more sense.

tools/llvm-objdump/llvm-objdump.h

28

Sorry I didn't address this before submitting, that was unintentional I have to get better at phabricator.

29

I think I have SVN configured to make sure it only submits proper LF. This should just be an artifact of the diff upload.

test/tools/llvm-objdump/section-filter.test

+// This test checks that --section works correctly
+// RUN: llvm-objdump -h %p/Inputs/section-filter.obj --section=.text \
+// RUN: --section=.bss | FileCheck %s
+
+# CHECK: .text
+# CHECK-NOT: .data
+# CHECK: .bss
 No newline at end of file

tools/llvm-objdump/MachODump.cpp

                        cl::desc("Print the linker optimization hints for "
                                 "Mach-O objects (requires -macho)"));
 
-cl::list<std::string>
-    llvm::DumpSections("section",
-                       cl::desc("Prints the specified segment,section for "
-                                "Mach-O objects (requires -macho)"));
-
 cl::opt<bool>
     llvm::InfoPlist("info-plist",
                     cl::desc("Print the info plist section as strings for "

   if (verbose)
     CreateSymbolAddressMap(O, &AddrMap);
 
-  for (unsigned i = 0; i < DumpSections.size(); ++i) {
-    StringRef DumpSection = DumpSections[i];
+  for (unsigned i = 0; i < Sections.size(); ++i) {
+    StringRef DumpSection = Sections[i];
     std::pair<StringRef, StringRef> DumpSegSectName;
     DumpSegSectName = DumpSection.split(',');
     StringRef DumpSegName, DumpSectName;

   // UniversalHeaders or ArchiveHeaders.
   if (Disassemble || PrivateHeaders || ExportsTrie || Rebase || Bind ||
       LazyBind || WeakBind || IndirectSymbols || DataInCode || LinkOptHints ||
-      DylibsUsed || DylibId || ObjcMetaData || (DumpSections.size() != 0)) {
+      DylibsUsed || DylibId || ObjcMetaData || (Sections.size() != 0)) {
     outs() << Filename;
     if (!ArchiveMemberName.empty())
       outs() << '(' << ArchiveMemberName << ')';

     PrintSectionHeaders(MachOOF);
   if (SectionContents)
     PrintSectionContents(MachOOF);
-  if (DumpSections.size() != 0)
+  if (Sections.size() != 0)
     DumpSectionContents(Filename, MachOOF, !NonVerbose);
   if (InfoPlist)
     DumpInfoPlistSectionContents(Filename, MachOOF);

     diContext.reset(new DWARFContextInMemory(*DbgObj));
   }
 
-  if (DumpSections.size() == 0)
+  if (Sections.size() == 0)
     outs() << "(" << DisSegName << "," << DisSectName << ") section\n";
 
   for (unsigned SectIdx = 0; SectIdx != Sections.size(); SectIdx++) {

tools/llvm-objdump/llvm-objdump.h

 extern cl::opt<std::string> ArchName;
 extern cl::opt<std::string> MCPU;
 extern cl::list<std::string> MAttrs;
-extern cl::list<std::string> DumpSections;
-extern cl::opt<bool> Disassemble;
+extern cl::list<std::string> Sections;
+extern cl::opt<bool> Disassemble;
 extern cl::opt<bool> DisassembleAll;
 extern cl::opt<bool> NoShowRawInsn;
 extern cl::opt<bool> PrivateHeaders;

tools/llvm-objdump/llvm-objdump.cpp

                       cl::aliasopt(SectionHeaders));
 
 cl::list<std::string>
+llvm::Sections("section",
+               cl::desc("Operate on the specified section only"
+                        " Mach-O specific: Spcify as segment,section pair"));
+static cl::alias
+Sectionsj("j", cl::desc("Alias for --section"), cl::aliasopt(Sections));
+
+cl::list<std::string>
 llvm::MAttrs("mattr",
   cl::CommaSeparated,
   cl::desc("Target specific attributes"),

 static StringRef ToolName;
 static int ReturnValue = EXIT_SUCCESS;
 
+namespace {
+typedef std::function<int(llvm::object::SectionRef const &)> FilterPredicate;
+class SectionFilterIterator {
+public:
+  SectionFilterIterator(FilterPredicate P,
+                        llvm::object::section_iterator const &I,
+                        llvm::object::section_iterator const &E)
+      : Predicate(P), Iterator(I), End(E) {
+    ScanPredicate();
+  }
+  llvm::object::SectionRef operator*() const { return *Iterator; }
+  SectionFilterIterator &operator++() {
+    ++Iterator;
+    ScanPredicate();
+    return *this;
+  }
+  bool operator!=(SectionFilterIterator const &Other) const {
+    return Iterator != Other.Iterator;
+  }
+
+private:
+  void ScanPredicate() {
+    while (Iterator != End && Predicate(*Iterator)) {
+      ++Iterator;
+    }
+  }
+  FilterPredicate Predicate;
+  llvm::object::section_iterator Iterator;
+  llvm::object::section_iterator End;
+};
+class SectionFilter {
+public:
+  SectionFilter(FilterPredicate P, llvm::object::ObjectFile const &O)
+      : Predicate(P), Object(O) {}
+  SectionFilterIterator begin() {
+    return SectionFilterIterator(Predicate, Object.section_begin(),
+                                 Object.section_end());
+  }
+  SectionFilterIterator end() {
+    return SectionFilterIterator(Predicate, Object.section_end(),
+                                 Object.section_end());
+  }
+
+private:
+  FilterPredicate Predicate;
+  llvm::object::ObjectFile const &Object;
+};
+SectionFilter ToolSectionFilter(llvm::object::ObjectFile const &O) {
+  if (Sections.empty()) {
+    return SectionFilter([](llvm::object::SectionRef const &) { return 0; }, O);
+  }
+  return SectionFilter([](llvm::object::SectionRef const &S) {
+                         llvm::StringRef String;
+                         std::error_code error = S.getName(String);
+                         if (error) {
+                           return error.value();
+                         }
+                         if (std::find(Sections.begin(), Sections.end(),
+                                       String) != Sections.end()) {
+                           return 0;
+                         }
+                         return 1;
+                       },
+                       O);
+}
+}
+
 bool llvm::error(std::error_code EC) {
   if (!EC)
     return false;

 
     // If we couldn't find a symbol that this relocation refers to, try
     // to find a section beginning instead.
-    for (const SectionRef &Section : O->sections()) {
+    for (const SectionRef &Section : ToolSectionFilter(*O)) {
       std::error_code ec;
 
       StringRef Name;

   // in RelocSecs contain the relocations for section S.
   std::error_code EC;
   std::map<SectionRef, SmallVector<SectionRef, 1>> SectionRelocMap;
-  for (const SectionRef &Section : Obj->sections()) {
+  for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
     section_iterator Sec2 = Section.getRelocatedSection();
     if (Sec2 != Obj->section_end())
       SectionRelocMap[*Sec2].push_back(Section);

     array_pod_sort(AllSymbols.begin(), AllSymbols.end());
   }
 
-  for (const SectionRef &Section : Obj->sections()) {
+  for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
     if (!DisassembleAll && (!Section.isText() || Section.isVirtual()))
       continue;
 

   if (!Obj->isRelocatableObject())
     return;
 
-  for (const SectionRef &Section : Obj->sections()) {
+  for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
     if (Section.relocation_begin() == Section.relocation_end())
       continue;
     StringRef secname;

   outs() << "Sections:\n"
             "Idx Name          Size      Address          Type\n";
   unsigned i = 0;
-  for (const SectionRef &Section : Obj->sections()) {
+  for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
     StringRef Name;
     if (error(Section.getName(Name)))
       return;

 
 void llvm::PrintSectionContents(const ObjectFile *Obj) {
   std::error_code EC;
-  for (const SectionRef &Section : Obj->sections()) {
+  for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
     StringRef Name;
     StringRef Contents;
     if (error(Section.getName(Name)))

   }
 
   Optional<object::SectionRef> ClangASTSection;
-  for (auto Sec : Obj->sections()) {
+  for (auto Sec : ToolSectionFilter(*Obj)) {
     StringRef Name;
     Sec.getName(Name);
     if (Name == ClangASTSectionName) {

 
   Optional<object::SectionRef> FaultMapSection;
 
-  for (auto Sec : Obj->sections()) {
+  for (auto Sec : ToolSectionFilter(*Obj)) {
     StringRef Name;
     Sec.getName(Name);
     if (Name == FaultMapSectionName) {

       && !(DylibsUsed && MachOOpt)
       && !(DylibId && MachOOpt)
       && !(ObjcMetaData && MachOOpt)
-      && !(DumpSections.size() != 0 && MachOOpt)
+      && !(Sections.size() != 0 && MachOOpt)
       && !PrintFaultMaps) {
     cl::PrintHelpMessage();
     return 2;