[PATCH] D88715: [llvm-objdump] --source: drop the warning when there is no debug info

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1018
+  } else {
+    // TODO Untested.
+    reportWarning("failed to parse debug information for " + ObjectFilename +
----------------
rupprecht wrote:
> MaskRay wrote:
> > rupprecht wrote:
> > > It would be good to figure out a test case for this
> > The last paragraph of the description mentions this.
> > 
> > "The only code path is probably a broken symbol table, but we probably already emit a warning
> > in that case" (I have checked various code paths)
> > 
> > If it is a symbol table problem, it should not be a "debug info problem".
> The error message still says "failed to parse debug information", did you have plans to change that if it's not a debug info problem?
It says `"failed to parse debug information"`. The code path is likely dead now. If future changes to `Symbolizer->symbolizeCode` makes it actually possible to return an Error, this will give some diagnostics.

This diagnostic was added to report "no debug info", which isn't an appropriate message. But I don't want to remove it entirely either.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1019-1021
+    reportWarning("failed to parse debug information for " + ObjectFilename +
+                      ": " + toString(ExpectedLineInfo.takeError()),
+                  ObjectFilename);
----------------
rupprecht wrote:
> MaskRay wrote:
> > rupprecht wrote:
> > > `ObjectFilename` is in the warning twice now. It can be omitted from the error message string; passing it as the second arg should make it print something like: `warn: ObjectFilename: failed to parse debug information`
> > The code path is likely dead. This can be considered when ExpectedLineInfo does return an `Error`. (Note, there is no test for multiple warnings)
> I'm not sure what you mean by dead here -- are you saying `Symbolizer->symbolizeCode()` never returns an error?
`Symbolizer->symbolizeCode()` returning an `Error` is entirely untested in check-llvm.

By reading the code, I guess it may return an `Error` with a broken symbol table. But then, we would report a warning earlier in `disassembleObject`.

When `WarnedNoDebugInfo` was added, there was no test for multiple warning. It was a premature decision.

I can remove this piece entirely, but I think a TODO may be more appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88715