[PATCH] D63475: [llvm-readobj] Allow --hex-dump/--string-dump to dump multiple sections

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: test/tools/llvm-readobj/hex-dump.test:2
+Both 9 and .strtab refer to .strtab. Test we dump the section only once.
+RUN: llvm-readobj -x 9 -x .strtab -x .strtab %p/Inputs/trivial.obj.elf-x86-64 2>&1 | \
+RUN:   FileCheck %s --check-prefix=ELF
----------------
I'd suggest starting the lines from '#'. That will be useful for git history
when we convert all inputs to YAML and inline them here.

Also, you list `-x .strtab` twice, but `-x 9` only once, so may be list it twice too?


================
Comment at: test/tools/llvm-readobj/hex-dump.test:5
+RUN: llvm-readobj -x 9 -x .strtab -x 10 -x not_exist \
+RUN:   %p/Inputs/trivial.obj.elf-x86-64 2>&1 | FileCheck %s --check-prefixes=ELF-WARN,ELF
+
----------------
Can you add check lines showing that .symtab really has index == 9?


================
Comment at: test/tools/llvm-readobj/hex-dump.test:17
+
+RUN: llvm-readobj -x 1 %p/Inputs/trivial.obj.coff-x86-64 \
+RUN:     | FileCheck %s --check-prefix COFF
----------------
In this and below tests you just check that can use section index with `-x` for
the different targets, right? Please add a comment explaining that.


================
Comment at: tools/llvm-readobj/ObjDumper.cpp:40
+  std::vector<object::SectionRef> Ret;
+  StringMap<bool> SecNames;
+  std::unordered_map<int, bool> SecIndices;
----------------
Map of `bool` is the same as `set` I think. Can you use `StringSet` instead of `StringMap<bool>`?


================
Comment at: tools/llvm-readobj/ObjDumper.cpp:88
+    else
+      W.startLine() << '\n';
+    W.startLine() << "String dump of section '" << SectionName << "':\n";
----------------
I'd:


```
if (First)
  W.startLine() << '\n';
First = false;
```


================
Comment at: tools/llvm-readobj/ObjDumper.cpp:123
+    else
+      W.startLine() << '\n';
+    W.startLine() << "Hex dump of section '" << SectionName << "':\n";
----------------
The same.


================
Comment at: tools/llvm-readobj/ObjDumper.h:107
 
-  void printSectionAsString(const object::ObjectFile *Obj, StringRef SecName);
-  void printSectionAsHex(const object::ObjectFile *Obj, StringRef SecName);
+  void printSectionAsString(const object::ObjectFile *Obj,
+                            const std::vector<std::string> &Sections);
----------------
printSection**s**AsString, printSection**s**AsHex ?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63475/new/

https://reviews.llvm.org/D63475