r374841 - [Modules][PCH] Hash input files content

Bruno Cardoso Lopes via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 14 16:17:13 PDT 2019


Thanks Eric!

On Mon, Oct 14, 2019 at 8:12 PM Eric Christopher <echristo at gmail.com> wrote:
>
> This was breaking a few bots and I couldn't find you on irc so I've
> reverted it thusly:
>
> echristo at jhereg ~/s/llvm-project> git llvm push
> Pushing 1 commit:
>   175b1b856ea Temporarily Revert [Modules][PCH] Hash input files
> content as it's breaking a few bots.
> Sending        cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
> Sending        cfe/trunk/include/clang/Driver/Options.td
> Sending        cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
> Sending        cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> Sending        cfe/trunk/include/clang/Serialization/ASTReader.h
> Sending        cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> Sending        cfe/trunk/lib/Frontend/CompilerInstance.cpp
> Sending        cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> Sending        cfe/trunk/lib/Serialization/ASTReader.cpp
> Sending        cfe/trunk/lib/Serialization/ASTWriter.cpp
> Deleting       cfe/trunk/test/Modules/validate-file-content.m
> Deleting       cfe/trunk/test/PCH/validate-file-content.m
> Transmitting file data ..........done
> Committing transaction...
> Committed revision 374842.
> Committed 175b1b856ea to svn.
>
> Sorry for the inconvenience!
>
> -eric
>
> On Mon, Oct 14, 2019 at 3:59 PM Bruno Cardoso Lopes via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> >
> > Author: bruno
> > Date: Mon Oct 14 16:02:03 2019
> > New Revision: 374841
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=374841&view=rev
> > Log:
> > [Modules][PCH] Hash input files content
> >
> > Summary:
> > When files often get touched during builds, the mtime based validation
> > leads to different problems in implicit modules builds, even when the
> > content doesn't actually change:
> >
> > - Modules only: module invalidation due to out of date files. Usually causing rebuild traffic.
> > - Modules + PCH: build failures because clang cannot rebuild a module if it comes from building a PCH.
> > - PCH: build failures because clang cannot rebuild a PCH in case one of the input headers has different mtime.
> >
> > This patch proposes hashing the content of input files (headers and
> > module maps), which is performed during serialization time. When looking
> > at input files for validation, clang only computes the hash in case
> > there's a mtime mismatch.
> >
> > I've tested a couple of different hash algorithms availble in LLVM in
> > face of building modules+pch for `#import <Cocoa/Cocoa.h>`:
> > - `hash_code`: performace diff within the noise, total module cache increased by 0.07%.
> > - `SHA1`: 5% slowdown. Haven't done real size measurements, but it'd be BLOCK_ID+20 bytes per input file, instead of BLOCK_ID+8 bytes from `hash_code`.
> > - `MD5`: 3% slowdown. Like above, but BLOCK_ID+16 bytes per input file.
> >
> > Given the numbers above, the patch uses `hash_code`. The patch also
> > improves invalidation error msgs to point out which type of problem the
> > user is facing: "mtime", "size" or "content".
> >
> > rdar://problem/29320105
> >
> > Reviewers: dexonsmith, arphaman, rsmith, aprantl
> >
> > Subscribers: jkorous, cfe-commits, ributzka
> >
> > Tags: #clang
> >
> > Differential Revision: https://reviews.llvm.org/D67249
> >
> > Added:
> >     cfe/trunk/test/Modules/validate-file-content.m
> >     cfe/trunk/test/PCH/validate-file-content.m
> > Modified:
> >     cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
> >     cfe/trunk/include/clang/Driver/Options.td
> >     cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
> >     cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> >     cfe/trunk/include/clang/Serialization/ASTReader.h
> >     cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> >     cfe/trunk/lib/Frontend/CompilerInstance.cpp
> >     cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> >     cfe/trunk/lib/Serialization/ASTReader.cpp
> >     cfe/trunk/lib/Serialization/ASTWriter.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
> > +++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Mon Oct 14 16:02:03 2019
> > @@ -18,13 +18,16 @@ def err_fe_pch_malformed : Error<
> >  def err_fe_pch_malformed_block : Error<
> >      "malformed block record in PCH file: '%0'">, DefaultFatal;
> >  def err_fe_pch_file_modified : Error<
> > -    "file '%0' has been modified since the precompiled header '%1' was built">,
> > +    "file '%0' has been modified since the precompiled header '%1' was built"
> > +    ": %select{size|mtime|content}2 changed">,
> >      DefaultFatal;
> >  def err_fe_module_file_modified : Error<
> > -    "file '%0' has been modified since the module file '%1' was built">,
> > +    "file '%0' has been modified since the module file '%1' was built"
> > +    ": %select{size|mtime|content}2 changed">,
> >      DefaultFatal;
> >  def err_fe_ast_file_modified : Error<
> > -    "file '%0' has been modified since the AST file '%1' was built">,
> > +    "file '%0' has been modified since the AST file '%1' was built"
> > +    ": %select{size|mtime|content}2 changed">,
> >      DefaultFatal;
> >  def err_fe_pch_file_overridden : Error<
> >      "file '%0' from the precompiled header has been overridden">;
> > @@ -399,6 +402,8 @@ def warn_module_uses_date_time : Warning
> >  def err_module_no_size_mtime_for_header : Error<
> >    "cannot emit module %0: %select{size|mtime}1 must be explicitly specified "
> >    "for missing header file \"%2\"">;
> > +def err_module_unable_to_hash_content : Error<
> > +  "failed to hash content for '%0' because memory buffer cannot be retrieved">;
> >  } // let CategoryName
> >  } // let Component
> >
> >
> > Modified: cfe/trunk/include/clang/Driver/Options.td
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/include/clang/Driver/Options.td (original)
> > +++ cfe/trunk/include/clang/Driver/Options.td Mon Oct 14 16:02:03 2019
> > @@ -1368,6 +1368,28 @@ def fmodules_validate_system_headers : F
> >    HelpText<"Validate the system headers that a module depends on when loading the module">;
> >  def fno_modules_validate_system_headers : Flag<["-"], "fno-modules-validate-system-headers">,
> >    Group<i_Group>, Flags<[DriverOption]>;
> > +
> > +def fvalidate_ast_input_files_content:
> > +  Flag <["-"], "fvalidate-ast-input-files-content">,
> > +  Group<f_Group>, Flags<[CC1Option]>,
> > +  HelpText<"Compute and store the hash of input files used to build an AST."
> > +           " Files with mismatching mtime's are considered valid"
> > +           " if both contents is identical">;
> > +def fmodules_validate_input_files_content:
> > +  Flag <["-"], "fmodules-validate-input-files-content">,
> > +  Group<f_Group>, Flags<[DriverOption]>,
> > +  HelpText<"Validate PCM input files based on content if mtime differs">;
> > +def fno_modules_validate_input_files_content:
> > +  Flag <["-"], "fno_modules-validate-input-files-content">,
> > +  Group<f_Group>, Flags<[DriverOption]>;
> > +def fpch_validate_input_files_content:
> > +  Flag <["-"], "fpch-validate-input-files-content">,
> > +  Group<f_Group>, Flags<[DriverOption]>,
> > +  HelpText<"Validate PCH input files based on content if mtime differs">;
> > +def fno_pch_validate_input_files_content:
> > +  Flag <["-"], "fno_pch-validate-input-files-content">,
> > +  Group<f_Group>, Flags<[DriverOption]>;
> > +
> >  def fmodules : Flag <["-"], "fmodules">, Group<f_Group>,
> >    Flags<[DriverOption, CC1Option]>,
> >    HelpText<"Enable the 'modules' language feature">;
> >
> > Modified: cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearchOptions.h?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/include/clang/Lex/HeaderSearchOptions.h (original)
> > +++ cfe/trunk/include/clang/Lex/HeaderSearchOptions.h Mon Oct 14 16:02:03 2019
> > @@ -195,6 +195,10 @@ public:
> >    /// Whether to validate system input files when a module is loaded.
> >    unsigned ModulesValidateSystemHeaders : 1;
> >
> > +  // Whether the content of input files should be hashed and used to
> > +  // validate consistency.
> > +  unsigned ValidateASTInputFilesContent : 1;
> > +
> >    /// Whether the module includes debug information (-gmodules).
> >    unsigned UseDebugInfo : 1;
> >
> > @@ -208,7 +212,8 @@ public:
> >          UseBuiltinIncludes(true), UseStandardSystemIncludes(true),
> >          UseStandardCXXIncludes(true), UseLibcxx(false), Verbose(false),
> >          ModulesValidateOncePerBuildSession(false),
> > -        ModulesValidateSystemHeaders(false), UseDebugInfo(false),
> > +        ModulesValidateSystemHeaders(false),
> > +        ValidateASTInputFilesContent(false), UseDebugInfo(false),
> >          ModulesValidateDiagnosticOptions(true), ModulesHashContent(false) {}
> >
> >    /// AddPath - Add the \p Path path to the specified \p Group list.
> >
> > Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
> > +++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Mon Oct 14 16:02:03 2019
> > @@ -382,7 +382,10 @@ namespace serialization {
> >      /// inside the control block.
> >      enum InputFileRecordTypes {
> >        /// An input file.
> > -      INPUT_FILE = 1
> > +      INPUT_FILE = 1,
> > +
> > +      /// The input file content hash
> > +      INPUT_FILE_HASH
> >      };
> >
> >      /// Record types that occur within the AST block itself.
> >
> > Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
> > +++ cfe/trunk/include/clang/Serialization/ASTReader.h Mon Oct 14 16:02:03 2019
> > @@ -930,6 +930,9 @@ private:
> >    /// Whether validate system input files.
> >    bool ValidateSystemInputs;
> >
> > +  /// Whether validate headers and module maps using hash based on contents.
> > +  bool ValidateASTInputFilesContent;
> > +
> >    /// Whether we are allowed to use the global module index.
> >    bool UseGlobalIndex;
> >
> > @@ -1203,6 +1206,7 @@ private:
> >
> >    struct InputFileInfo {
> >      std::string Filename;
> > +    uint64_t ContentHash;
> >      off_t StoredSize;
> >      time_t StoredTime;
> >      bool Overridden;
> > @@ -1437,6 +1441,8 @@ private:
> >    void Error(StringRef Msg) const;
> >    void Error(unsigned DiagID, StringRef Arg1 = StringRef(),
> >               StringRef Arg2 = StringRef()) const;
> > +  void Error(unsigned DiagID, StringRef Arg1, StringRef Arg2,
> > +             unsigned Select) const;
> >    void Error(llvm::Error &&Err) const;
> >
> >  public:
> > @@ -1485,7 +1491,9 @@ public:
> >              StringRef isysroot = "", bool DisableValidation = false,
> >              bool AllowASTWithCompilerErrors = false,
> >              bool AllowConfigurationMismatch = false,
> > -            bool ValidateSystemInputs = false, bool UseGlobalIndex = true,
> > +            bool ValidateSystemInputs = false,
> > +            bool ValidateASTInputFilesContent = false,
> > +            bool UseGlobalIndex = true,
> >              std::unique_ptr<llvm::Timer> ReadTimer = {});
> >    ASTReader(const ASTReader &) = delete;
> >    ASTReader &operator=(const ASTReader &) = delete;
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
> > +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Oct 14 16:02:03 2019
> > @@ -2777,6 +2777,10 @@ static void RenderModulesOptions(Compila
> >            std::string("-fprebuilt-module-path=") + A->getValue()));
> >        A->claim();
> >      }
> > +    if (Args.hasFlag(options::OPT_fmodules_validate_input_files_content,
> > +                     options::OPT_fno_modules_validate_input_files_content,
> > +                     false))
> > +      CmdArgs.push_back("-fvalidate-ast-input-files-content");
> >    }
> >
> >    // -fmodule-name specifies the module that is currently being built (or
> > @@ -4899,6 +4903,10 @@ void Clang::ConstructJob(Compilation &C,
> >        Std && (Std->containsValue("c++2a") || Std->containsValue("c++latest"));
> >    RenderModulesOptions(C, D, Args, Input, Output, CmdArgs, HaveModules);
> >
> > +  if (Args.hasFlag(options::OPT_fpch_validate_input_files_content,
> > +                   options::OPT_fno_pch_validate_input_files_content, false))
> > +    CmdArgs.push_back("-fvalidate-ast-input-files-content");
> > +
> >    Args.AddLastArg(CmdArgs, options::OPT_fexperimental_new_pass_manager,
> >                    options::OPT_fno_experimental_new_pass_manager);
> >
> >
> > Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
> > +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Oct 14 16:02:03 2019
> > @@ -510,7 +510,8 @@ IntrusiveRefCntPtr<ASTReader> CompilerIn
> >        PP, ModuleCache, &Context, PCHContainerRdr, Extensions,
> >        Sysroot.empty() ? "" : Sysroot.data(), DisablePCHValidation,
> >        AllowPCHWithCompilerErrors, /*AllowConfigurationMismatch*/ false,
> > -      HSOpts.ModulesValidateSystemHeaders, UseGlobalModuleIndex));
> > +      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.
> > @@ -1492,6 +1493,7 @@ void CompilerInstance::createModuleManag
> >          /*AllowASTWithCompilerErrors=*/false,
> >          /*AllowConfigurationMismatch=*/false,
> >          HSOpts.ModulesValidateSystemHeaders,
> > +        HSOpts.ValidateASTInputFilesContent,
> >          getFrontendOpts().UseGlobalModuleIndex, std::move(ReadTimer));
> >      if (hasASTConsumer()) {
> >        ModuleManager->setDeserializationListener(
> >
> > Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
> > +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon Oct 14 16:02:03 2019
> > @@ -2081,6 +2081,8 @@ static void ParseHeaderSearchArgs(Header
> >        getLastArgUInt64Value(Args, OPT_fbuild_session_timestamp, 0);
> >    Opts.ModulesValidateSystemHeaders =
> >        Args.hasArg(OPT_fmodules_validate_system_headers);
> > +  Opts.ValidateASTInputFilesContent =
> > +      Args.hasArg(OPT_fvalidate_ast_input_files_content);
> >    if (const Arg *A = Args.getLastArg(OPT_fmodule_format_EQ))
> >      Opts.ModuleFormat = A->getValue();
> >
> >
> > Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> > +++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Oct 14 16:02:03 2019
> > @@ -1247,6 +1247,12 @@ void ASTReader::Error(unsigned DiagID,
> >      Diag(DiagID) << Arg1 << Arg2;
> >  }
> >
> > +void ASTReader::Error(unsigned DiagID, StringRef Arg1, StringRef Arg2,
> > +                      unsigned Select) const {
> > +  if (!Diags.isDiagnosticInFlight())
> > +    Diag(DiagID) << Arg1 << Arg2 << Select;
> > +}
> > +
> >  void ASTReader::Error(llvm::Error &&Err) const {
> >    Error(toString(std::move(Err)));
> >  }
> > @@ -2241,6 +2247,24 @@ ASTReader::readInputFileInfo(ModuleFile
> >    R.TopLevelModuleMap = static_cast<bool>(Record[5]);
> >    R.Filename = Blob;
> >    ResolveImportedPath(F, R.Filename);
> > +
> > +  Expected<llvm::BitstreamEntry> MaybeEntry = Cursor.advance();
> > +  if (!MaybeEntry) // FIXME this drops errors on the floor.
> > +    consumeError(MaybeEntry.takeError());
> > +  llvm::BitstreamEntry Entry = MaybeEntry.get();
> > +  assert(Entry.Kind == llvm::BitstreamEntry::Record &&
> > +         "expected record type for input file hash");
> > +
> > +  Record.clear();
> > +  if (Expected<unsigned> Maybe = Cursor.readRecord(Entry.ID, Record))
> > +    assert(static_cast<InputFileRecordTypes>(Maybe.get()) == INPUT_FILE_HASH &&
> > +           "invalid record type for input file hash");
> > +  else {
> > +    // FIXME this drops errors on the floor.
> > +    consumeError(Maybe.takeError());
> > +  }
> > +  R.ContentHash = (static_cast<uint64_t>(Record[1]) << 32) |
> > +                  static_cast<uint64_t>(Record[0]);
> >    return R;
> >  }
> >
> > @@ -2271,6 +2295,7 @@ InputFile ASTReader::getInputFile(Module
> >    bool Overridden = FI.Overridden;
> >    bool Transient = FI.Transient;
> >    StringRef Filename = FI.Filename;
> > +  uint64_t StoredContentHash = FI.ContentHash;
> >
> >    const FileEntry *File = nullptr;
> >    if (auto FE = FileMgr.getFile(Filename, /*OpenFile=*/false))
> > @@ -2325,14 +2350,46 @@ InputFile ASTReader::getInputFile(Module
> >      }
> >    }
> >
> > -  bool IsOutOfDate = false;
> > +  enum ModificationType {
> > +    Size,
> > +    ModTime,
> > +    Content,
> > +    None,
> > +  };
> > +  auto HasInputFileChanged = [&]() {
> > +    if (StoredSize != File->getSize())
> > +      return ModificationType::Size;
> > +    if (!DisableValidation && 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)
> > +            return ModificationType::ModTime;
> > +          std::string ErrorStr = "could not get buffer for file '";
> > +          ErrorStr += File->getName();
> > +          ErrorStr += "'";
> > +          Error(ErrorStr);
> > +          return ModificationType::ModTime;
> > +        }
> > +
> > +        auto ContentHash = hash_value(MemBuffOrError.get()->getBuffer());
> > +        if (StoredContentHash == static_cast<uint64_t>(ContentHash))
> > +          return ModificationType::None;
> > +        return ModificationType::Content;
> > +      }
> > +      return ModificationType::ModTime;
> > +    }
> > +    return ModificationType::None;
> > +  };
> >
> > +  bool IsOutOfDate = false;
> > +  auto FileChange = HasInputFileChanged();
> >    // For an overridden file, there is nothing to validate.
> > -  if (!Overridden && //
> > -      (StoredSize != File->getSize() ||
> > -       (StoredTime && StoredTime != File->getModificationTime() &&
> > -        !DisableValidation)
> > -       )) {
> > +  if (!Overridden && FileChange != ModificationType::None) {
> >      if (Complain) {
> >        // Build a list of the PCH imports that got us here (in reverse).
> >        SmallVector<ModuleFile *, 4> ImportStack(1, &F);
> > @@ -2341,13 +2398,17 @@ InputFile ASTReader::getInputFile(Module
> >
> >        // The top-level PCH is stale.
> >        StringRef TopLevelPCHName(ImportStack.back()->FileName);
> > -      unsigned DiagnosticKind = moduleKindForDiagnostic(ImportStack.back()->Kind);
> > +      unsigned DiagnosticKind =
> > +          moduleKindForDiagnostic(ImportStack.back()->Kind);
> >        if (DiagnosticKind == 0)
> > -        Error(diag::err_fe_pch_file_modified, Filename, TopLevelPCHName);
> > +        Error(diag::err_fe_pch_file_modified, Filename, TopLevelPCHName,
> > +              (unsigned)FileChange);
> >        else if (DiagnosticKind == 1)
> > -        Error(diag::err_fe_module_file_modified, Filename, TopLevelPCHName);
> > +        Error(diag::err_fe_module_file_modified, Filename, TopLevelPCHName,
> > +              (unsigned)FileChange);
> >        else
> > -        Error(diag::err_fe_ast_file_modified, Filename, TopLevelPCHName);
> > +        Error(diag::err_fe_ast_file_modified, Filename, TopLevelPCHName,
> > +              (unsigned)FileChange);
> >
> >        // Print the import stack.
> >        if (ImportStack.size() > 1 && !Diags.isDiagnosticInFlight()) {
> > @@ -5192,6 +5253,8 @@ bool ASTReader::readASTFileControlBlock(
> >            consumeError(MaybeRecordType.takeError());
> >          }
> >          switch ((InputFileRecordTypes)MaybeRecordType.get()) {
> > +        case INPUT_FILE_HASH:
> > +          break;
> >          case INPUT_FILE:
> >            bool Overridden = static_cast<bool>(Record[3]);
> >            std::string Filename = Blob;
> > @@ -12153,7 +12216,7 @@ ASTReader::ASTReader(Preprocessor &PP, I
> >                       StringRef isysroot, bool DisableValidation,
> >                       bool AllowASTWithCompilerErrors,
> >                       bool AllowConfigurationMismatch, bool ValidateSystemInputs,
> > -                     bool UseGlobalIndex,
> > +                     bool ValidateASTInputFilesContent, bool UseGlobalIndex,
> >                       std::unique_ptr<llvm::Timer> ReadTimer)
> >      : Listener(DisableValidation
> >                     ? cast<ASTReaderListener>(new SimpleASTReaderListener(PP))
> > @@ -12167,6 +12230,7 @@ ASTReader::ASTReader(Preprocessor &PP, I
> >        AllowASTWithCompilerErrors(AllowASTWithCompilerErrors),
> >        AllowConfigurationMismatch(AllowConfigurationMismatch),
> >        ValidateSystemInputs(ValidateSystemInputs),
> > +      ValidateASTInputFilesContent(ValidateASTInputFilesContent),
> >        UseGlobalIndex(UseGlobalIndex), CurrSwitchCaseStmts(&SwitchCaseStmts) {
> >    SourceMgr.setExternalSLocEntrySource(this);
> >
> >
> > Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=374841&r1=374840&r2=374841&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> > +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Mon Oct 14 16:02:03 2019
> > @@ -1099,6 +1099,7 @@ void ASTWriter::WriteBlockInfoBlock() {
> >
> >    BLOCK(INPUT_FILES_BLOCK);
> >    RECORD(INPUT_FILE);
> > +  RECORD(INPUT_FILE_HASH);
> >
> >    // AST Top-Level Block.
> >    BLOCK(AST_BLOCK);
> > @@ -1764,6 +1765,7 @@ struct InputFileEntry {
> >    bool IsTransient;
> >    bool BufferOverridden;
> >    bool IsTopLevelModuleMap;
> > +  uint32_t ContentHash[2];
> >  };
> >
> >  } // namespace
> > @@ -1787,6 +1789,13 @@ void ASTWriter::WriteInputFiles(SourceMa
> >    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;
> > @@ -1810,6 +1819,25 @@ void ASTWriter::WriteInputFiles(SourceMa
> >      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
> > @@ -1834,17 +1862,26 @@ void ASTWriter::WriteInputFiles(SourceMa
> >
> >      // Emit size/modification time for this file.
> >      // And whether this file was overridden.
> > -    RecordData::value_type Record[] = {
> > -        INPUT_FILE,
> > -        InputFileOffsets.size(),
> > -        (uint64_t)Entry.File->getSize(),
> > -        (uint64_t)getTimestampForOutput(Entry.File),
> > -        Entry.BufferOverridden,
> > -        Entry.IsTransient,
> > -        Entry.IsTopLevelModuleMap};
> > +    {
> > +      RecordData::value_type Record[] = {
> > +          INPUT_FILE,
> > +          InputFileOffsets.size(),
> > +          (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());
> > +    }
> >
> > -    // FIXME: The path should be taken from the FileEntryRef.
> > -    EmitRecordWithPath(IFAbbrevCode, Record, Entry.File->getName());
> > +    // Emit content hash for this file.
> > +    {
> > +      RecordData::value_type Record[] = {INPUT_FILE_HASH, Entry.ContentHash[0],
> > +                                         Entry.ContentHash[1]};
> > +      Stream.EmitRecordWithAbbrev(IFHAbbrevCode, Record);
> > +    }
> >    }
> >
> >    Stream.ExitBlock();
> >
> > Added: cfe/trunk/test/Modules/validate-file-content.m
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/validate-file-content.m?rev=374841&view=auto
> > ==============================================================================
> > --- cfe/trunk/test/Modules/validate-file-content.m (added)
> > +++ cfe/trunk/test/Modules/validate-file-content.m Mon Oct 14 16:02:03 2019
> > @@ -0,0 +1,33 @@
> > +// REQUIRES: shell
> > +//
> > +// Check driver works
> > +// RUN: %clang -fmodules -fsyntax-only -fmodules-validate-input-files-content %s -### 2>&1 | FileCheck --check-prefix=CHECK-CC1 %s
> > +// CHECK-CC1: -fvalidate-ast-input-files-content
> > +//
> > +// PCH+Modules: Test that a mtime mismatch without content change is fine
> > +// RUN: rm -rf %t
> > +// RUN: mkdir %t
> > +// RUN: echo '// m.h' > %t/m.h
> > +// RUN: echo '#include "m.h"' > %t/a.h
> > +// RUN: echo 'module m { header "m.h" }' > %t/module.modulemap
> > +// RUN: %clang_cc1 -emit-pch -fmodules-cache-path=%t/cache -fmodules -fimplicit-module-maps -o %t/a.pch -I %t -x objective-c-header %t/a.h -fvalidate-ast-input-files-content
> > +// RUN: touch -m -a -t 202901010000 %t/m.h
> > +// RUN: %clang_cc1 -fsyntax-only -fmodules-cache-path=%t/cache -fmodules -fimplicit-module-maps -I %t -include-pch %t/a.pch %s -verify -fvalidate-ast-input-files-content
> > +//
> > +// PCH+Modules: Test that a mtime mismatch with content change
> > +// RUN: rm -rf %t
> > +// RUN: mkdir %t
> > +// RUN: echo '// m.h' > %t/m.h
> > +// RUN: echo '#include "m.h"' > %t/a.h
> > +// RUN: echo 'module m { header "m.h" }' > %t/module.modulemap
> > +// RUN: %clang_cc1 -emit-pch -fmodules-cache-path=%t/cache -fmodules -fimplicit-module-maps -o %t/a.pch -I %t -x objective-c-header %t/a.h -fvalidate-ast-input-files-content
> > +// RUN: echo '// m.x' > %t/m.h
> > +// RUN: touch -m -a -t 202901010000 %t/m.h
> > +// RUN: not %clang_cc1 -fsyntax-only -fmodules-cache-path=%t/cache -fmodules -fimplicit-module-maps -I %t -include-pch %t/a.pch %s -fvalidate-ast-input-files-content 2> %t/stderr
> > +// RUN: FileCheck %s < %t/stderr
> > +//
> > +// CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*/a\.pch]]' was built: content changed
> > +// CHECK: '[[M_H]]' required by '[[M_PCM:.*[/\\]m.*\.pcm]]'
> > +// CHECK: '[[M_PCM]]' required by '[[A_PCH]]'
> > +// CHECK: please rebuild precompiled header '[[A_PCH]]'
> > +// expected-no-diagnostics
> >
> > Added: cfe/trunk/test/PCH/validate-file-content.m
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/validate-file-content.m?rev=374841&view=auto
> > ==============================================================================
> > --- cfe/trunk/test/PCH/validate-file-content.m (added)
> > +++ cfe/trunk/test/PCH/validate-file-content.m Mon Oct 14 16:02:03 2019
> > @@ -0,0 +1,29 @@
> > +// REQUIRES: shell
> > +//
> > +// Check driver works
> > +// RUN: %clang -x objective-c-header -fsyntax-only -fpch-validate-input-files-content %t/a.h -### 2>&1 | FileCheck --check-prefix=CHECK-CC1 %s
> > +// CHECK-CC1: -fvalidate-ast-input-files-content
> > +//
> > +// PCH only: Test that a mtime mismatch without content change is fine
> > +// RUN: rm -rf %t
> > +// RUN: mkdir %t
> > +// RUN: echo '// m.h' > %t/m.h
> > +// RUN: echo '#include "m.h"' > %t/a.h
> > +// RUN: %clang_cc1 -emit-pch -o %t/a.pch -I %t -x objective-c-header %t/a.h -fvalidate-ast-input-files-content
> > +// RUN: touch -m -a -t 202901010000 %t/m.h
> > +// RUN: %clang_cc1 -fsyntax-only -I %t -include-pch %t/a.pch %s -verify -fvalidate-ast-input-files-content
> > +//
> > +// PCH only: Test that a mtime mismatch with content change
> > +// RUN: rm -rf %t
> > +// RUN: mkdir %t
> > +// RUN: echo '// m.h' > %t/m.h
> > +// RUN: echo '#include "m.h"' > %t/a.h
> > +// RUN: %clang_cc1 -emit-pch -o %t/a.pch -I %t -x objective-c-header %t/a.h -fvalidate-ast-input-files-content
> > +// RUN: echo '// m.x' > %t/m.h
> > +// RUN: touch -m -a -t 202901010000 %t/m.h
> > +// RUN: not %clang_cc1 -fsyntax-only -I %t -include-pch %t/a.pch %s -fvalidate-ast-input-files-content 2> %t/stderr
> > +// RUN: FileCheck %s < %t/stderr
> > +//
> > +// CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*/a\.pch]]' was built: content changed
> > +// CHECK: please rebuild precompiled header '[[A_PCH]]'
> > +// expected-no-diagnostics
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc


More information about the cfe-commits mailing list