Place control block after AST block in PCM files (Patch series 1/3)

[dang]

Summary

This places the control block after the AST block in PCMs so that the AST block
can be more easily hashed.

The next patch is at D79998

[dang]

After some further discussion offline with @Bigcheese the cleaner course of action is to make the AST block relocatable see revision D80383

clang/include/clang/Serialization/ASTWriter.h

private:
   /// A mapping from each known submodule to its ID number, which will
   /// be a positive integer.
   llvm::DenseMap<Module *, unsigned> SubmoduleIDs;
 
   /// A list of the module file extension writers.
   std::vector<std::unique_ptr<ModuleFileExtensionWriter>>
     ModuleFileExtensionWriters;
 
+  /// An input file.
+  struct InputFileEntry {
+    const FileEntry *File;
+    bool IsSystemFile;
+    bool IsTransient;
+    bool BufferOverridden;
+    bool IsTopLevelModuleMap;
+    uint32_t ContentHash[2];
+  };
+
+  /// All the ContentCache objects for files, sorted by whether the file is a
+  /// system one or not. System files go at the back, user files at the front.
+  std::deque<InputFileEntry> SortedInputFiles;
+
+  /// Generate IDs for the input files
+  void populateInputFileIDs(SourceManager &SourceMgr);
+
   /// Retrieve or create a submodule ID for this module.
   unsigned getSubmoduleID(Module *Mod);
 
   /// Write the given subexpression to the bitstream.
   void WriteSubStmt(Stmt *S);
 
   void WriteBlockInfoBlock();
   void WriteControlBlock(Preprocessor &PP, ASTContext &Context,

clang/lib/Serialization/ASTReader.cpp

 
 ASTReader::ASTReadResult
 ASTReader::ReadControlBlock(ModuleFile &F,
                             SmallVectorImpl<ImportedModule> &Loaded,
                             const ModuleFile *ImportedBy,
                             unsigned ClientLoadCapabilities) {
   BitstreamCursor &Stream = F.Stream;
 
-  if (llvm::Error Err = Stream.EnterSubBlock(CONTROL_BLOCK_ID)) {
-    Error(std::move(Err));
-    return Failure;
-  }
-
   // Lambda to read the unhashed control block the first time it's called.
   //
   // For PCM files, the unhashed control block cannot be read until after the
   // MODULE_NAME record.  However, PCH files have no MODULE_NAME, and yet still
   // need to look ahead before reading the IMPORTS record.  For consistency,
   // this block is always read somehow (see BitstreamEntry::EndBlock).
   bool HasReadUnhashedControlBlock = false;
   auto readUnhashedControlBlockOnce = [&]() {
ASTReader::ReadASTCore(StringRef FileName,
 
   // Sniff for the signature.
   if (llvm::Error Err = doesntStartWithASTFileMagic(Stream)) {
     Diag(diag::err_module_file_invalid)
         << moduleKindForDiagnostic(Type) << FileName << std::move(Err);
     return Failure;
   }
 
-  // This is used for compatibility with older PCH formats.
-  bool HaveReadControlBlock = false;
-  while (true) {
-    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
-    if (!MaybeEntry) {
-      Error(MaybeEntry.takeError());
-      return Failure;
-    }
-    llvm::BitstreamEntry Entry = MaybeEntry.get();
-
-    switch (Entry.Kind) {
-    case llvm::BitstreamEntry::Error:
-    case llvm::BitstreamEntry::Record:
-    case llvm::BitstreamEntry::EndBlock:
-      Error("invalid record at top-level of AST file");
+  {
+    // The control block needs to be read first but the AST block comes before
+    // it in the PCM, so lookahead to the control block and read it now.
+    BitstreamCursor &PreviousCursor = F.Stream;
+    SavedStreamPosition SavedPosition(PreviousCursor);
+    if (SkipCursorToBlock(Stream, CONTROL_BLOCK_ID))
       return Failure;
-
-    case llvm::BitstreamEntry::SubBlock:
-      break;
-    }
-
-    switch (Entry.ID) {
-    case CONTROL_BLOCK_ID:
-      HaveReadControlBlock = true;
-      switch (ReadControlBlock(F, Loaded, ImportedBy, ClientLoadCapabilities)) {
-      case Success:
-        // Check that we didn't try to load a non-module AST file as a module.
-        //
-        // FIXME: Should we also perform the converse check? Loading a module as
-        // a PCH file sort of works, but it's a bit wonky.
-        if ((Type == MK_ImplicitModule || Type == MK_ExplicitModule ||
-             Type == MK_PrebuiltModule) &&
-            F.ModuleName.empty()) {
-          auto Result = (Type == MK_ImplicitModule) ? OutOfDate : Failure;
-          if (Result != OutOfDate ||
-              (ClientLoadCapabilities & ARR_OutOfDate) == 0)
-            Diag(diag::err_module_file_not_module) << FileName;
-          return Result;
-        }
-        break;
 
-      case Failure: return Failure;
-      case Missing: return Missing;
-      case OutOfDate: return OutOfDate;
-      case VersionMismatch: return VersionMismatch;
-      case ConfigurationMismatch: return ConfigurationMismatch;
-      case HadErrors: return HadErrors;
-      }
-      break;
+    case Failure:
+      return Failure;
+    case Missing:
+      return Missing;
+    case OutOfDate:
+      return OutOfDate;
+    case VersionMismatch:
+      return VersionMismatch;
+    case ConfigurationMismatch:
+      return ConfigurationMismatch;
+    case HadErrors:
+      return HadErrors;
+    }
 
-    case AST_BLOCK_ID:
-      if (!HaveReadControlBlock) {
-        if ((ClientLoadCapabilities & ARR_VersionMismatch) == 0)
-          Diag(diag::err_pch_version_too_old);
-        return VersionMismatch;
-      }
+  while (true) {
+    Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
+    if (!MaybeEntry) {
+      Error(MaybeEntry.takeError());
+      return Failure;
+    }
+    llvm::BitstreamEntry Entry = MaybeEntry.get();
+
+    switch (Entry.Kind) {
+    case llvm::BitstreamEntry::Error:
+    case llvm::BitstreamEntry::Record:
+    case llvm::BitstreamEntry::EndBlock:
+      Error("invalid record at top-level of AST file");
+      return Failure;
+
+    case llvm::BitstreamEntry::SubBlock:
+      break;
 
+    switch (Entry.ID) {
+    case AST_BLOCK_ID:
       // Record that we've loaded this module.
       Loaded.push_back(ImportedModule(M, ImportedBy, ImportLoc));
       ShouldFinalizePCM = true;
       return Success;
 
     case UNHASHED_CONTROL_BLOCK_ID:
       // This block is handled using look-ahead during ReadControlBlock.  We
       // shouldn't get here!

clang/lib/Serialization/ASTWriter.cpp

 //===- ASTWriter.cpp - AST File Writer ------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines the ASTWriter class, which writes AST files.
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/AST/OpenMPClause.h"
-#include "clang/Serialization/ASTRecordWriter.h"
 #include "ASTCommon.h"
 #include "ASTReaderInternals.h"
 #include "MultiOnDiskHashTable.h"
-#include "clang/AST/AbstractTypeWriter.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTUnresolvedSet.h"
+#include "clang/AST/AbstractTypeWriter.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclContextInternals.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLocVisitor.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"

 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/Token.h"
 #include "clang/Sema/IdentifierResolver.h"
 #include "clang/Sema/ObjCMethodList.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/Weak.h"
 #include "clang/Serialization/ASTReader.h"
+#include "clang/Serialization/ASTRecordWriter.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
 #include "clang/Serialization/ModuleFile.h"
 #include "clang/Serialization/ModuleFileExtension.h"
 #include "clang/Serialization/SerializationDiagnostic.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Bitstream/BitCodes.h"
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "llvm/Support/Casting.h"
if (WritingModule) {
     Abbrev->Add(BitCodeAbbrevOp(MODULE_NAME));
     Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
     unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev));
     RecordData::value_type Record[] = {MODULE_NAME};
     Stream.EmitRecordWithBlob(AbbrevCode, Record, WritingModule->Name);
   }
 
   if (WritingModule && WritingModule->Directory) {
-    SmallString<128> BaseDir(WritingModule->Directory->getName());
-    cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
-
     // If the home of the module is the current working directory, then we
     // want to pick up the cwd of the build process loading the module, not
     // our cwd, when we load this module.
     if (!PP.getHeaderSearchInfo()
              .getHeaderSearchOpts()
              .ModuleMapFileHomeIsCwd ||
         WritingModule->Directory->getName() != StringRef(".")) {
       // Module directory.
       auto Abbrev = std::make_shared<BitCodeAbbrev>();
       Abbrev->Add(BitCodeAbbrevOp(MODULE_DIRECTORY));
       Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Directory
       unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev));
 
       RecordData::value_type Record[] = {MODULE_DIRECTORY};
-      Stream.EmitRecordWithBlob(AbbrevCode, Record, BaseDir);
+      Stream.EmitRecordWithBlob(AbbrevCode, Record, BaseDirectory);
     }
-
-    // Write out all other paths relative to the base directory if possible.
-    BaseDirectory.assign(BaseDir.begin(), BaseDir.end());
-  } else if (!isysroot.empty()) {
-    // Write out paths relative to the sysroot if possible.
-    BaseDirectory = std::string(isysroot);
   }
 
   // Module map file
   if (WritingModule && WritingModule->Kind == Module::ModuleMapModule) {
     Record.clear();
 
     auto &Map = PP.getHeaderSearchInfo().getModuleMap();
     AddPath(WritingModule->PresumedModuleMapFile.empty()
#include "clang/Basic/Sanitizers.def"
   }
 
   WriteInputFiles(Context.SourceMgr,
                   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
                   PP.getLangOpts().Modules);
   Stream.ExitBlock();
 }
 
-namespace  {
-
-/// An input file.
-struct InputFileEntry {
-  const FileEntry *File;
-  bool IsSystemFile;
-  bool IsTransient;
-  bool BufferOverridden;
-  bool IsTopLevelModuleMap;
-  uint32_t ContentHash[2];
-};
-
-} // namespace
-
 void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
-                                HeaderSearchOptions &HSOpts,
-                                bool Modules) {
+                                HeaderSearchOptions &HSOpts, bool Modules) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
 
   // Create input-file abbreviation.
   auto IFAbbrev = std::make_shared<BitCodeAbbrev>();
   IFAbbrev->Add(BitCodeAbbrevOp(INPUT_FILE));
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // ID
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 12)); // Size
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 32)); // Modification time
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Overridden
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Transient
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Module map
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // File name
   unsigned IFAbbrevCode = Stream.EmitAbbrev(std::move(IFAbbrev));
 
   // Create input file hash abbreviation.
   auto IFHAbbrev = std::make_shared<BitCodeAbbrev>();
   IFHAbbrev->Add(BitCodeAbbrevOp(INPUT_FILE_HASH));
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
-  // Get all ContentCache objects for files, sorted by whether the file is a
-  // system one or not. System files go at the back, users files at the front.
-  std::deque<InputFileEntry> SortedFiles;
-  for (unsigned I = 1, N = SourceMgr.local_sloc_entry_size(); I != N; ++I) {
-    // Get this source location entry.
-    const SrcMgr::SLocEntry *SLoc = &SourceMgr.getLocalSLocEntry(I);
-    assert(&SourceMgr.getSLocEntry(FileID::get(I)) == SLoc);
-
-    // We only care about file entries that were not overridden.
-    if (!SLoc->isFile())
-      continue;
-    const SrcMgr::FileInfo &File = SLoc->getFile();
-    const SrcMgr::ContentCache *Cache = File.getContentCache();
-    if (!Cache->OrigEntry)
-      continue;
-
-    InputFileEntry Entry;
-    Entry.File = Cache->OrigEntry;
-    Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
-    Entry.IsTransient = Cache->IsTransient;
-    Entry.BufferOverridden = Cache->BufferOverridden;
-    Entry.IsTopLevelModuleMap = isModuleMap(File.getFileCharacteristic()) &&
-                                File.getIncludeLoc().isInvalid();
-
-    auto ContentHash = hash_code(-1);
-    if (PP->getHeaderSearchInfo()
-            .getHeaderSearchOpts()
-            .ValidateASTInputFilesContent) {
-      auto *MemBuff = Cache->getRawBuffer();
-      if (MemBuff)
-        ContentHash = hash_value(MemBuff->getBuffer());
-      else
-        // FIXME: The path should be taken from the FileEntryRef.
-        PP->Diag(SourceLocation(), diag::err_module_unable_to_hash_content)
-            << Entry.File->getName();
-    }
-    auto CH = llvm::APInt(64, ContentHash);
-    Entry.ContentHash[0] =
-        static_cast<uint32_t>(CH.getLoBits(32).getZExtValue());
-    Entry.ContentHash[1] =
-        static_cast<uint32_t>(CH.getHiBits(32).getZExtValue());
-
-    if (Entry.IsSystemFile)
-      SortedFiles.push_back(Entry);
-    else
-      SortedFiles.push_front(Entry);
-  }
-
   unsigned UserFilesNum = 0;
   // Write out all of the input files.
   std::vector<uint64_t> InputFileOffsets;
