[ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

This addresses an issue with how the PCH preable works, specifically:

1. When using a PCH/preamble the module hash changes and a different cache directory is used
2. When the preamble is used, PCH & PCM validation is disabled.

Due to combination of #1 and #2, reparsing with preamble enabled can end up loading a stale module file before a header change and using it without updating it because validation is disabled and it doesn’t check that the header has changed and the module file is out-of-date.

rdar://72611253

Differential Revision: https://reviews.llvm.org/D95159

Author: akyrtzi

clang/include/clang/Driver/Options.td

@@ -5090,7 +5090,8 @@ def fhalf_no_semantic_interposition : Flag<["-"], "fhalf-no-semantic-interpositi
   MarshallingInfoFlag<"LangOpts->HalfNoSemanticInterposition">;
 def fno_validate_pch : Flag<["-"], "fno-validate-pch">,
   HelpText<"Disable validation of precompiled headers">,
-  MarshallingInfoFlag<"PreprocessorOpts->DisablePCHValidation">;
+  MarshallingInfoFlag<"PreprocessorOpts->DisablePCHOrModuleValidation", "DisableValidationForModuleKind::None">,
+  Normalizer<"makeFlagToValueNormalizer(DisableValidationForModuleKind::All)">;
 def fallow_pch_with_errors : Flag<["-"], "fallow-pch-with-compiler-errors">,
   HelpText<"Accept a PCH file that was created with compiler errors">;
 def fallow_pcm_with_errors : Flag<["-"], "fallow-pcm-with-compiler-errors">,

clang/include/clang/Frontend/CompilerInstance.h

@@ -50,6 +50,7 @@ class Preprocessor;
 class Sema;
 class SourceManager;
 class TargetInfo;
+enum class DisableValidationForModuleKind;
 
 /// CompilerInstance - Helper class for managing a single instance of the Clang
 /// compiler.
@@ -673,16 +674,17 @@ class CompilerInstance : public ModuleLoader {
 
   /// Create an external AST source to read a PCH file and attach it to the AST
   /// context.
-  void createPCHExternalASTSource(StringRef Path, bool DisablePCHValidation,
-                                  bool AllowPCHWithCompilerErrors,
-                                  void *DeserializationListener,
-                                  bool OwnDeserializationListener);
+  void createPCHExternalASTSource(
+      StringRef Path, DisableValidationForModuleKind DisableValidation,
+      bool AllowPCHWithCompilerErrors, void *DeserializationListener,
+      bool OwnDeserializationListener);
 
   /// Create an external AST source to read a PCH file.
   ///
   /// \return - The new object on success, or null on failure.
   static IntrusiveRefCntPtr<ASTReader> createPCHExternalASTSource(
-      StringRef Path, StringRef Sysroot, bool DisablePCHValidation,
+      StringRef Path, StringRef Sysroot,
+      DisableValidationForModuleKind DisableValidation,
       bool AllowPCHWithCompilerErrors, Preprocessor &PP,
       InMemoryModuleCache &ModuleCache, ASTContext &Context,
       const PCHContainerReader &PCHContainerRdr,

clang/include/clang/Lex/PreprocessorOptions.h

@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_LEX_PREPROCESSOROPTIONS_H_
 #define LLVM_CLANG_LEX_PREPROCESSOROPTIONS_H_
 
+#include "clang/Basic/BitmaskEnum.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h"
 #include "llvm/ADT/StringRef.h"
@@ -40,6 +41,24 @@ enum ObjCXXARCStandardLibraryKind {
   ARCXX_libstdcxx
 };
 
+/// Whether to disable the normal validation performed on precompiled
+/// headers and module files when they are loaded.
+enum class DisableValidationForModuleKind {
+  /// Perform validation, don't disable it.
+  None = 0,
+
+  /// Disable validation for a precompiled header and the modules it depends on.
+  PCH = 0x1,
+
+  /// Disable validation for module files.
+  Module = 0x2,
+
+  /// Disable validation for all kinds.
+  All = PCH | Module,
+
+  LLVM_MARK_AS_BITMASK_ENUM(Module)
+};
+
 /// PreprocessorOptions - This class is used for passing the various options
 /// used in preprocessor initialization to InitializePreprocessor().
 class PreprocessorOptions {
@@ -79,9 +98,10 @@ class PreprocessorOptions {
   /// Headers that will be converted to chained PCHs in memory.
   std::vector<std::string> ChainedIncludes;
 
-  /// When true, disables most of the normal validation performed on
-  /// precompiled headers.
-  bool DisablePCHValidation = false;
+  /// Whether to disable most of the normal validation performed on
+  /// precompiled headers and module files.
+  DisableValidationForModuleKind DisablePCHOrModuleValidation =
+      DisableValidationForModuleKind::None;
 
   /// When true, a PCH with compiler errors will not be rejected.
   bool AllowPCHWithCompilerErrors = false;

clang/include/clang/Serialization/ASTReader.h

@@ -23,6 +23,7 @@
 #include "clang/Lex/ExternalPreprocessorSource.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PreprocessingRecord.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/ExternalSemaSource.h"
 #include "clang/Sema/IdentifierResolver.h"
 #include "clang/Serialization/ASTBitCodes.h"
@@ -440,6 +441,9 @@ class ASTReader
   /// imported from. For non-module AST types it should be invalid.
   SourceLocation CurrentImportLoc;
 
+  /// The module kind that is currently deserializing.
+  Optional<ModuleKind> CurrentDeserializingModuleKind;
+
   /// The global module index, if loaded.
   std::unique_ptr<GlobalModuleIndex> GlobalIndex;
 
@@ -923,8 +927,8 @@ class ASTReader
   std::string isysroot;
 
   /// Whether to disable the normal validation performed on precompiled
-  /// headers when they are loaded.
-  bool DisableValidation;
+  /// headers and module files when they are loaded.
+  DisableValidationForModuleKind DisableValidationKind;
 
   /// Whether to accept an AST file with compiler errors.
   bool AllowASTWithCompilerErrors;
@@ -1207,6 +1211,8 @@ class ASTReader
 
   llvm::DenseMap<const Decl *, bool> DefinitionSource;
 
+  bool shouldDisableValidationForFile(const serialization::ModuleFile &M) const;
+
   /// Reads a statement from the specified cursor.
   Stmt *ReadStmtFromStream(ModuleFile &F);
 
@@ -1467,9 +1473,9 @@ class ASTReader
   /// user. This is only used with relocatable PCH files. If non-NULL,
   /// a relocatable PCH file will use the default path "/".
   ///
-  /// \param DisableValidation If true, the AST reader will suppress most
+  /// \param DisableValidationKind If set, the AST reader will suppress most
   /// of its regular consistency checking, allowing the use of precompiled
-  /// headers that cannot be determined to be compatible.
+  /// headers and module files that cannot be determined to be compatible.
   ///
   /// \param AllowASTWithCompilerErrors If true, the AST reader will accept an
   /// AST file the was created out of an AST with compiler errors,
@@ -1490,7 +1496,9 @@ class ASTReader
   ASTReader(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
             ASTContext *Context, const PCHContainerReader &PCHContainerRdr,
             ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
-            StringRef isysroot = "", bool DisableValidation = false,
+            StringRef isysroot = "",
+            DisableValidationForModuleKind DisableValidationKind =
+                DisableValidationForModuleKind::None,
             bool AllowASTWithCompilerErrors = false,
             bool AllowConfigurationMismatch = false,
             bool ValidateSystemInputs = false,

clang/lib/Frontend/ASTUnit.cpp

@@ -809,9 +809,10 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
                               PP.getIdentifierTable(), PP.getSelectorTable(),
                               PP.getBuiltinInfo());
 
-  bool disableValid = false;
+  DisableValidationForModuleKind disableValid =
+      DisableValidationForModuleKind::None;
   if (::getenv("LIBCLANG_DISABLE_PCH_VALIDATION"))
-    disableValid = true;
+    disableValid = DisableValidationForModuleKind::All;
   AST->Reader = new ASTReader(
       PP, *AST->ModuleCache, AST->Ctx.get(), PCHContainerRdr, {},
       /*isysroot=*/"",

clang/lib/Frontend/ChainedIncludesSource.cpp

@@ -83,10 +83,10 @@ createASTReader(CompilerInstance &CI, StringRef pchFile,
                 ASTDeserializationListener *deserialListener = nullptr) {
   Preprocessor &PP = CI.getPreprocessor();
   std::unique_ptr<ASTReader> Reader;
-  Reader.reset(new ASTReader(PP, CI.getModuleCache(), &CI.getASTContext(),
-                             CI.getPCHContainerReader(),
-                             /*Extensions=*/{},
-                             /*isysroot=*/"", /*DisableValidation=*/true));
+  Reader.reset(new ASTReader(
+      PP, CI.getModuleCache(), &CI.getASTContext(), CI.getPCHContainerReader(),
+      /*Extensions=*/{},
+      /*isysroot=*/"", DisableValidationForModuleKind::PCH));
   for (unsigned ti = 0; ti < bufNames.size(); ++ti) {
     StringRef sr(bufNames[ti]);
     Reader->addInMemoryBuffer(sr, std::move(MemBufs[ti]));
@@ -129,7 +129,8 @@ IntrusiveRefCntPtr<ExternalSemaSource> clang::createChainedIncludesSource(
 
     CInvok->getPreprocessorOpts().ChainedIncludes.clear();
     CInvok->getPreprocessorOpts().ImplicitPCHInclude.clear();
-    CInvok->getPreprocessorOpts().DisablePCHValidation = true;
+    CInvok->getPreprocessorOpts().DisablePCHOrModuleValidation =
+        DisableValidationForModuleKind::PCH;
     CInvok->getPreprocessorOpts().Includes.clear();
     CInvok->getPreprocessorOpts().MacroIncludes.clear();
     CInvok->getPreprocessorOpts().Macros.clear();

clang/lib/Frontend/CompilerInstance.cpp

@@ -505,11 +505,12 @@ void CompilerInstance::createASTContext() {
 // ExternalASTSource
 
 void CompilerInstance::createPCHExternalASTSource(
-    StringRef Path, bool DisablePCHValidation, bool AllowPCHWithCompilerErrors,
-    void *DeserializationListener, bool OwnDeserializationListener) {
+    StringRef Path, DisableValidationForModuleKind DisableValidation,
+    bool AllowPCHWithCompilerErrors, void *DeserializationListener,
+    bool OwnDeserializationListener) {
   bool Preamble = getPreprocessorOpts().PrecompiledPreambleBytes.first != 0;
   TheASTReader = createPCHExternalASTSource(
-      Path, getHeaderSearchOpts().Sysroot, DisablePCHValidation,
+      Path, getHeaderSearchOpts().Sysroot, DisableValidation,
       AllowPCHWithCompilerErrors, getPreprocessor(), getModuleCache(),
       getASTContext(), getPCHContainerReader(),
       getFrontendOpts().ModuleFileExtensions, DependencyCollectors,
@@ -518,7 +519,8 @@ void CompilerInstance::createPCHExternalASTSource(
 }
 
 IntrusiveRefCntPtr<ASTReader> CompilerInstance::createPCHExternalASTSource(
-    StringRef Path, StringRef Sysroot, bool DisablePCHValidation,
+    StringRef Path, StringRef Sysroot,
+    DisableValidationForModuleKind DisableValidation,
     bool AllowPCHWithCompilerErrors, Preprocessor &PP,
     InMemoryModuleCache &ModuleCache, ASTContext &Context,
     const PCHContainerReader &PCHContainerRdr,
@@ -530,7 +532,7 @@ IntrusiveRefCntPtr<ASTReader> CompilerInstance::createPCHExternalASTSource(
 
   IntrusiveRefCntPtr<ASTReader> Reader(new ASTReader(
       PP, ModuleCache, &Context, PCHContainerRdr, Extensions,
-      Sysroot.empty() ? "" : Sysroot.data(), DisablePCHValidation,
+      Sysroot.empty() ? "" : Sysroot.data(), DisableValidation,
       AllowPCHWithCompilerErrors, /*AllowConfigurationMismatch*/ false,
       HSOpts.ModulesValidateSystemHeaders, HSOpts.ValidateASTInputFilesContent,
       UseGlobalModuleIndex));
@@ -1532,7 +1534,8 @@ void CompilerInstance::createASTReader() {
   TheASTReader = new ASTReader(
       getPreprocessor(), getModuleCache(), &getASTContext(),
       getPCHContainerReader(), getFrontendOpts().ModuleFileExtensions,
-      Sysroot.empty() ? "" : Sysroot.c_str(), PPOpts.DisablePCHValidation,
+      Sysroot.empty() ? "" : Sysroot.c_str(),
+      PPOpts.DisablePCHOrModuleValidation,
       /*AllowASTWithCompilerErrors=*/FEOpts.AllowPCMWithCompilerErrors,
       /*AllowConfigurationMismatch=*/false, HSOpts.ModulesValidateSystemHeaders,
       HSOpts.ValidateASTInputFilesContent,

clang/lib/Frontend/FrontendAction.cpp

@@ -879,9 +879,9 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
       if (!CI.getPreprocessorOpts().ImplicitPCHInclude.empty()) {
         CI.createPCHExternalASTSource(
             CI.getPreprocessorOpts().ImplicitPCHInclude,
-            CI.getPreprocessorOpts().DisablePCHValidation,
-          CI.getPreprocessorOpts().AllowPCHWithCompilerErrors, DeserialListener,
-            DeleteDeserialListener);
+            CI.getPreprocessorOpts().DisablePCHOrModuleValidation,
+            CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
+            DeserialListener, DeleteDeserialListener);
         if (!CI.getASTContext().getExternalSource())
           goto failure;
       }

clang/lib/Frontend/FrontendActions.cpp

@@ -344,7 +344,7 @@ void VerifyPCHAction::ExecuteAction() {
       CI.getPreprocessor(), CI.getModuleCache(), &CI.getASTContext(),
       CI.getPCHContainerReader(), CI.getFrontendOpts().ModuleFileExtensions,
       Sysroot.empty() ? "" : Sysroot.c_str(),
-      /*DisableValidation*/ false,
+      DisableValidationForModuleKind::None,
       /*AllowASTWithCompilerErrors*/ false,
       /*AllowConfigurationMismatch*/ true,
       /*ValidateSystemInputs*/ true));

clang/lib/Frontend/PrecompiledPreamble.cpp

@@ -812,7 +812,8 @@ void PrecompiledPreamble::configurePreamble(
   PreprocessorOpts.PrecompiledPreambleBytes.first = Bounds.Size;
   PreprocessorOpts.PrecompiledPreambleBytes.second =
       Bounds.PreambleEndsAtStartOfLine;
-  PreprocessorOpts.DisablePCHValidation = true;
+  PreprocessorOpts.DisablePCHOrModuleValidation =
+      DisableValidationForModuleKind::PCH;
 
   setupPreambleStorage(Storage, PreprocessorOpts, VFS);
 }