[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())));
----------------
grimar wrote:
> 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?
Follow-up: D83208


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

https://reviews.llvm.org/D83129