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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
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
----------------
grimar wrote:
> 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.
This file was renamed from `print-hex.test` to be consistent with the option name (also, `string-dump.test` is not named `print-string.test`)


================
Comment at: tools/llvm-readobj/ObjDumper.cpp:69
+    if (!S.second)
+      reportWarning(formatv("could not find section '{0}'", S.first()).str());
+  for (std::pair<int, bool> S : SecIndices)
----------------
grimar wrote:
> I do not think you can do this, because StringMap iteration order is not guaranteed to be deterministic.
Good catch! Changed to `std::map`.


================
Comment at: tools/llvm-readobj/ObjDumper.cpp:70
+      reportWarning(formatv("could not find section '{0}'", S.first()).str());
+  for (std::pair<int, bool> S : SecIndices)
+    if (!S.second)
----------------
grimar wrote:
> grimar wrote:
> > The same. You should use `std::map` I think.
> Or, if your intention was to keep the insertion order in the outpuit, you probably need `MapVector`.
The logic is:

```
for each section:
  if section name is specified by -x or section index is specified by -x:
    dump()
```

The insertion order doesn't matter. I'll use `std::map` for simplicity.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63475