[patch] Fix use after free in BitcodeReader.cpp error path

[Rafael Espíndola]

> Using fatal errors in the context of a library is unfortunate, as the fatal
> error message on errs() may often not reach the user.

Our fatal error implementation is more flexible than that. The tool
could do pretty much anything with the error: log it, open a dialog,
etc, as long as it remains fatal.

> The asserts() seem to
> have been added under the assumption that invalid
> bitcode files only happen for developers. One could claim that reading
> invalid bitcode files should not happen for users, but as I run into this
> and Nick asked for more debug output, it seems to not be such a rare case.

I agree that that assumption is false. We should be able to handle
corrupt files in release builds.

> Even if we can not catch all cases of invalid bitcode files,
> I believe we should try to avoid fatal errors if we can.

Given the flexibility we give to the fatal error handling, the real
question I think is: Do we expect any tool to handle this in a non
fatal way? Right now the answer for in tree tools is no. Any such tool
out of tree would also be in a very fragile situation given the
existing asserts.

> Consequently I believe using or extending the already existing 'const char
> *' interface is the right thing to do. As the Error String is not in the hot
> path and as the bitcode reader does not seem a class that is
> instantiated very often, what about making ErrorString a std::string?

This would work for this particular case, but then how do we interface
"rich" non fatal errors with APIs that use error_code? Do we extend
error code to carry a std::string too?

Cheers,
Rafael