[PATCH] D20936: [pdb] Parse module line info

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 2 23:23:40 PDT 2016


size is unsigned, so it could happen if you had a stream which was exactly
2^32 bytes long, and it was an array of uint8's.  :-/  Not that that would
ever happen.  I think Rui has a patch up which changes this a little, so
I'll wait until that goes in.

On Thu, Jun 2, 2016 at 11:07 PM David Majnemer <david.majnemer at gmail.com>
wrote:

>
>
> On Thursday, June 2, 2016, Zachary Turner <zturner at google.com> wrote:
>
>> zturner added inline comments.
>>
>> ================
>> Comment at: include/llvm/DebugInfo/CodeView/ModuleSubstreamVisitor.h:43
>> @@ +42,3 @@
>> +        (sizeof(LineNumberEntry) + (HasColumn ?
>> sizeof(ColumnNumberEntry) : 0));
>> +    uint32_t Size = BlockHeader->BlockSize - sizeof(LineFileBlockHeader);
>> +    if (LineInfoSize > Size)
>> ----------------
>> majnemer wrote:
>> > What if BlockSize is smaller than LineFileBlockHeader?
>> That would indicate a corrupt record.  So yea, I should check for that.
>>
>> ================
>> Comment at: include/llvm/DebugInfo/CodeView/StreamArray.h:213
>> @@ +212,3 @@
>> +      : Array(Array), Index(ArrayIndex) {
>> +    if (Array.size() <= Index)
>> +      Index = uint32_t(-1);
>> ----------------
>> majnemer wrote:
>> > Should it be an error for Array.size() to be -1?
>> If it were easy to make it work, then we should, but to do that we'd
>> probably need to increase the size of the iterator, and I'm not sure it's
>> worth it.   I think the only way that can ever happen is if your StreamRef
>> is 2^32-1 bytes long, and I don't know that it's even possible to have a
>> stream that big (I recall there are constants somewhere in the cv header
>> files that determine how big a stream can be based on the block size and
>> whether large blocks are enabled).  I can look into it a bit more when I
>> get home, but this would be a pretty exceptional circumstance.
>>
>>
> It should be impossible because a stream size of -1 means "no stream"
>
> I think an assert would be sufficient
>
>
>>
>> http://reviews.llvm.org/D20936
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160603/53f2b7d4/attachment.html>


More information about the llvm-commits mailing list