Fix a 32-bit overflow issue when reading LTO-generated bitcode files whose strtab are of size > 2^29

[stephan.yichao.zhao]

Summary

This happens when using -flto and -Wl,--plugin-opt=emit-llvm to create a linked LTO bitcode file, and the bitcode file has a strtab with size > 2^29.

The code path is GetBitcodeFileContents->readBlobInRecord->readRecord

All the changes relate to a pattern like this

size_t x64 = y64 + z32 * C

When z32 is >= (2^32)/C, z32 * C overflows.

[MaskRay]

LGTM. This only affects very large bitcode files. I don't know how to create a reasonable small test for this. Worth waiting one day or so before committing to give others some time to respond.

[MaskRay]

llvm/lib/Bitstream/Reader/BitstreamReader.cpp

190–191

const size_t while you are updating it

[stephan.yichao.zhao]

addressed comments

llvm/lib/Bitstream/Reader/BitstreamReader.cpp

if (Op.getEncoding() == BitCodeAbbrevOp::Array) {
 
       // Read all the elements.
       // Decode the value as we are commanded.
       switch (EltEnc.getEncoding()) {
       default:
         report_fatal_error("Array element type can't be an Array or a Blob");
       case BitCodeAbbrevOp::Fixed:
         assert((unsigned)EltEnc.getEncodingData() <= MaxChunkSize);
-        if (Error Err = JumpToBit(GetCurrentBitNo() +
-                                  NumElts * EltEnc.getEncodingData()))
+        if (Error Err =
+                JumpToBit(GetCurrentBitNo() + static_cast<uint64_t>(NumElts) *
+                                                  EltEnc.getEncodingData()))
           return std::move(Err);
         break;
       case BitCodeAbbrevOp::VBR:
         assert((unsigned)EltEnc.getEncodingData() <= MaxChunkSize);
         for (; NumElts; --NumElts)
           if (Expected<uint64_t> Res =
                   ReadVBR64((unsigned)EltEnc.getEncodingData()))
             ; // Skip!
for (unsigned i = 1, e = Abbv->getNumOperandInfos(); i < e; ++i) {
     // Blob case.  Read the number of bytes as a vbr6.
     Expected<uint32_t> MaybeNum = ReadVBR(6);
     if (!MaybeNum)
       return MaybeNum.takeError();
     unsigned NumElts = MaybeNum.get();
     SkipToFourByteBoundary();  // 32-bit alignment
 
     // Figure out where the end of this blob will be including tail padding.
-    size_t NewEnd = GetCurrentBitNo()+((NumElts+3)&~3)*8;
+    const size_t NewEnd =
+        GetCurrentBitNo() + ((static_cast<uint64_t>(NumElts) + 3) & ~3) * 8;
 
     // If this would read off the end of the bitcode file, just set the
     // record to empty and return.
     if (!canSkipToPos(NewEnd/8)) {
       skipToEnd();
       break;
     }
 
for (unsigned i = 1, e = Abbv->getNumOperandInfos(); i != e; ++i) {
     Expected<uint32_t> MaybeNumElts = ReadVBR(6);
     if (!MaybeNumElts)
       return MaybeNumElts.takeError();
     uint32_t NumElts = MaybeNumElts.get();
     SkipToFourByteBoundary();  // 32-bit alignment
 
     // Figure out where the end of this blob will be including tail padding.
     size_t CurBitPos = GetCurrentBitNo();
-    size_t NewEnd = CurBitPos+((NumElts+3)&~3)*8;
+    const size_t NewEnd =
+        CurBitPos + ((static_cast<uint64_t>(NumElts) + 3) & ~3) * 8;
 
     // If this would read off the end of the bitcode file, just set the
     // record to empty and return.
     if (!canSkipToPos(NewEnd/8)) {
       Vals.append(NumElts, 0);
       skipToEnd();
       break;
     }