[PATCH] Fix for bitcode reader crash on invalid input [PR18704]

[Nick Lewycky]

On 2 March 2014 21:13, Reid Kleckner <rnk at google.com> wrote:

> Nick and I looked at this and we think there are other issues of a similar
> nature in the bitcode reader.  There are many places where ValueList is
> indexed and accessed without any null check.  I think Nick said the list
> shouldn't contain any null elements unless we're returning an error, which
> is what your test case does.
>

I did say that, but I take it back. There is a place where it does reserve
values in advance, and that's what happens in this testcase. The testcase
then demonstrates an access without a null check in error handling code,
but I don't see why the other non-error handling code shouldn't have it
either (I kept looking, which is why I never replied).


> On Sun, Mar 2, 2014 at 9:08 PM, Dinesh Dwivedi <dinesh.d at samsung.com>wrote:
>
>> ping..
>>
>>
>> Regards
>> Dinesh Dwivedi
>>
>> ------- Original Message -------
>> Sender : Dinesh Dwivedi<dinesh.d at samsung.com>  Chief
>> Engineer/SRI-Bangalore-Native Framework/Samsung Electronics
>> Date   : Feb 27, 2014 15:51 (GMT+05:30)
>> Title  : [PATCH] Fix for bitcode reader crash on invalid input [PR18704]
>>
>>
>> Hi Reid,
>>
>> I have attached a separate binary patch for bc file. This is same file
>> attached in bug report. I have added llvm-bcanalyzer as comment in test
>> case. But I do not understand much of it.
>>
>> I have updated my fix a bit. I have update dyn_cast to dyn_cast_or_null
>> for all location casting values from ValueList to something. If code was
>> already returning some error code on null value, I left them as it is. For
>> other cases, I have added null check and on null it will return
>> "InvalidValue".
>>
>> Regards
>> Dinesh Dwivedi
>>
>>
>> ------- Original Message -------
>> Sender : Reid Kleckner<rnk at google.com>
>> Date : Feb 27, 2014 00:16 (GMT+05:30)
>> Title : Re: [PATCH] Fix for bitcode reader crash on invalid input
>> [PR18704]
>>
>> The binary content of the .bc file isn't present in the diff.  Can you
>> attach it, and maybe paste the output of llvm-bcanalyzer into the test case
>> as a comment?  Also, the fix doesn't look correct.  It looks like
>> subsequent Arguments might leak, although that isn't really critical.
>>
>>
>>
>> On Wed, Feb 26, 2014 at 7:37 AM, Dinesh Dwivedi <dinesh.d at samsung.com>
>> wrote:
>>
>> Attaching patch for PR18704. I have just gone through back-trace for the
>> assert [dyn_cast was getting applied to NULL value], locate code causing
>> this and added check to print error for NULL values.
>>
>> Regards
>> Dinesh Dwivedi
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> <p> </p><p> </p>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140302/75db9ab3/attachment.html>