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

[akyrtzi]

Summary

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

[akyrtzi]

clang-format changes

[bnbarham]

LGTM, just the one minor comment

clang/lib/Serialization/ASTReader.cpp

2221–2222

There's a getValueOr method that you could use instead (ie. CurrentDeserializingModuleKind.getValueOr(M.Kind)).

clang/test/Index/preamble-reparse-changed-module.m

9

For some reason I was expecting the trial's to be 1 based, not 0. I'm not sure why though.

[akyrtzi]

Use getValueOr

[akyrtzi]

Fix typo in commit message, 'state' -> 'stale'

[mibintc]

The newly added test case clang/test/Index/preamble-reparse-changed-module.m is failing on the patch that I'm trying to commit, https://reviews.llvm.org/D93769

Actually the result of the test is a bit flaky, when I build the Debug version on Linux, running ninja check-clang does show the test fail, but when I do an identical build and in addition enabling llvm assertions, llvm-lit does not show this lit test failing.

I think the problem has something to do with the macro FLT_EVAL_METHOD. The patch now treats this macro specially, more like LINE a macro that can vary depending on where it is seen in the source code.

In my patch, FLT_EVAL_MACRO is not expanded in -E mode, it just appears as its name in the preprocessed output. The reason is, the actual value of the macro depends on the floating point settings at a particular source code location (depends on #pragma float_control settings). The pragma are determined in Sema, and -E runs before Sema so the actual value isn't available.

This macro is the only reason i can think why this test is failing.

Can you help me understand what to do about this test case/about my patch/ so that i can get my patch committed? I don't understand what this test is doing. In the debugger it hits a failed assertion, looks like index out-of-range. here's a little massaged traceback
__assert_fail
llvm::SmallVectorTemplateCommon unsigned long, void ::indexing operator
clang::ASTReader::ReadString
clang::ASTReader::ParseLanguageOptions
clang::ASTReader::ReadOptionsBlock
clang::ASTReader::ReadControlBlock
clang::ASTReader::ReadASTCore
clang::ASTReader::ReadAST
clang::CompilerInstance::findOrCompileModuleAndReadAST
clang::CompilerInstance::loadModule
clang::Preprocessor::LexAfterModuleImport

Thanks a lot

[mibintc]

@aaron.ballman suggested I may have made a mistake adding the new option, I'll look there.

[mibintc]

More info about the test failure I'm seeing for clang/test/Index/preamble-reparse-changed-module.m

It appears it might be a non-deterministic failure, perhaps some test dependencies are missing?

Yesterday before I pushed my clang patch, I saw it fail when I used "ninja check-clang". I quickly executed each step used by this test (llvm-lit -v ; then save all the RUN commands to a file ; then "source" the file -- nothing was printed to stdout and also the return code was 0). I thought the fail report wasn't real at this point and I pushed the patch. This was a Debug build.

Then I saw a bot failure, @MaskRay pinged my clang patch and said my patch had caused this test to fail. He told me there was an assertion fail.

Then I created a new build area, configured with Debug and "enable llvm assertions", and "ninja check-clang". This time 0 tests reported as fail.

Then I went back into the build-debug build where "ninja check-clang" had reported the fail and used gdb to debug. In the course of debugging gdb told me that the build was out-of-date w.r.t. the source file, so I exitted gdb, "ninja clean" "ninja check-clang". But now no tests are failing. I don't know why gdb told me the build was out of date, I believe I did not change any of the files but I'm not 100% certain.

I removed the patch and created another build area, and rebuilt with Debug and asserts, everything is clean there too.

I captured the stdout from executing c-index-test, there is a difference between patched and unpatched. The FLT_EVAL_METHOD line is not present in the patched, this is expected.

Can you suggest how to find the problem? Thanks a lot.

[bnbarham]

In D95159#2897941, @mibintc wrote:

Can you suggest how to find the problem? Thanks a lot.

@mibintc the test is reusing the module cache directory and that's causing some issues between runs, sorry about that. You could add a rm -rf %t or rm -rf %t/mcp to this test to fix that. Looks like the same issue was run into in https://reviews.llvm.org/D96816#2572589. I'd prefer clang/test/Index/preamble-reparse-changed-module.m add the rm and another test added for the LLVM_APPEND_VC_REV=NO if we want that.

[mibintc]

@bnbarham
I think you mean a patch like this, is it right? I'd like to fix the test in a pre-commit and then re-push my patch

 git diff
diff --git a/clang/test/Index/preamble-reparse-changed-module.m b/clang/test/Index/preamble-reparse-changed-module.m
index 1c63e802ce0c..fc336c6e0670 100644
--- a/clang/test/Index/preamble-reparse-changed-module.m
+++ b/clang/test/Index/preamble-reparse-changed-module.m
@@ -1,6 +1,7 @@
 // REQUIRES: shell

-// RUN: mkdir -p %t/mod
+// RUN: rm -rf %t
+// RUN: mkdir %t/mod
 // RUN: touch %t/empty.h
 // RUN: cp %S/Inputs/preamble-reparse-changed-module/module.modulemap %t/mod
 // RUN: cp %S/Inputs/preamble-reparse-changed-module/head.h %t/mod

[mibintc]

Whoops, shouldn't mess with the mkdir line

diff --git a/clang/test/Index/preamble-reparse-changed-module.m b/clang/test/Index/preamble-reparse-changed-module.m
index 1c63e802ce0c..349ed0db27d0 100644

• a/clang/test/Index/preamble-reparse-changed-module.m

+++ b/clang/test/Index/preamble-reparse-changed-module.m
@@ -1,5 +1,6 @@
// REQUIRES: shell

+ RUN: rm -rf %t
RUN: mkdir -p %t/mod
RUN: touch %t/empty.h
RUN: cp %S/Inputs/preamble-reparse-changed-module/module.modulemap %t/mod

[bnbarham]

Yep, that would be perfect :). Thanks @mibintc

