llvm dev - Jan 2015 - [LLVMdev] Fwd: Bitcode abbreviations for something that's not a record

Hi all!

Fuzzing llvm's bitcode reader, I found a problem where the reader assumes
that the first field in an abbreviation will not be an array or a blob (and
asserts otherwise).

I don't know if this is expected (but not documented) or not. The
documentation, to me, reads like it doesn't disallow it, but we might be
assuming all abreviations start with a full record, which would make the
first operand never be an array or a blob.

The bug comes from r181639 (
http://llvm.org/klaus/llvm/commit/1197e38f3338b8db76f0fa38c2687c65b2bcea5c/),
which took the code to read the first argument and put it outside of the
loop, but didn't take the Array/Blob verification + reading code too
(It's
a bug because that commit was supposed to not have changed functionality
:-) ).

This could be “fixed” with, either a report_fatal_error (if we eventually
have better error handling on that code, we can make that non-fatal and
report to the caller), or by hoisting the Array/Blob reading code out of
the loop too (actually, write a helper function).

What should be done about this?

Thanks,

  Filipe
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150123/63365075/attachment.html>

The restriction looks reasonable: A record starts with a code. The code can
be encoded as a literal or be part of the abbreviation.

There is probably not a lot of value in supporting a code embedded in the
first element of an array or blob.

On 23 January 2015 at 16:47, Filipe Cabecinhas <filcab at gmail.com>
wrote:
> Hi all!
>
> Fuzzing llvm's bitcode reader, I found a problem where the reader
assumes
> that the first field in an abbreviation will not be an array or a blob (and
> asserts otherwise).
>
> I don't know if this is expected (but not documented) or not. The
> documentation, to me, reads like it doesn't disallow it, but we might
be
> assuming all abreviations start with a full record, which would make the
> first operand never be an array or a blob.
>
> The bug comes from r181639 (
>
http://llvm.org/klaus/llvm/commit/1197e38f3338b8db76f0fa38c2687c65b2bcea5c/),
> which took the code to read the first argument and put it outside of the
> loop, but didn't take the Array/Blob verification + reading code too
(It's
> a bug because that commit was supposed to not have changed functionality
> :-) ).
>
> This could be “fixed” with, either a report_fatal_error (if we eventually
> have better error handling on that code, we can make that non-fatal and
> report to the caller), or by hoisting the Array/Blob reading code out of
> the loop too (actually, write a helper function).
>
> What should be done about this?
>
> Thanks,
>
>   Filipe
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150123/6d1a3f15/attachment.html>

Ok, I'll submit a patch to turn that into a report_fatal_error saying you
can't start an abbrev with an array or blob.

Thanks,

  Filipe

  F

On Fri, Jan 23, 2015 at 2:12 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:
> The restriction looks reasonable: A record starts with a code. The code
> can be encoded as a literal or be part of the abbreviation.
>
> There is probably not a lot of value in supporting a code embedded in the
> first element of an array or blob.
>
> On 23 January 2015 at 16:47, Filipe Cabecinhas <filcab at gmail.com>
wrote:
>
>> Hi all!
>>
>> Fuzzing llvm's bitcode reader, I found a problem where the reader
assumes
>> that the first field in an abbreviation will not be an array or a blob
(and
>> asserts otherwise).
>>
>> I don't know if this is expected (but not documented) or not. The
>> documentation, to me, reads like it doesn't disallow it, but we
might be
>> assuming all abreviations start with a full record, which would make
the
>> first operand never be an array or a blob.
>>
>> The bug comes from r181639 (
>>
http://llvm.org/klaus/llvm/commit/1197e38f3338b8db76f0fa38c2687c65b2bcea5c/),
>> which took the code to read the first argument and put it outside of
the
>> loop, but didn't take the Array/Blob verification + reading code
too (It's
>> a bug because that commit was supposed to not have changed
functionality
>> :-) ).
>>
>> This could be “fixed” with, either a report_fatal_error (if we
eventually
>> have better error handling on that code, we can make that non-fatal and
>> report to the caller), or by hoisting the Array/Blob reading code out
of
>> the loop too (actually, write a helper function).
>>
>> What should be done about this?
>>
>> Thanks,
>>
>>   Filipe
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150123/0e6bdedb/attachment.html>