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

Nick Lewycky nlewycky at google.com
Wed Oct 30 15:52:28 PDT 2013


On 30 October 2013 13:24, Tobias Grosser <tobias at grosser.es> wrote:

> On 10/30/2013 07:24 PM, Rafael EspĂ­ndola wrote:
>
>>  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.
>>
>
> Just out of interest. Are you planning to make the use of bitcode files
> supported for a wider audience? The problem I see here is that there is no
> good way for the tools to check if a certain bitcode file can be correctly
> read. At the moment, we are sometimes crashing, but e.g. the bug that I
> fixed caused invalid bitcode to be read. So bitcode files are only known to
> be really compatible for tools that use the very same version of LLVM (we
> try to maintain the ability to read older bitcode files, but this is not
> very well tested).  If we want to change this, we probably need some kind
> of version marker.
>
>
>  Another option would be to just drop the AttributeKind id, in case Nick is
>>> OK with this.
>>>
>>
>> Yes, that would also work.
>>
>
> Given the choice between this and the ability of gracefully fail if
> reading bitcode files, I very much prefer this.
>
> I copied Nick to get his opinion.
>

Trying to loading an invalid .bc file shouldn't be fatal. I don't have a
user which relies on this, but it seems like the right policy and is more
important than including the number.

It is just intended to be a debugging aid; I can live without it as long as
you can live with me cursing your name when I next get this error message.
:-P

I haven't looked too closely. Is it that bad to make Error() take an
std::string? Does this leak through the C API or something?

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131030/4c21513c/attachment.html>


More information about the llvm-commits mailing list