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

[Rafael Espíndola]

>>> 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.
>
>
> Yes, it would be in a fragile situation and I would not advice anybody to
> rely on the error handling in the bitcode reader.
>
> On the other hand, the existing error reporting infrastructure seems
> to be written exactly to support non-fatal errors. If we make fatal errors
> an official policy for the bitcode reader, it the interface would be
> unnecessary, no?

Yes, which is why the current situation is odd. The asserts say "fatal
errors", the interface says "non-fatal errors". We should pick one.

> Regarding long running users, I can see e.g. OpenCL runtimes that use
> bitcode to cache binary programs and that may run due to some bugs into
> bitcode incompatibilities. If those runtimes have the chance to return an
> OpenCL error code, but to continue to work properly for other kernels, that
> seems to be beneficial.

For anything actually using the code generator I think using a helper
process is the only reasonable solution. It is probably possible to
make some basic llvm libraries "crash proof", but not something as big
and complicated as codegen.

>> 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?
>
>
> $ grep error_code lib/Bitcode/* -R
> $
>
> This gives zero results. Which API are you talking about? Do those API's
> really need std::string? Or are you saying they are forwarding the
> result of the Bitcode reader?

I found this when I started to implement a ObjectFile interface for
bitcode files. I want that so that llvm-nm and llvm-ar (for now) can
transparently use bitcode files. The interface used by ObjectFile is
based on error_code.

> Another option would be to just drop the AttributeKind id, in case Nick is
> OK with this.

Yes, that would also work.

Cheers,
Rafael