clang/include/clang/Driver/Options.td

 def pic_is_pie : Flag<["-"], "pic-is-pie">,
   HelpText<"File is for a position independent executable">,
   MarshallingInfoFlag<LangOpts<"PIE">>;
 def fhalf_no_semantic_interposition : Flag<["-"], "fhalf-no-semantic-interposition">,
   HelpText<"Like -fno-semantic-interposition but don't use local aliases">,
   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_pcm_with_errors : Flag<["-"], "fallow-pcm-with-compiler-errors">,
   HelpText<"Accept a PCM file that was created with compiler errors">,
   MarshallingInfoFlag<FrontendOpts<"AllowPCMWithCompilerErrors">>;
 def fallow_pch_with_errors : Flag<["-"], "fallow-pch-with-compiler-errors">,
   HelpText<"Accept a PCH file that was created with compiler errors">,
   MarshallingInfoFlag<PreprocessorOpts<"AllowPCHWithCompilerErrors">>,
   ImpliedByAnyOf<[fallow_pcm_with_errors]>;
 def dump_deserialized_pch_decls : Flag<["-"], "dump-deserialized-decls">,

clang/include/clang/Frontend/CompilerInstance.h

 class FileManager;
 class FrontendAction;
 class InMemoryModuleCache;
 class Module;
 class Preprocessor;
 class Sema;
 class SourceManager;
 class TargetInfo;
+enum class DisableValidationForModuleKind;
 
 /// CompilerInstance - Helper class for managing a single instance of the Clang
 /// compiler.
 ///
 /// The CompilerInstance serves two purposes:
 ///  (1) It manages the various objects which are necessary to run the compiler,
 ///      for example the preprocessor, the target information, and the AST
 ///      context.
std::string getSpecificModuleCachePath() {
     return getSpecificModuleCachePath(getInvocation().getModuleHash());
   }
 
   /// Create the AST context.
   void createASTContext();
 
   /// 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,
 
   /// 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,
       ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
       ArrayRef<std::shared_ptr<DependencyCollector>> DependencyCollectors,
       void *DeserializationListener, bool OwnDeserializationListener,
       bool Preamble, bool UseGlobalModuleIndex);
 

clang/include/clang/Lex/PreprocessorOptions.h

 //===- PreprocessorOptions.h ------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "llvm/ADT/StringSet.h"
 #include <functional>
 #include <map>
 #include <memory>
 #include <set>
