[PATCH] D62462: [llvm-objdump] Add warning messages if disassembly + source for problematic inputs

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:621
+    if(!WarnedNoDebugInfo) {
+      warn("unable to find debug information for " + ObjectFilename);
+      WarnedNoDebugInfo = true;
----------------
mmpozulp wrote:
> jhenderson wrote:
> > It's probably worth sticking with the "failed to parse..." phrasing in case there was a genuine problem parsing it:
> > "failed to parse debug information for " + ObjectFilename.
> > 
> > Bonus points if it makes sense to use the infromation in the ExpectedLinkInfo error in this case.
> I ran it in lldb and observed that in this case `ExpectedLineInfo` has no error, so according to the [[ http://llvm.org/docs/ProgrammersManual.html#recoverable-errors | programmer's manual ]] all it can tell us is "success":
> 
> > If an Expected<T> value is in success mode then the takeError() method will return a success value.
I'm not quite sure I follow. Above, ExpectedLineInfo is checked to determine if there is an error, but we then throw it away without updating the LineInfo FileName (see lines 614-617). Presumably this code exists precisely because the debug data couldn't be parsed, and the Expected error in this case will contain the reason why. There may be cases where the line info is successfully fetched, but with no file information, which we should warn about without any additional context, as happens now. Otherwise, it would be good to provide the extra context. Something like the following:
```
  std::string Msg;
  if (!ExpectedLineInfo) {
    // pseudo code
    Msg = ExpectedLineInfo.error.message();
    consumeError(ExpectedLineInfo.takeError());
  }
  else
    LineInfo = *ExpectedLineInfo;

  if (LineInfo.FileName == DILineInfo::BadString) {
    if(!WarnedNoDebugInfo) {
      std::string Warning = "failed to parse debug information for " + ObjectFilename;
      if (!Msg.empty())
        Warning += ": " + Msg;
      warn(Warning);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62462