llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Jan Svoboda (jansvoboda11) <details> <summary>Changes</summary> Force-validation of user headers was implemented in acb803e8 to deal with files changing during build. The dependency scanner guarantees an immutable file system during single build session, so the validation is unnecessary. (We don't hit the disk too often due to the caching VFS, but even avoiding going to the cache and deserializing the input files makes sense.) --- Full diff: https://github.com/llvm/llvm-project/pull/139091.diff 8 Files Affected: - (modified) clang/include/clang/Driver/Options.td (+7) - (modified) clang/include/clang/Lex/HeaderSearchOptions.h (+6) - (modified) clang/include/clang/Serialization/ASTReader.h (+5-1) - (modified) clang/lib/Frontend/CompilerInstance.cpp (+4-2) - (modified) clang/lib/Frontend/FrontendActions.cpp (+1-1) - (modified) clang/lib/Serialization/ASTReader.cpp (+3-1) - (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp (+1) - (modified) clang/test/Modules/fmodules-validate-once-per-build-session.c (+37-2) ``````````diff diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 30ea75bb108d5..9a9d71cf2d447 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3244,6 +3244,13 @@ def fmodules_disable_diagnostic_validation : Flag<["-"], "fmodules-disable-diagn Group<i_Group>, Visibility<[ClangOption, CC1Option]>, HelpText<"Disable validation of the diagnostic options when loading the module">, MarshallingInfoNegativeFlag<HeaderSearchOpts<"ModulesValidateDiagnosticOptions">>; +defm modules_force_validate_user_headers : BoolOption<"f", "modules-force-validate-user-headers", + HeaderSearchOpts<"ModulesForceValidateUserHeaders">, DefaultTrue, + PosFlag<SetTrue, [], [], "Force">, + NegFlag<SetFalse, [], [CC1Option], "Do not force">, + BothFlags<[], [ClangOption], + " validation of user headers when repeatedly loading a module file within single build session">>, + Group<i_Group>; defm modules_validate_system_headers : BoolOption<"f", "modules-validate-system-headers", HeaderSearchOpts<"ModulesValidateSystemHeaders">, DefaultFalse, PosFlag<SetTrue, [], [ClangOption, CC1Option], diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h index 68308f5693ac6..2f33c0749f02a 100644 --- a/clang/include/clang/Lex/HeaderSearchOptions.h +++ b/clang/include/clang/Lex/HeaderSearchOptions.h @@ -217,6 +217,11 @@ class HeaderSearchOptions { LLVM_PREFERRED_TYPE(bool) unsigned ModulesValidateSystemHeaders : 1; + /// Whether to force the validation of user input files when a module is + /// loaded (even despite the build session saying that is not necessary). + LLVM_PREFERRED_TYPE(bool) + unsigned ModulesForceValidateUserHeaders : 1; + // Whether the content of input files should be hashed and used to // validate consistency. LLVM_PREFERRED_TYPE(bool) @@ -286,6 +291,7 @@ class HeaderSearchOptions { UseStandardCXXIncludes(true), UseLibcxx(false), Verbose(false), ModulesValidateOncePerBuildSession(false), ModulesValidateSystemHeaders(false), + ModulesForceValidateUserHeaders(true), ValidateASTInputFilesContent(false), ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false), ModulesValidateDiagnosticOptions(true), diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 2765c827ece2b..57b0266af26bb 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1091,9 +1091,12 @@ class ASTReader /// from the current compiler instance. bool AllowConfigurationMismatch; - /// Whether validate system input files. + /// Whether to validate system input files. bool ValidateSystemInputs; + /// Whether to force the validation of user input files. + bool ForceValidateUserInputs; + /// Whether validate headers and module maps using hash based on contents. bool ValidateASTInputFilesContent; @@ -1767,6 +1770,7 @@ class ASTReader bool AllowASTWithCompilerErrors = false, bool AllowConfigurationMismatch = false, bool ValidateSystemInputs = false, + bool ForceValidateUserInputs = true, bool ValidateASTInputFilesContent = false, bool UseGlobalIndex = true, std::unique_ptr<llvm::Timer> ReadTimer = {}); diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index b59496babb62c..503d36467653e 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -638,8 +638,9 @@ IntrusiveRefCntPtr<ASTReader> CompilerInstance::createPCHExternalASTSource( PP, ModCache, &Context, PCHContainerRdr, Extensions, Sysroot.empty() ? "" : Sysroot.data(), DisableValidation, AllowPCHWithCompilerErrors, /*AllowConfigurationMismatch*/ false, - HSOpts.ModulesValidateSystemHeaders, HSOpts.ValidateASTInputFilesContent, - UseGlobalModuleIndex)); + HSOpts.ModulesValidateSystemHeaders, + HSOpts.ModulesForceValidateUserHeaders, + HSOpts.ValidateASTInputFilesContent, UseGlobalModuleIndex)); // We need the external source to be set up before we read the AST, because // eagerly-deserialized declarations may use it. @@ -1752,6 +1753,7 @@ void CompilerInstance::createASTReader() { PPOpts.DisablePCHOrModuleValidation, /*AllowASTWithCompilerErrors=*/FEOpts.AllowPCMWithCompilerErrors, /*AllowConfigurationMismatch=*/false, HSOpts.ModulesValidateSystemHeaders, + HSOpts.ModulesForceValidateUserHeaders, HSOpts.ValidateASTInputFilesContent, getFrontendOpts().UseGlobalModuleIndex, std::move(ReadTimer)); if (hasASTConsumer()) { diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 51e9f2c6b39a7..8c75e1a46da54 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -364,7 +364,7 @@ void VerifyPCHAction::ExecuteAction() { DisableValidationForModuleKind::None, /*AllowASTWithCompilerErrors*/ false, /*AllowConfigurationMismatch*/ true, - /*ValidateSystemInputs*/ true)); + /*ValidateSystemInputs*/ true, /*ForceValidateUserInputs*/ true)); Reader->ReadAST(getCurrentFile(), Preamble ? serialization::MK_Preamble diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index a17d6229ee3a1..d068f5e163176 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3105,7 +3105,7 @@ ASTReader::ReadControlBlock(ModuleFile &F, if (HSOpts.ModulesValidateOncePerBuildSession && F.InputFilesValidationTimestamp > HSOpts.BuildSessionTimestamp && F.Kind == MK_ImplicitModule) - N = NumUserInputs; + N = ForceValidateUserInputs ? NumUserInputs : 0; for (unsigned I = 0; I < N; ++I) { InputFile IF = getInputFile(F, I+1, Complain); @@ -10974,6 +10974,7 @@ ASTReader::ASTReader(Preprocessor &PP, ModuleCache &ModCache, DisableValidationForModuleKind DisableValidationKind, bool AllowASTWithCompilerErrors, bool AllowConfigurationMismatch, bool ValidateSystemInputs, + bool ForceValidateUserInputs, bool ValidateASTInputFilesContent, bool UseGlobalIndex, std::unique_ptr<llvm::Timer> ReadTimer) : Listener(bool(DisableValidationKind & DisableValidationForModuleKind::PCH) @@ -10989,6 +10990,7 @@ ASTReader::ASTReader(Preprocessor &PP, ModuleCache &ModCache, AllowASTWithCompilerErrors(AllowASTWithCompilerErrors), AllowConfigurationMismatch(AllowConfigurationMismatch), ValidateSystemInputs(ValidateSystemInputs), + ForceValidateUserInputs(ForceValidateUserInputs), ValidateASTInputFilesContent(ValidateASTInputFilesContent), UseGlobalIndex(UseGlobalIndex), CurrSwitchCaseStmts(&SwitchCaseStmts) { SourceMgr.setExternalSLocEntrySource(this); diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 5c9cf3e416ca5..21eea72b198b3 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -530,6 +530,7 @@ class DependencyScanningAction : public tooling::ToolAction { ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true; ScanInstance.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings = true; + ScanInstance.getHeaderSearchOpts().ModulesForceValidateUserHeaders = false; // Avoid some checks and module map parsing when loading PCM files. ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false; diff --git a/clang/test/Modules/fmodules-validate-once-per-build-session.c b/clang/test/Modules/fmodules-validate-once-per-build-session.c index c3e67834802b6..f873755d4cc9b 100644 --- a/clang/test/Modules/fmodules-validate-once-per-build-session.c +++ b/clang/test/Modules/fmodules-validate-once-per-build-session.c @@ -19,32 +19,44 @@ // Compile the module. // RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s // RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s +// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s // RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp // RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp // RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp // RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp +// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp +// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp // RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-before.pcm // RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-before.pcm // RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-before-user.pcm // RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-before-user.pcm +// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-before-user-no-force.pcm +// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-before-user-no-force.pcm // === // Use it, and make sure that we did not recompile it. // RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s // RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s +// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-use-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s // RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp // RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp // RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp // RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp +// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp +// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp // RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm // RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm // RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm // RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm +// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm +// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm // RUN: diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm // RUN: diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm // RUN: diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm // RUN: diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm +// RUN: diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm +// RUN: diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm // === // Change the sources. @@ -53,31 +65,54 @@ // === // Use the module, and make sure that we did not recompile it if foo.h or -// module.modulemap are system files, even though the sources changed. +// module.modulemap are system files or user files with force validation disabled, +// even though the sources changed. // RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s // RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s +// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s // RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp // RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp // RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp // RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp +// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp +// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp // RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm // RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm // RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm // RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm +// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm +// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm // RUN: diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm // RUN: diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm -// When foo.h is a user header, we will always validate it. +// When foo.h is an user header, we will validate it by default. // RUN: not diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm // RUN: not diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm +// When foo.h is an user header, we will not validate it if force validation is turned off. +// RUN: diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm +// RUN: diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm // === // Recompile the module if the today's date is before 01 January 2100. // RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s +// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s +// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s // RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp // RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp +// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp +// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp +// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp +// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp // RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm // RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm +// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm +// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm +// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm +// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm // RUN: not diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm // RUN: not diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm +// RUN: not diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm +// RUN: not diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm +// RUN: not diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm +// RUN: not diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm `````````` </details> https://github.com/llvm/llvm-project/pull/139091 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
