Add more error checking to llvm-objdump and MachODump

[Lang Hames via llvm-commits]

I think we're essentially in agreement. I wasn't trying to say that error
handling will be trivial, just that if Pete and Kevin are prepared to do
the hard work (which I know they are, since they both want this feature) we
should push forward on it.

On a side note - you've mentioned a couple of times the danger of dropping
errors, which is one of my concerns with std::error_code. Have we
considered actually coding in a check for this? I.e. an error type (or
diagnostics class) that will fatal_error in the destructor if errors are
not explicitly handled or dismissed?

Cheers,
Lang.


On Tue, Dec 15, 2015 at 6:51 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> > No comment on the rest of the patch (I haven't read it), but regarding
> error
> > handling schemes I'm not sure I understand the objection. Most users
> would
> > be unaffected by any change in error handling since errors in objects
> files
> > are uncommon. Some users care a lot, but they're the same people
> > volunteering to do the development and maintenance (e.g. Kevin and Pete).
> > The changes shouldn't place undue burden on anyone else.
>
> That is not the case. Error handling has a substantial cost in code
> complexity and therefore correctness. Broken .o files are uncommon,
> but it is important that we can at least error on them.
>
> My dislike for half attempts to use non fatal errors grew while
> looking at the old lld. A good case would look something like
>
> * main -> foo -> bar -> ... -> zed
> * zed finds an error and returns an error_code
> * the code in foo, bar, etc is correct and return out the error_code
> * main prints a pretty horrible error message (EC.message()) since it
> has no context
>
> That is bad for anyone seeing the error and horrible for anyone
> debugging. You have to step in until you find that it is zed that is
> producing the error.
>
> And that is the *good* case. In reality it is pretty likely that some
> function along the way forgot to return EC.
>
> From that experience I think some properties we must ensure are
>
> * Diagnostic handling must be done close to where the error is found.
> This is very similar to what clang does. The calling functions can
> record context and then when zed calls a diagnostic handler it can say
> what was the error and what the program was trying to do.
> * We have to try to make sure errors are handled. By far the simplest
> way to do that is to have fatal errors. This can be done in library
> code by passing an diagnostic handler that exits or more simply in
> program code by just calling exit.
>
> The comparison with clang I think is enlightening. In clang good
> diagnostic are extremely important since broken c++ files are common.
> Since it is a major features, it is treated like that:
>
> * It uses a diagnostic handler so that errors can be printed with a
> lot of context.
> * It has special features to make error testing simple.
> * It has *lots* of test for its errors and warnings.
>
> So I can see two self consistent ways of coding objdump
>
> * We have to detect errors, but they are uncommon and we just have to
> make sure we are not silent about it.
> * Error handing of broken .o files is a major feature and we treat it like
> that.
>
> There nothing in the middle because there no such thing as simple
> non-fatal error handling, so the cost is high. If the const is high,
> there must be a big justification for it.
>
> What I cannot possibly understand is saying it is not a big thing but
> complicating the code anyway. If we are going to have non fatal (i.e.:
> complicated) error handling, I insist that we must:
>
> * Produce good error messages with good context (foo is broken in the
> FDE of function bar of zed.o in bah.a)
> * Check in .o files that illustrate every error.
> * Have .o files that illustrate we handling multiple error gracefully.
>
> I.E.: If it is a major feature, treat it like that.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151215/20f79795/attachment.html>