enum ObjCXXARCStandardLibraryKind {
 
   /// libc++
   ARCXX_libcxx,
 
   /// libstdc++
   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 {
 public:
   std::vector<std::pair<std::string, bool/*isUndef*/>> Macros;
   std::vector<std::string> Includes;
   std::vector<std::string> MacroIncludes;
 
public:
   std::string PCHThroughHeader;
 
   /// The implicit PCH included at the start of the translation unit, or empty.
   std::string ImplicitPCHInclude;
 
   /// 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;
 
   /// Dump declarations that are deserialized from PCH, for testing.
   bool DumpDeserializedPCHDecls = false;
 
   /// This is a set of names for decls that we do not want to be

clang/include/clang/Serialization/ASTReader.h

 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/OpenCLOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Version.h"
 #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/Sema/Sema.h"
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ContinuousRangeMap.h"
 #include "clang/Serialization/ModuleFile.h"
 #include "clang/Serialization/ModuleFileExtension.h"
 #include "clang/Serialization/ModuleManager.h"
private:
 
   /// A timer used to track the time spent deserializing.
   std::unique_ptr<llvm::Timer> ReadTimer;
 
   /// The location where the module file will be considered as
   /// 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;
 
   /// A map of global bit offsets to the module that stores entities
   /// at those bit offsets.
   ContinuousRangeMap<uint64_t, ModuleFile*, 4> GlobalBitOffsetsMap;
 
   /// A map of negated SLocEntryIDs to the modules containing them.
private:
   SmallVector<ImportedSubmodule, 2> ImportedModules;
   //@}
 
   /// The system include root to be used when loading the
   /// precompiled header.
   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;
 
   /// Whether to accept an AST file that has a different configuration
   /// from the current compiler instance.
   bool AllowConfigurationMismatch;
 
private:
   /// buffer computed to build the PCH file will be identical to the
   /// predefines buffer computed from the command line. However, when
   /// there are differences that the PCH reader can work around, this
   /// predefines buffer may contain additional definitions.
   std::string SuggestedPredefines;
 
   llvm::DenseMap<const Decl *, bool> DefinitionSource;
 
+  bool shouldDisableValidationForFile(const serialization::ModuleFile &M) const;
+
   /// Reads a statement from the specified cursor.
   Stmt *ReadStmtFromStream(ModuleFile &F);
 
   struct InputFileInfo {
     std::string Filename;
     uint64_t ContentHash;
     off_t StoredSize;
     time_t StoredTime;
public:
   ///
   /// \param Extensions the list of module file extensions that can be loaded
   /// from the AST files.
   ///
   /// \param isysroot If non-NULL, the system include path specified by the
   /// 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,
   /// otherwise it will reject it.
   ///
   /// \param AllowConfigurationMismatch If true, the AST reader will not check
   /// for configuration differences between the AST file and the invocation.
   ///
   /// \param ValidateSystemInputs If true, the AST reader will validate
   /// system input files in addition to user input files. This is only
   /// meaningful if \p DisableValidation is false.
   ///
   /// \param UseGlobalIndex If true, the AST reader will try to load and use
   /// the global module index.
   ///
   /// \param ReadTimer If non-null, a timer used to track the time spent
   /// deserializing.
   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,
             bool ValidateASTInputFilesContent = false,
             bool UseGlobalIndex = true,
             std::unique_ptr<llvm::Timer> ReadTimer = {});
   ASTReader(const ASTReader &) = delete;
   ASTReader &operator=(const ASTReader &) = delete;

clang/lib/Frontend/ASTUnit.cpp

AST->PP = std::make_shared<Preprocessor>(
       /*OwnsHeaderSearch=*/false);
   Preprocessor &PP = *AST->PP;
 
   if (ToLoad >= LoadASTOnly)
     AST->Ctx = new ASTContext(*AST->LangOpts, AST->getSourceManager(),
                               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=*/"",
       /*DisableValidation=*/disableValid, AllowASTWithCompilerErrors);
 
   AST->Reader->setListener(std::make_unique<ASTInfoCollector>(
       *AST->PP, AST->Ctx.get(), *AST->HSOpts, *AST->PPOpts, *AST->LangOpts,
       AST->TargetOpts, AST->Target, Counter));

clang/lib/Frontend/ChainedIncludesSource.cpp

 
 static ASTReader *
 createASTReader(CompilerInstance &CI, StringRef pchFile,
                 SmallVectorImpl<std::unique_ptr<llvm::MemoryBuffer>> &MemBufs,
                 SmallVectorImpl<std::string> &bufNames,
                 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(),
+      /*isysroot=*/"", DisableValidationForModuleKind::PCH));
   for (unsigned ti = 0; ti < bufNames.size(); ++ti) {
     StringRef sr(bufNames[ti]);
     Reader->addInMemoryBuffer(sr, std::move(MemBufs[ti]));
   }
   Reader->setDeserializationListener(deserialListener);
   switch (Reader->ReadAST(pchFile, serialization::MK_PCH, SourceLocation(),
                           ASTReader::ARR_None)) {
   case ASTReader::Success:
IntrusiveRefCntPtr<ExternalSemaSource> clang::createChainedIncludesSource(
 
   for (unsigned i = 0, e = includes.size(); i != e; ++i) {
     bool firstInclude = (i == 0);
     std::unique_ptr<CompilerInvocation> CInvok;
     CInvok.reset(new CompilerInvocation(CI.getInvocation()));
 
     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();
 
     CInvok->getFrontendOpts().Inputs.clear();
     FrontendInputFile InputFile(includes[i], IK);
     CInvok->getFrontendOpts().Inputs.push_back(InputFile);
 

clang/lib/Frontend/CompilerInstance.cpp

auto *Context = new ASTContext(getLangOpts(), PP.getSourceManager(),
                                  PP.getBuiltinInfo());
   Context->InitBuiltinTypes(getTarget(), getAuxTarget());
   setASTContext(Context);
 }
 
 // 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,
       DeserializationListener, OwnDeserializationListener, Preamble,
       getFrontendOpts().UseGlobalModuleIndex);
 }
 
 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,
     ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
     ArrayRef<std::shared_ptr<DependencyCollector>> DependencyCollectors,
     void *DeserializationListener, bool OwnDeserializationListener,
     bool Preamble, bool UseGlobalModuleIndex) {
   HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
 
   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));
 
   // We need the external source to be set up before we read the AST, because
   // eagerly-deserialized declarations may use it.
   Context.setExternalSource(Reader.get());
 