-  for (const auto &Entry : SortedFiles) {
-    uint32_t &InputFileID = InputFileIDs[Entry.File];
-    if (InputFileID != 0)
-      continue; // already recorded this file.
-
+  for (const auto &Entry : SortedInputFiles) {
     // Record this entry's offset.
     InputFileOffsets.push_back(Stream.GetCurrentBitNo());
 
-    InputFileID = InputFileOffsets.size();
-
     if (!Entry.IsSystemFile)
       ++UserFilesNum;
 
     // Emit size/modification time for this file.
     // And whether this file was overridden.
     {
       RecordData::value_type Record[] = {
           INPUT_FILE,
-          InputFileOffsets.size(),
+          InputFileIDs[Entry.File],
           (uint64_t)Entry.File->getSize(),
           (uint64_t)getTimestampForOutput(Entry.File),
           Entry.BufferOverridden,
           Entry.IsTransient,
           Entry.IsTopLevelModuleMap};
 
       // FIXME: The path should be taken from the FileEntryRef.
       EmitRecordWithPath(IFAbbrevCode, Record, Entry.File->getName());
unsigned ASTWriter::getLocalOrImportedSubmoduleID(Module *Mod) {
   if (Top != WritingModule &&
       (getLangOpts().CompilingPCH ||
        !Top->fullModuleNameIs(StringRef(getLangOpts().CurrentModule))))
     return 0;
 
   return SubmoduleIDs[Mod] = NextSubmoduleID++;
 }
 
+void ASTWriter::populateInputFileIDs(SourceManager &SourceMgr) {
+  for (unsigned I = 1, N = SourceMgr.local_sloc_entry_size(); I != N; ++I) {
+    // Get this source location entry.
+    const SrcMgr::SLocEntry *SLoc = &SourceMgr.getLocalSLocEntry(I);
+    assert(&SourceMgr.getSLocEntry(FileID::get(I)) == SLoc);
+
+    // We only care about file entries that were not overridden.
+    if (!SLoc->isFile())
+      continue;
+    const SrcMgr::FileInfo &File = SLoc->getFile();
+    const SrcMgr::ContentCache *Cache = File.getContentCache();
+    if (!Cache->OrigEntry)
+      continue;
+
+    InputFileEntry Entry;
+    Entry.File = Cache->OrigEntry;
+    Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
+    Entry.IsTransient = Cache->IsTransient;
+    Entry.BufferOverridden = Cache->BufferOverridden;
+    Entry.IsTopLevelModuleMap = isModuleMap(File.getFileCharacteristic()) &&
+                                File.getIncludeLoc().isInvalid();
+
+    auto ContentHash = llvm::hash_code(-1);
+    if (PP->getHeaderSearchInfo()
+            .getHeaderSearchOpts()
+            .ValidateASTInputFilesContent) {
+      auto *MemBuff = Cache->getRawBuffer();
+      if (MemBuff)
+        ContentHash = hash_value(MemBuff->getBuffer());
+      else
+        // FIXME: The path should be taken from the FileEntryRef.
+        PP->Diag(SourceLocation(), diag::err_module_unable_to_hash_content)
+            << Entry.File->getName();
+    }
+    auto CH = llvm::APInt(64, ContentHash);
+    Entry.ContentHash[0] =
+        static_cast<uint32_t>(CH.getLoBits(32).getZExtValue());
+    Entry.ContentHash[1] =
+        static_cast<uint32_t>(CH.getHiBits(32).getZExtValue());
+
+    if (Entry.IsSystemFile)
+      SortedInputFiles.push_back(Entry);
+    else
+      SortedInputFiles.push_front(Entry);
+  }
+
+  llvm::SmallPtrSet<const FileEntry *, 32> SeenFileEntries;
+  auto NewEnd =
+      std::remove_if(SortedInputFiles.begin(), SortedInputFiles.end(),
+                     [&SeenFileEntries](const InputFileEntry &Entry) {
+                       return !SeenFileEntries.insert(Entry.File).second;
+                     });
+  SortedInputFiles.erase(NewEnd, SortedInputFiles.end());
+
+  uint32_t NextID = 1;
+  for (const auto &Entry : SortedInputFiles) {
+    uint32_t &InputFileID = InputFileIDs[Entry.File];
+    if (InputFileID != 0)
+      continue; // already generated an ID for this file.
+
+    InputFileID = NextID++;
+  }
+}
+
 unsigned ASTWriter::getSubmoduleID(Module *Mod) {
   // FIXME: This can easily happen, if we have a reference to a submodule that
   // did not result in us loading a module file for that submodule. For
   // instance, a cross-top-level-module 'conflict' declaration will hit this.
   unsigned ID = getLocalOrImportedSubmoduleID(Mod);
   assert((ID || !Mod) &&
          "asked for module ID for non-local, non-imported module");
   return ID;
for (const auto &DeleteExprsInfo :
       DeleteExprsToAnalyze.push_back(DeleteExprsInfo.second.size());
       for (const auto &DeleteLoc : DeleteExprsInfo.second) {
         AddSourceLocation(DeleteLoc.first, DeleteExprsToAnalyze);
         DeleteExprsToAnalyze.push_back(DeleteLoc.second);
       }
     }
   }
 
-  // Write the control block
-  WriteControlBlock(PP, Context, isysroot, OutputFile);
+  if (WritingModule && WritingModule->Directory) {
+    SmallString<128> BaseDir(WritingModule->Directory->getName());
+    cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
+    // Write out all other paths relative to the base directory if possible.
+    BaseDirectory.assign(BaseDir.begin(), BaseDir.end());
+  } else if (!isysroot.empty()) {
+    // Write out paths relative to the sysroot if possible.
+    BaseDirectory = std::string(isysroot);
+  }
+
+  populateInputFileIDs(Context.SourceMgr);
 
   // Write the remaining AST contents.
   Stream.EnterSubblock(AST_BLOCK_ID, 5);
 
   // This is so that older clang versions, before the introduction
   // of the control block, can read and reject the newer PCH format.
   {
     RecordData Record = {VERSION_MAJOR};
ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
   WriteFloatControlPragmaOptions(SemaRef);
 
   // Some simple statistics
   RecordData::value_type Record[] = {
       NumStatements, NumMacros, NumLexicalDeclContexts, NumVisibleDeclContexts};
   Stream.EmitRecord(STATISTICS, Record);
   Stream.ExitBlock();
 
+  // Write the control block
+  WriteControlBlock(PP, Context, isysroot, OutputFile);
+
   // Write the module file extension blocks.
   for (const auto &ExtWriter : ModuleFileExtensionWriters)
     WriteModuleFileExtension(SemaRef, *ExtWriter);
 
   return writeUnhashedControlBlock(PP, Context);
 }
 
 void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {