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

[David Majnemer via llvm-commits]

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/20160602/b490db7b/attachment.html>