Fix PR25101 - Handle anonymous functions without VST entries

[tejohnson]

Summary

The change to use the VST function entries for lazy deserialization did
not handle the case of anonymous functions without aliases. In that case
we must fall back to scanning the function blocks as there is no VST
entry.

[• tstellarAMD]

I have tested and verified that this fixes PR25101, but I have not reviewed the code.

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

 
 class BitcodeReader : public GVMaterializer {
   LLVMContext &Context;
   DiagnosticHandlerFunction DiagnosticHandler;
   Module *TheModule = nullptr;
   std::unique_ptr<MemoryBuffer> Buffer;
   std::unique_ptr<BitstreamReader> StreamFile;
   BitstreamCursor Stream;
+  // Next offset to start scanning for lazy parsing of function bodies.
   uint64_t NextUnreadBit = 0;
+  // Last function offset found in the VST.
+  uint64_t LastFunctionBlockBit = 0;
   bool SeenValueSymbolTable = false;
   unsigned VSTOffset = 0;
 
   std::vector<Type*> TypeList;
   BitcodeReaderValueList ValueList;
   BitcodeReaderMDValueList MDValueList;
   std::vector<Comdat *> ComdatList;
   SmallVector<Instruction *, 64> InstructionList;
Value *getValueSigned(SmallVectorImpl<uint64_t> &Record, unsigned Slot,
     return getFnValueByID(ValNo, Ty, OC);
   }
 
   /// Converts alignment exponent (i.e. power of two (or zero)) to the
   /// corresponding alignment to use. If alignment is too large, returns
   /// a corresponding error code.
   std::error_code parseAlignmentValue(uint64_t Exponent, unsigned &Alignment);
   std::error_code parseAttrKind(uint64_t Code, Attribute::AttrKind *Kind);
-  std::error_code parseModule(bool Resume, bool ShouldLazyLoadMetadata = false);
+  std::error_code parseModule(uint64_t ResumeBit,
+                              bool ShouldLazyLoadMetadata = false);
   std::error_code parseAttributeBlock();
   std::error_code parseAttributeGroupBlock();
   std::error_code parseTypeTable();
   std::error_code parseTypeTableBody();
   std::error_code parseOperandBundleTags();
 
   ErrorOr<Value *> recordValue(SmallVectorImpl<uint64_t> &Record,
                                unsigned NameIndex, Triple &TT);
   std::error_code parseValueSymbolTable(unsigned Offset = 0);
   std::error_code parseConstants();
+  std::error_code rememberAndSkipFunctionBodies();
   std::error_code rememberAndSkipFunctionBody();
   /// Save the positions of the Metadata blocks and skip parsing the blocks.
   std::error_code rememberAndSkipMetadata();
   std::error_code parseFunctionBody(Function *F);
   std::error_code globalCleanup();
   std::error_code resolveGlobalAndAliasInits();
   std::error_code parseMetadata();
   std::error_code parseMetadataAttachment(Function &F);
case bitc::VST_CODE_FNENTRY: {
         assert(GO);
       }
 
       uint64_t FuncWordOffset = Record[1];
       Function *F = dyn_cast<Function>(GO);
       assert(F);
       uint64_t FuncBitOffset = FuncWordOffset * 32;
       DeferredFunctionInfo[F] = FuncBitOffset + FuncBitcodeOffsetDelta;
-      // Set the NextUnreadBit to point to the last function block.
+      // 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 > NextUnreadBit)
-        NextUnreadBit = FuncBitOffset;
+      if (FuncBitOffset > LastFunctionBlockBit)
+        LastFunctionBlockBit = FuncBitOffset;
       break;
     }
     case bitc::VST_CODE_BBENTRY: {
       if (convertToString(Record, 1, ValueName))
         return error("Invalid record");
       BasicBlock *BB = getBasicBlock(Record[0]);
       if (!BB)
         return error("Invalid record");
std::error_code BitcodeReader::rememberAndSkipFunctionBody() {
   if (FunctionsWithBodies.empty())
     return error("Insufficient function protos");
 
   Function *Fn = FunctionsWithBodies.back();
   FunctionsWithBodies.pop_back();
 
   // Save the current stream state.
   uint64_t CurBit = Stream.GetCurrentBitNo();
+  assert(
+      (DeferredFunctionInfo[Fn] == 0 || DeferredFunctionInfo[Fn] == CurBit) &&
+      "Mismatch between VST and scanned function offsets");
   DeferredFunctionInfo[Fn] = CurBit;
 
   // Skip over the function block for now.
   if (Stream.SkipBlock())
     return error("Invalid record");
   return std::error_code();
 }
 
std::error_code BitcodeReader::globalCleanup() {
 
   // Force deallocation of memory for these vectors to favor the client that
   // want lazy deserialization.
   std::vector<std::pair<GlobalVariable*, unsigned> >().swap(GlobalInits);
   std::vector<std::pair<GlobalAlias*, unsigned> >().swap(AliasInits);
   return std::error_code();
 }
 
-std::error_code BitcodeReader::parseModule(bool Resume,
-                                           bool ShouldLazyLoadMetadata) {
-  if (Resume)
-    Stream.JumpToBit(NextUnreadBit);
+/// Support for lazy parsing of function bodies. This is required if we
+/// either have an old bitcode file without a VST forward declaration record,
+/// or if we have an anonymous function being materialized, since anonymous
+/// functions do not have a name and are therefore not in the VST.
+std::error_code BitcodeReader::rememberAndSkipFunctionBodies() {
+
+  if (Stream.AtEndOfStream()) return error("Could not find function in stream");
+
+  assert(SeenFirstFunctionBody);
+  // An old bitcode file with the symbol table at the end would have
+  // finished the parse greedily.
+  assert(SeenValueSymbolTable);
+
+  SmallVector<uint64_t, 64> Record;
+
+  while (1) {
+    BitstreamEntry Entry = Stream.advance();
+    switch (Entry.Kind) {
+      default:
+        return error("Expect SubBlock");
+      case BitstreamEntry::SubBlock:
+        switch (Entry.ID) {
+          default:
+            return error("Expect function block");
+          case bitc::FUNCTION_BLOCK_ID:
+            if (std::error_code EC = rememberAndSkipFunctionBody()) return EC;
+            NextUnreadBit = Stream.GetCurrentBitNo();
+            return std::error_code();
+        }
+    }
+  }
+}
+
+std::error_code BitcodeReader::parseModule(uint64_t ResumeBit,
+                                           bool ShouldLazyLoadMetadata) {
+  if (ResumeBit)
+    Stream.JumpToBit(ResumeBit);
   else if (Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
     return error("Invalid record");
 
   SmallVector<uint64_t, 64> Record;
   std::vector<std::string> SectionTable;
   std::vector<std::string> GCTable;
 
   // Read all the records for this module.
case BitstreamEntry::SubBlock:
           // If we have a VST forward declaration record, make sure we
           // parse the VST now if we haven't already. It is needed to
           // set up the DeferredFunctionInfo vector for lazy reading.
           if (!SeenValueSymbolTable) {
             if (std::error_code EC =
                     BitcodeReader::parseValueSymbolTable(VSTOffset))
               return EC;
             SeenValueSymbolTable = true;
-            return std::error_code();
+            // Fall through so that we record the NextUnreadBit below.
+            // This is necessary in case we have an anonymous function that
+            // is later materialized. Since it will not have a VST entry we
+            // need to fall back to the lazy parse to find its offset.
           } else {
             // If we have a VST forward declaration record, but have already
             // parsed the VST (just above, when the first function body was
             // encountered here), then we are resuming the parse after
-            // materializing functions. The NextUnreadBit points to the start
-            // of the last function block recorded in the VST (set when
-            // parsing the VST function entries). Skip it.
+            // materializing functions. The ResumeBit points to the
+            // start of the last function block recorded in the
+            // DeferredFunctionInfo map. Skip it.
             if (Stream.SkipBlock())
               return error("Invalid record");
             continue;
           }
         }
 
         // Support older bitcode files that did not have the function
-        // index in the VST, nor a VST forward declaration record.
+        // index in the VST, nor a VST forward declaration record, as
+        // well as anonymous functions that do not have VST entries.
         // Build the DeferredFunctionInfo vector on the fly.
         if (std::error_code EC = rememberAndSkipFunctionBody())
           return EC;
+
         // Suspend parsing when we reach the function bodies. Subsequent
         // materialization calls will resume it when necessary. If the bitcode
         // file is old, the symbol table will be at the end instead and will not
         // have been seen yet. In this case, just finish the parse now.
         if (SeenValueSymbolTable) {
           NextUnreadBit = Stream.GetCurrentBitNo();
           return std::error_code();
         }
while (1) {
 
     BitstreamEntry Entry =
       Stream.advance(BitstreamCursor::AF_DontAutoprocessAbbrevs);
 
     if (Entry.Kind != BitstreamEntry::SubBlock)
       return error("Malformed block");
 
     if (Entry.ID == bitc::MODULE_BLOCK_ID)
-      return parseModule(false, ShouldLazyLoadMetadata);
+      return parseModule(0, ShouldLazyLoadMetadata);
 
     if (Stream.SkipBlock())
       return error("Invalid record");
   }
 }
 
 ErrorOr<std::string> BitcodeReader::parseModuleTriple() {
   if (Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))

 }
 
 /// Find the function body in the bitcode stream
 std::error_code BitcodeReader::findFunctionInStream(
     Function *F,
     DenseMap<Function *, uint64_t>::iterator DeferredFunctionInfoIterator) {
   while (DeferredFunctionInfoIterator->second == 0) {
     // This is the fallback handling for the old format bitcode that
-    // didn't contain the function index in the VST. Assert if we end up
-    // here for the new format (which is the only time the VSTOffset would
-    // be non-zero).
-    assert(VSTOffset == 0);
-    if (Stream.AtEndOfStream())
-      return error("Could not find function in stream");
-    // ParseModule will parse the next body in the stream and set its
-    // position in the DeferredFunctionInfo map.
-    if (std::error_code EC = parseModule(true))
-      return EC;
+    // didn't contain the function index in the VST, or when we have
+    // an anonymous function which would not have a VST entry.
+    // Assert that we have one of those two cases.
+    assert(VSTOffset == 0 || !F->hasName());
+    // Parse the next body in the stream and set its position in the
+    // DeferredFunctionInfo map.
+    if (std::error_code EC = rememberAndSkipFunctionBodies()) return EC;
   }
   return std::error_code();
 }
 
 //===----------------------------------------------------------------------===//
 // GVMaterializer implementation
 //===----------------------------------------------------------------------===//
 
std::error_code BitcodeReader::materializeModule(Module *M) {
 
   // Iterate over the module, deserializing any functions that are still on
   // disk.
   for (Module::iterator F = TheModule->begin(), E = TheModule->end();
        F != E; ++F) {
     if (std::error_code EC = materialize(F))
       return EC;
   }
-  // At this point, if there are any function bodies, the current bit is
-  // pointing to the END_BLOCK record after them. Now make sure the rest
-  // of the bits in the module have been read.
-  if (NextUnreadBit)
-    parseModule(true);
+  // At this point, if there are any function bodies, parse the rest of
+  // the bits in the module past the last function block we have recorded
+  // through either lazy scanning or the VST.
+  if (LastFunctionBlockBit || NextUnreadBit)
+    parseModule(LastFunctionBlockBit > NextUnreadBit ? LastFunctionBlockBit
+                                                     : NextUnreadBit);
 
   // Check that all block address forward references got resolved (as we
   // promised above).
   if (!BasicBlockFwdRefs.empty())
     return error("Never resolved function from blockaddress");
 
   // Upgrade any intrinsic calls that slipped through (should not happen!) and
   // delete the old functions to clean up. We can't do this unless the entire

llvm/trunk/test/Bitcode/anon-functions.ll

+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+; PR25101
+
+; CHECK: define void @0()
+define void @0() {
+  ret void
+}
+
+; CHECK: define void @f()
+define void @f() {
+  ret void
+}
+
+; CHECK: define void @1()
+define void @1() {
+  ret void
+}
+