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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D62462#1595071 <https://reviews.llvm.org/D62462#1595071>, @mmpozulp wrote:

> @jhenderson, can you think of an input that would cause `Symbolizer->symbolizeCode(*Obj, Address)` to return an `Error`?
>
> I went a few calls deep underneath `Symbolizer->symbolizeCode(*Obj, Address)` and it looks like `LLVMSymbolizer` and `DWARFContext` return a default-constructed `DILineInfo` when they fail to parse the debug info. Can we cause a failure that returns an `Error` instead? If so we can check whether the `Error` is actually an `ErrorInfo` subclass which is how custom errors are created according to error handling Detailed Description <https://llvm.org/doxygen/classllvm_1_1Error.html#details>. A better implementation of `SourcePrinter::printSourceLine()` might call `handleErrors()` instead of `consumeError()`. Grepping the code for `handleErrors` and `handleAllErrors` provides some example uses. First I need some inputs to cause errors so I can see whether any `ErrorInfo` subclasses are returned and if they have useful context which we can incorporate into our warning. This is my first time wading into LLVM's error handling so please let me know if you have any reading suggestions. In addition to the doxygen docs for Error.h I also found a section on error handling in the programmer's manual <http://llvm.org/docs/ProgrammersManual.html#error-handling>.


I dug into the code myself, and there are a number of examples in the call tree where Expected/Error etc might be returned. For example, for a PPC ELF, when "SymbolizableObjectFile::create" is called, it tries to fetch the section name. If it fails to do so (e.g. because the sh_name field of the section header is invalid). There may well be many others too. I'm not sure how straightforward it would be to create this situation, but you could try using yaml2obj to create such an object for a test case (you can see an example of this error message in test/Object/invalid.test, looking for the "BROKEN-SECNAME" test case).

When it comes to handling the errors, you might want to look at `logAllUnhandledErrors` or `toString` in Error.h. The former can be used to write the error to a stream (which could be a string stream of some sort), whilst the latter converts the Error into a std::string, which you can then use.

It's worth noting that except for success values, ALL Error instances have an ErrorInfoBase member. The comments in the docs about ErrorInfo being what is used for custom errors is somewhat irrelevant to this case here, and simply means that you should inherit from ErrorInfo and not ErrorInfoBase when implementing custom error types. I'd therefore just assume that any non-success Error instancesdo have useful additional context and always use it.

Hopefully that gives you the information you need to go about my suggestion. I think in my pseudo code, you can just replace the line `Msg = ExpectedLineInfo.error.message();` with `Msg = toString(ExpectedLineInfo.takeError())`. That would also avoid the need to call `consumeError()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62462