[PATCH] D83129: [llvm-readobj] - Allow dumping partially corrupted SHT_LLVM_CALL_GRAPH_PROFILE sections.

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6559-6566
+    if (Expected<std::string> NameOrErr =
+            this->dumper()->getStaticSymbolName(Index))
+      return *NameOrErr;
+    else
+      this->reportUniqueWarning(
+          createError("unable to read the name of symbol with index " +
+                      Twine(Index) + ": " + toString(NameOrErr.takeError())));
----------------
jhenderson wrote:
> This seems like a pattern we're likely to have in several different parts of the ELFDumper. Is there any code we could share to avoid duplication? Maybe it just makes sense to change `getStaticSymbolName` to report the warning/return the `<?>` itself?
>From what I see, the `getStaticSymbolName` is used in one more place:

```
template <class ELFT>
void LLVMStyle<ELFT>::printAddrsig(const ELFFile<ELFT> *Obj) {
...
  for (uint64_t Sym : *V) {
    Expected<std::string> NameOrErr = this->dumper()->getStaticSymbolName(Sym);
    if (NameOrErr) {
      W.printNumber("Sym", *NameOrErr, Sym);
      continue;
    }
    reportWarning(NameOrErr.takeError(), this->FileName);
    W.printNumber("Sym", "<?>", Sym);
  }
}
```

And it looks like it should be reasonable and possible to do what you suggest. Follow-up?


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

https://reviews.llvm.org/D83129