void CompilerInstance::createASTReader() {
 
   if (FrontendTimerGroup)
     ReadTimer = std::make_unique<llvm::Timer>("reading_modules",
                                                 "Reading modules",
                                                 *FrontendTimerGroup);
   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,
       getFrontendOpts().UseGlobalModuleIndex, std::move(ReadTimer));
   if (hasASTConsumer()) {
     TheASTReader->setDeserializationListener(
         getASTConsumer().GetASTDeserializationListener());
     getASTContext().setASTMutationListener(

clang/lib/Frontend/FrontendAction.cpp

if (!CI.getPreprocessorOpts().ChainedIncludes.empty()) {
             CI.getASTContext(),
             CI.getPreprocessorOpts().DeserializedPCHDeclsToErrorOn,
             DeserialListener, DeleteDeserialListener);
         DeleteDeserialListener = true;
       }
       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;
       }
       // If modules are enabled, create the AST reader before creating
       // any builtins, so that all declarations know that they might be
       // extended by an external source.
       if (CI.getLangOpts().Modules || !CI.hasASTContext() ||
           !CI.getASTContext().getExternalSource()) {

clang/lib/Frontend/FrontendActions.cpp

 void VerifyPCHAction::ExecuteAction() {
   CompilerInstance &CI = getCompilerInstance();
   bool Preamble = CI.getPreprocessorOpts().PrecompiledPreambleBytes.first != 0;
   const std::string &Sysroot = CI.getHeaderSearchOpts().Sysroot;
   std::unique_ptr<ASTReader> Reader(new ASTReader(
       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));
 
   Reader->ReadAST(getCurrentFile(),
                   Preamble ? serialization::MK_Preamble
                            : serialization::MK_PCH,
                   SourceLocation(),

clang/lib/Frontend/PrecompiledPreamble.cpp

void PrecompiledPreamble::configurePreamble(
   // Remap main file to point to MainFileBuffer.
   auto MainFilePath = CI.getFrontendOpts().Inputs[0].getFile();
   PreprocessorOpts.addRemappedFile(MainFilePath, MainFileBuffer);
 
   // Configure ImpicitPCHInclude.
   PreprocessorOpts.PrecompiledPreambleBytes.first = Bounds.Size;
   PreprocessorOpts.PrecompiledPreambleBytes.second =
       Bounds.PreambleEndsAtStartOfLine;
-  PreprocessorOpts.DisablePCHValidation = true;
+  PreprocessorOpts.DisablePCHOrModuleValidation =
+      DisableValidationForModuleKind::PCH;
 
   setupPreambleStorage(Storage, PreprocessorOpts, VFS);
 }
 
 void PrecompiledPreamble::setupPreambleStorage(
     const PCHStorage &Storage, PreprocessorOptions &PreprocessorOpts,
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> &VFS) {
   if (Storage.getKind() == PCHStorage::Kind::TempFile) {

clang/lib/Serialization/ASTReader.cpp

if (Earliest)
       Earliest->setPrevious(MD);
     Earliest = MD;
   }
 
   if (Latest)
     PP.setLoadedMacroDirective(II, Earliest, Latest);
 }
 
+bool ASTReader::shouldDisableValidationForFile(
+    const serialization::ModuleFile &M) const {
+  if (DisableValidationKind == DisableValidationForModuleKind::None)
+    return false;
+
+  // If a PCH is loaded and validation is disabled for PCH then disable
+  // validation for the PCH and the modules it loads.
+  ModuleKind K = CurrentDeserializingModuleKind.getValueOr(M.Kind);
+
+  switch (K) {
+  case MK_MainFile:
+  case MK_Preamble:
+  case MK_PCH:
+    return bool(DisableValidationKind & DisableValidationForModuleKind::PCH);
+  case MK_ImplicitModule:
+  case MK_ExplicitModule:
+  case MK_PrebuiltModule:
+    return bool(DisableValidationKind & DisableValidationForModuleKind::Module);
+  }
+
+  return false;
+}
+
 ASTReader::InputFileInfo
 ASTReader::readInputFileInfo(ModuleFile &F, unsigned ID) {
   // Go find this input file.
   BitstreamCursor &Cursor = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
   if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
     // FIXME this drops errors on the floor.
     consumeError(std::move(Err));
enum ModificationType {
     Size,
     ModTime,
     Content,
     None,
   };
   auto HasInputFileChanged = [&]() {
     if (StoredSize != File->getSize())
       return ModificationType::Size;
-    if (!DisableValidation && StoredTime &&
+    if (!shouldDisableValidationForFile(F) && StoredTime &&
         StoredTime != File->getModificationTime()) {
       // In case the modification time changes but not the content,
       // accept the cached file as legit.
       if (ValidateASTInputFilesContent &&
           StoredContentHash != static_cast<uint64_t>(llvm::hash_code(-1))) {
         auto MemBuffOrError = FileMgr.getBufferForFile(File);
         if (!MemBuffOrError) {
           if (!Complain)
if (!HasReadUnhashedControlBlock) {
       HasReadUnhashedControlBlock = true;
       if (ASTReadResult Result =
               readUnhashedControlBlock(F, ImportedBy, ClientLoadCapabilities))
         return Result;
     }
     return Success;
   };
 
+  bool DisableValidation = shouldDisableValidationForFile(F);
+
   // Read all of the records and blocks in the control block.
   RecordData Record;
   unsigned NumInputs = 0;
   unsigned NumUserInputs = 0;
   StringRef BaseDirectoryAsWritten;
   while (true) {
     Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
     if (!MaybeEntry) {
case MODULE_DIRECTORY: {
       // have a better path for it (relative to the current build).
       Module *M = PP.getHeaderSearchInfo().lookupModule(
           F.ModuleName, /*AllowSearch*/ true,
           /*AllowExtraModuleMapSearch*/ true);
       if (M && M->Directory) {
         // If we're implicitly loading a module, the base directory can't
         // change between the build and use.
         // Don't emit module relocation error if we have -fno-validate-pch
-        if (!PP.getPreprocessorOpts().DisablePCHValidation &&
+        if (!bool(PP.getPreprocessorOpts().DisablePCHOrModuleValidation &
+                  DisableValidationForModuleKind::Module) &&
             F.Kind != MK_ExplicitModule && F.Kind != MK_PrebuiltModule) {
           auto BuildDir = PP.getFileManager().getDirectory(Blob);
           if (!BuildDir || *BuildDir != M->Directory) {
             if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
               Diag(diag::err_imported_module_relocated)
                   << F.ModuleName << Blob << M->Directory->getName();
             return OutOfDate;
           }
assert(!F.ModuleName.empty() &&
          "MODULE_NAME should come before MODULE_MAP_FILE");
   if (F.Kind == MK_ImplicitModule && ModuleMgr.begin()->Kind != MK_MainFile) {
     // An implicitly-loaded module file should have its module listed in some
     // module map file that we've already loaded.
     Module *M = PP.getHeaderSearchInfo().lookupModule(F.ModuleName);
     auto &Map = PP.getHeaderSearchInfo().getModuleMap();
     const FileEntry *ModMap = M ? Map.getModuleMapFileForUniquing(M) : nullptr;
     // Don't emit module relocation error if we have -fno-validate-pch
-    if (!PP.getPreprocessorOpts().DisablePCHValidation && !ModMap) {
+    if (!bool(PP.getPreprocessorOpts().DisablePCHOrModuleValidation &
+              DisableValidationForModuleKind::Module) &&
+        !ModMap) {
       if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) {
         if (auto ASTFE = M ? M->getASTFile() : None) {
           // This module was defined by an imported (explicit) module.
           Diag(diag::err_module_file_conflict) << F.ModuleName << F.FileName
                                                << ASTFE->getName();
         } else {
           // This module was built with a different module map.
           Diag(diag::err_imported_module_not_found)

 
 ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName,
                                             ModuleKind Type,
                                             SourceLocation ImportLoc,
                                             unsigned ClientLoadCapabilities,
                                             SmallVectorImpl<ImportedSubmodule> *Imported) {
   llvm::SaveAndRestore<SourceLocation>
     SetCurImportLocRAII(CurrentImportLoc, ImportLoc);
+  llvm::SaveAndRestore<Optional<ModuleKind>> SetCurModuleKindRAII(
+      CurrentDeserializingModuleKind, Type);
 
   // Defer any pending actions until we get to the end of reading the AST file.
   Deserializing AnASTFile(this);
 
   // Bump the generation number.
   unsigned PreviousGeneration = 0;
   if (ContextObj)
     PreviousGeneration = incrementGeneration(*ContextObj);

 
 ASTReader::ASTReadResult
 ASTReader::readUnhashedControlBlock(ModuleFile &F, bool WasImportedBy,
                                     unsigned ClientLoadCapabilities) {
   const HeaderSearchOptions &HSOpts =
       PP.getHeaderSearchInfo().getHeaderSearchOpts();
   bool AllowCompatibleConfigurationMismatch =
       F.Kind == MK_ExplicitModule || F.Kind == MK_PrebuiltModule;
+  bool DisableValidation = shouldDisableValidationForFile(F);
 
   ASTReadResult Result = readUnhashedControlBlockImpl(
       &F, F.Data, ClientLoadCapabilities, AllowCompatibleConfigurationMismatch,
       Listener.get(),
       WasImportedBy ? false : HSOpts.ModulesValidateDiagnosticOptions);
 
   // If F was directly imported by another module, it's implicitly validated by
   // the importing module.
case SUBMODULE_DEFINITION: {
           SubmodulesLoaded[GlobalIndex]) {
         Error("too many submodules");
         return Failure;
       }
 
       if (!ParentModule) {
         if (const FileEntry *CurFile = CurrentModule->getASTFile()) {
           // Don't emit module relocation error if we have -fno-validate-pch
-          if (!PP.getPreprocessorOpts().DisablePCHValidation &&
+          if (!bool(PP.getPreprocessorOpts().DisablePCHOrModuleValidation &
+                    DisableValidationForModuleKind::Module) &&
               CurFile != F.File) {
             Error(diag::err_module_file_conflict,
                   CurrentModule->getTopLevelModuleName(), CurFile->getName(),
                   F.File->getName());
             return Failure;
           }
         }
 
if (std::find(SemaObj->IdResolver.begin(Name),
       SemaObj->TUScope->AddDecl(D);
   }
 }
 
 ASTReader::ASTReader(Preprocessor &PP, InMemoryModuleCache &ModuleCache,
                      ASTContext *Context,
                      const PCHContainerReader &PCHContainerRdr,
                      ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
-                     StringRef isysroot, bool DisableValidation,
+                     StringRef isysroot,
+                     DisableValidationForModuleKind DisableValidationKind,
                      bool AllowASTWithCompilerErrors,
                      bool AllowConfigurationMismatch, bool ValidateSystemInputs,
                      bool ValidateASTInputFilesContent, bool UseGlobalIndex,
                      std::unique_ptr<llvm::Timer> ReadTimer)
-    : Listener(DisableValidation
+    : Listener(bool(DisableValidationKind &DisableValidationForModuleKind::PCH)
                    ? cast<ASTReaderListener>(new SimpleASTReaderListener(PP))
                    : cast<ASTReaderListener>(new PCHValidator(PP, *this))),
       SourceMgr(PP.getSourceManager()), FileMgr(PP.getFileManager()),
       PCHContainerRdr(PCHContainerRdr), Diags(PP.getDiagnostics()), PP(PP),
       ContextObj(Context), ModuleMgr(PP.getFileManager(), ModuleCache,
                                      PCHContainerRdr, PP.getHeaderSearchInfo()),
       DummyIdResolver(PP), ReadTimer(std::move(ReadTimer)), isysroot(isysroot),
-      DisableValidation(DisableValidation),
+      DisableValidationKind(DisableValidationKind),
       AllowASTWithCompilerErrors(AllowASTWithCompilerErrors),
       AllowConfigurationMismatch(AllowConfigurationMismatch),
       ValidateSystemInputs(ValidateSystemInputs),
       ValidateASTInputFilesContent(ValidateASTInputFilesContent),
       UseGlobalIndex(UseGlobalIndex), CurrSwitchCaseStmts(&SwitchCaseStmts) {
   SourceMgr.setExternalSLocEntrySource(this);
 
   for (const auto &Ext : Extensions) {

clang/test/Index/Inputs/preamble-reparse-changed-module/head.h

+@interface I
+- (void)call_me;
+@end

clang/test/Index/Inputs/preamble-reparse-changed-module/module.modulemap

+module mod {
+  header "head.h"
+}

clang/test/Index/Inputs/preamble-reparse-changed-module/new-head.h

+@interface I
+- (void)call_me_new;
+@end

clang/test/Index/preamble-reparse-changed-module.m

+// REQUIRES: shell
+
+// RUN: mkdir -p %t/mod
+// RUN: touch %t/empty.h
+// RUN: cp %S/Inputs/preamble-reparse-changed-module/module.modulemap %t/mod
+// RUN: cp %S/Inputs/preamble-reparse-changed-module/head.h %t/mod
+
+// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_EXECUTE_COMMAND="cp %S/Inputs/preamble-reparse-changed-module/new-head.h %t/mod/head.h" CINDEXTEST_EXECUTE_AFTER_TRIAL=1 \
+// RUN:     c-index-test -test-load-source-reparse 3 local %s -I %t -I %t/mod -fmodules -fmodules-cache-path=%t/mcp 2>&1 | FileCheck %s
+
+// CHECK-NOT: warning:
+
+#include "empty.h"
+@import mod;
+
+void test(I *o) {
+  [o call_me_new];
+}

clang/tools/c-index-test/c-index-test.c

 
 #ifdef _WIN32
 #  include <direct.h>
 #else
 #  include <unistd.h>
 #endif
 
 extern int indextest_core_main(int argc, const char **argv);
+extern int indextest_perform_shell_execution(const char *command_line);
 
 /******************************************************************************/
 /* Utility functions.                                                         */
 /******************************************************************************/
 
 #ifdef _MSC_VER
 char *basename(const char* path)
 {
int perform_test_reparse_source(int argc, const char **argv, int trials,
   CXIndex Idx;
   CXTranslationUnit TU;
   struct CXUnsavedFile *unsaved_files = 0;
   int num_unsaved_files = 0;
   int compiler_arg_idx = 0;
   enum CXErrorCode Err;
   int result, i;
   int trial;
+  int execute_after_trial = 0;
+  const char *execute_command = NULL;
   int remap_after_trial = 0;
   char *endptr = 0;
   
   Idx = clang_createIndex(/* excludeDeclsFromPCH */
                           !strcmp(filter, "local") ? 1 : 0,
                           /* displayDiagnostics=*/1);
   
   if (parse_remapped_files(argc, argv, 0, &unsaved_files, &num_unsaved_files)) {
if (Err != CXError_Success) {
     free_remapped_files(unsaved_files, num_unsaved_files);
     clang_disposeIndex(Idx);
     return 1;
   }
   
   if (checkForErrors(TU) != 0)
     return -1;
 
+  if (getenv("CINDEXTEST_EXECUTE_COMMAND")) {
+    execute_command = getenv("CINDEXTEST_EXECUTE_COMMAND");
+  }
+  if (getenv("CINDEXTEST_EXECUTE_AFTER_TRIAL")) {
+    execute_after_trial =
+        strtol(getenv("CINDEXTEST_EXECUTE_AFTER_TRIAL"), &endptr, 10);
+  }
+
   if (getenv("CINDEXTEST_REMAP_AFTER_TRIAL")) {
     remap_after_trial =
         strtol(getenv("CINDEXTEST_REMAP_AFTER_TRIAL"), &endptr, 10);
   }
 
   for (trial = 0; trial < trials; ++trial) {
+    if (execute_command && trial == execute_after_trial) {
+      result = indextest_perform_shell_execution(execute_command);
+      if (result != 0)
+        return result;
+    }
+
     free_remapped_files(unsaved_files, num_unsaved_files);
     if (parse_remapped_files_with_try(trial, argc, argv, 0,
                                       &unsaved_files, &num_unsaved_files)) {
       clang_disposeTranslationUnit(TU);
       clang_disposeIndex(Idx);
       return -1;
     }
 

clang/tools/c-index-test/core_main.cpp

 //===-- core_main.cpp - Core Index Tool testbed ---------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/Mangle.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendAction.h"
-#include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
 #include "clang/Index/USRGeneration.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Serialization/ASTReader.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
-#include "llvm/Support/PrettyStackTrace.h"
 
 using namespace clang;
 using namespace clang::index;
 using namespace llvm;
 
 extern "C" int indextest_core_main(int argc, const char **argv);
+extern "C" int indextest_perform_shell_execution(const char *command_line);
 
 namespace {
 
 enum class ActionType {
   None,
   PrintSourceSymbols,
 };
 
if (options::Action == ActionType::PrintSourceSymbols) {
     }
     return printSourceSymbols(Executable.c_str(), CompArgs,
                               options::DumpModuleImports,
                               options::IncludeLocals);
   }
 
   return 0;
 }
+
+//===----------------------------------------------------------------------===//
+// Utility functions
+//===----------------------------------------------------------------------===//
+
+int indextest_perform_shell_execution(const char *command_line) {
+  BumpPtrAllocator Alloc;
+  llvm::StringSaver Saver(Alloc);
+  SmallVector<const char *, 4> Args;
+  llvm::cl::TokenizeGNUCommandLine(command_line, Saver, Args);
+  auto Program = llvm::sys::findProgramByName(Args[0]);
+  if (std::error_code ec = Program.getError()) {
+    llvm::errs() << "command not found: " << Args[0] << "\n";
+    return ec.value();
+  }
+  SmallVector<StringRef, 8> execArgs(Args.begin(), Args.end());
+  return llvm::sys::ExecuteAndWait(*Program, execArgs);
+}