Bitcode: Change expected layout of module blocks.

[pcc]

Summary

We now expect each module's identification block to appear immediately before
the module block. Any module block that appears without an identification block
immediately before it is interpreted as if it does not have a module block.

Also change the interpretation of VST and function offsets in bitcode.
The offset is always taken as relative to the start of the identification
(or module if not present) block, minus one word. This corresponds to the
historical interpretation of offsets, i.e. relative to the start of the file.

These changes allow for bitcode modules to be concatenated by copying bytes.

[mehdi_amini]

LGTM!
Thanks.

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

2311 ↗

(On Diff #79492)

Is "historically always the start of the file" correct on Darwin?

[pcc]

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

2311 ↗

(On Diff #79492)

I suppose not, fixed.

llvm/trunk/include/llvm/Bitcode/BitcodeReader.h

namespace llvm {
   ErrorOr<T> expectedToErrorOrAndEmitErrors(LLVMContext &Ctx, Expected<T> Val) {
     if (!Val)
       return errorToErrorCodeAndEmitErrors(Ctx, Val.takeError());
     return std::move(*Val);
   }
 
   /// Represents a module in a bitcode file.
   class BitcodeModule {
+    // This covers the identification (if present) and module blocks.
     ArrayRef<uint8_t> Buffer;
     StringRef ModuleIdentifier;
 
     // The bitstream location of the IDENTIFICATION_BLOCK.
     uint64_t IdentificationBit;
 
     // The bitstream location of this module's MODULE_BLOCK.
     uint64_t ModuleBit;

llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp

case bitc::VST_CODE_FNENTRY: {
         // If this is an alias, need to get the actual Function object
         // it aliases, in order to set up the DeferredFunctionInfo entry below.
         auto *GA = dyn_cast<GlobalAlias>(V);
         if (GA)
           GO = GA->getBaseObject();
         assert(GO);
       }
 
-      uint64_t FuncWordOffset = Record[1];
+      // Note that we subtract 1 here because the offset is relative to one word
+      // before the start of the identification or module block, which was
+      // historically always the start of the regular bitcode header.
+      uint64_t FuncWordOffset = Record[1] - 1;
       Function *F = dyn_cast<Function>(GO);
       assert(F);
       uint64_t FuncBitOffset = FuncWordOffset * 32;
       DeferredFunctionInfo[F] = FuncBitOffset + FuncBitcodeOffsetDelta;
       // Set the LastFunctionBlockBit to point to the last function block.
       // Later when parsing is resumed after function materialization,
       // we can simply skip that last function block.
       if (FuncBitOffset > LastFunctionBlockBit)
case bitc::MODULE_CODE_PURGEVALS:
       if (Record.size() < 1 || Record[0] > ValueList.size())
         return error("Invalid record");
       ValueList.shrinkTo(Record[0]);
       break;
     /// MODULE_CODE_VSTOFFSET: [offset]
     case bitc::MODULE_CODE_VSTOFFSET:
       if (Record.size() < 1)
         return error("Invalid record");
-      VSTOffset = Record[0];
+      // Note that we subtract 1 here because the offset is relative to one word
+      // before the start of the identification or module block, which was
+      // historically always the start of the regular bitcode header.
+      VSTOffset = Record[0] - 1;
       break;
     /// MODULE_CODE_SOURCE_FILENAME: [namechar x N]
     case bitc::MODULE_CODE_SOURCE_FILENAME:
       SmallString<128> ValueName;
       if (convertToString(Record, 0, ValueName))
         return error("Invalid record");
       TheModule->setSourceFileName(ValueName);
       break;

 
 Expected<std::vector<BitcodeModule>>
 llvm::getBitcodeModuleList(MemoryBufferRef Buffer) {
   Expected<BitstreamCursor> StreamOrErr = initStream(Buffer);
   if (!StreamOrErr)
     return StreamOrErr.takeError();
   BitstreamCursor &Stream = *StreamOrErr;
 
-  uint64_t IdentificationBit = -1ull;
   std::vector<BitcodeModule> Modules;
   while (true) {
+    uint64_t BCBegin = Stream.getCurrentByteNo();
+
     // We may be consuming bitcode from a client that leaves garbage at the end
     // of the bitcode stream (e.g. Apple's ar tool). If we are close enough to
     // the end that there cannot possibly be another module, stop looking.
-    if (Stream.getCurrentByteNo() + 8 >= Stream.getBitcodeBytes().size())
+    if (BCBegin + 8 >= Stream.getBitcodeBytes().size())
       return Modules;
 
     BitstreamEntry Entry = Stream.advance();
     switch (Entry.Kind) {
     case BitstreamEntry::EndBlock:
     case BitstreamEntry::Error:
       return error("Malformed block");
 
-    case BitstreamEntry::SubBlock:
-      if (Entry.ID == bitc::IDENTIFICATION_BLOCK_ID)
-        IdentificationBit = Stream.GetCurrentBitNo();
-      else if (Entry.ID == bitc::MODULE_BLOCK_ID)
-        Modules.push_back({Stream.getBitcodeBytes(),
+    case BitstreamEntry::SubBlock: {
+      uint64_t IdentificationBit = -1ull;
+      if (Entry.ID == bitc::IDENTIFICATION_BLOCK_ID) {
+        IdentificationBit = Stream.GetCurrentBitNo() - BCBegin * 8;
+        if (Stream.SkipBlock())
+          return error("Malformed block");
+
+        Entry = Stream.advance();
+        if (Entry.Kind != BitstreamEntry::SubBlock ||
+            Entry.ID != bitc::MODULE_BLOCK_ID)
+          return error("Malformed block");
+      }
+
+      if (Entry.ID == bitc::MODULE_BLOCK_ID) {
+        uint64_t ModuleBit = Stream.GetCurrentBitNo() - BCBegin * 8;
+        if (Stream.SkipBlock())
+          return error("Malformed block");
+
+        Modules.push_back({Stream.getBitcodeBytes().slice(
+                               BCBegin, Stream.getCurrentByteNo() - BCBegin),
                            Buffer.getBufferIdentifier(), IdentificationBit,
-                           Stream.GetCurrentBitNo()});
+                           ModuleBit});
+        continue;
+      }
 
       if (Stream.SkipBlock())
         return error("Malformed block");
       continue;
+    }
     case BitstreamEntry::Record:
       Stream.skipRecord(Entry.ID);
       continue;
     }
   }
 }
 
 /// \brief Get a lazy one-at-time loading module from bitcode.