[PATCH] D86957: [Bitstream] Use alignTo to make code more readable. NFC

[stephan.yichao.zhao via Phabricator via llvm-commits]

stephan.yichao.zhao added a comment.

In D86957#2250225 <https://reviews.llvm.org/D86957#2250225>, @craig.topper wrote:

> In D86957#2250193 <https://reviews.llvm.org/D86957#2250193>, @stephan.yichao.zhao wrote:
>
>> So this does both 64bit cast and alignment. Thank you.
>>
>> Hi Craig, in your case, will NumElts be actually larger than 2^32? NumElts is read from https://llvm.org/docs/BitCodeFormat.html#enter-subblock-encoding that defines the blocklen to be 32bit. So when it gets larger, the overflow can happen at the writer side (https://llvm.org/doxygen/BitstreamWriter_8h_source.html#l00384).
>
> My specific case was a blob for metadata strings that was ~1GB in size. The multipy by 8 to convert its size to bits was overflowing. I do worry that it might break again if the blob of metadata strings exceed 4GB.

The case I fixed is similar. One way to address is to extend that blocklen field to 64bit. imo this does not introduce any back-compatibility issue because 32 is not a fixed width, but VBR.

1. when an old reader reads a bitcode written by a new writer, it works if blocklen is <= 2^32. Although it gets broken if blocklen is > 2^32, this case it does not work anyway.
2. when a new reader reads a bitcode written by an old writer, it works fine since blocklen is <= 2^32.

So it is possible to extend it to 64bit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86957/new/

https://reviews.llvm.org/D86957