[clang][cas] Fix cache poison error with diagnostic options

Avoid emitting DIAGNOSTIC_OPTIONS block when caching. In cached builds,
there is no benefit to emitting diagnostic options into the PCM/PCH,
since we know it can never mismatch. Remove this block, because it can
contain volatile options such as fmessage-length that should not have
any impact on the output when using cached diagnostics.

Also add a test for -ferror-limit: while it is okay to clear this option
it's important to test it since it relies on the fact we are not caching
compilation failures. Unlike the other "formatting" options, this
affects the set of diagnostics passed to the diagnostic consumer.

rdar://109285136
(cherry picked from commit e939ada)

Author: benlangmuir

clang/include/clang/Driver/Options.td

@@ -6650,6 +6650,11 @@ defm cache_disable_replay : BoolFOption<"cache-disable-replay",
     PosFlag<SetTrue, [], "Disable replaying a cached compile job">,
     NegFlag<SetFalse>>;
 
+defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options",
+    HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse,
+    PosFlag<SetTrue, [], "Disable writing diagnostic options">,
+    NegFlag<SetFalse>>;
+
 def fcompilation_caching_service_path : Separate<["-"], "fcompilation-caching-service-path">,
     Group<f_Group>, MetaVarName<"<pathname>">,
     HelpText<"Specify the socket path for connecting to a remote caching service">,

clang/include/clang/Lex/HeaderSearchOptions.h

@@ -216,6 +216,9 @@ class HeaderSearchOptions {
 
   unsigned ModulesValidateDiagnosticOptions : 1;
 
+  /// Whether to entirely skip writing diagnostic options.
+  unsigned ModulesSkipDiagnosticOptions : 1;
+
   unsigned ModulesHashContent : 1;
 
   /// Whether we should include all things that could impact the module in the
@@ -234,7 +237,8 @@ class HeaderSearchOptions {
         ModulesValidateOncePerBuildSession(false),
         ModulesValidateSystemHeaders(false),
         ValidateASTInputFilesContent(false), UseDebugInfo(false),
-        ModulesValidateDiagnosticOptions(true), ModulesHashContent(false),
+        ModulesValidateDiagnosticOptions(true),
+        ModulesSkipDiagnosticOptions(false), ModulesHashContent(false),
         ModulesStrictContextHash(false) {}
 
   /// AddPath - Add the \p Path path to the specified \p Group list.

clang/lib/Frontend/CompileJobCacheKey.cpp

@@ -94,6 +94,8 @@ createCompileJobCacheKeyImpl(ObjectStore &CAS, DiagnosticsEngine &Diags,
   DiagOpts.UseANSIEscapeCodes = false;
   DiagOpts.VerifyDiagnostics = false;
   DiagOpts.setVerifyIgnoreUnexpected(DiagnosticLevelMask::None);
+  // Note: ErrorLimit affects the set of diagnostics emitted, but since we do
+  // not cache compilation failures, it's safe to clear here.
   DiagOpts.ErrorLimit = 0;
   DiagOpts.MacroBacktraceLimit = 0;
   DiagOpts.SnippetLineLimit = 0;

clang/lib/Serialization/ASTWriter.cpp

@@ -1161,6 +1161,9 @@ ASTFileSignature ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
   // Diagnostic options.
   const auto &Diags = Context.getDiagnostics();
   const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions();
+  if (!PP.getHeaderSearchInfo()
+           .getHeaderSearchOpts()
+           .ModulesSkipDiagnosticOptions) {
 #define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name);
 #define ENUM_DIAGOPT(Name, Type, Bits, Default)                                \
   Record.push_back(static_cast<unsigned>(DiagOpts.get##Name()));
@@ -1175,6 +1178,7 @@ ASTFileSignature ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
   // are generally transient files and will almost always be overridden.
   Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
   Record.clear();
+  }
 
   // Header search paths.
   Record.clear();

clang/lib/Tooling/DependencyScanning/ScanAndUpdateArgs.cpp

@@ -36,15 +36,20 @@ void tooling::dependencies::configureInvocationForCaching(
     CI.getCodeGenOpts().CoverageNotesFile.clear();
   }
 
+  HeaderSearchOptions &HSOpts = CI.getHeaderSearchOpts();
+  // Avoid writing potentially volatile diagnostic options into pcms.
+  HSOpts.ModulesSkipDiagnosticOptions = true;
+
   // "Fix" the CAS options.
   auto &FileSystemOpts = CI.getFileSystemOpts();
   if (ProduceIncludeTree) {
     FrontendOpts.CASIncludeTreeID = std::move(RootID);
     FrontendOpts.Inputs.clear();
     FrontendOpts.ModuleMapFiles.clear();
-    HeaderSearchOptions &HSOpts = CI.getHeaderSearchOpts();
     HeaderSearchOptions OriginalHSOpts;
     std::swap(HSOpts, OriginalHSOpts);
+    HSOpts.ModulesSkipDiagnosticOptions =
+        OriginalHSOpts.ModulesSkipDiagnosticOptions;
     // Preserve sysroot path to accommodate lookup for 'SDKSettings.json' during
     // availability checking.
     HSOpts.Sysroot = std::move(OriginalHSOpts.Sysroot);

clang/test/CAS/cached-diagnostics.c

@@ -54,10 +54,33 @@
 // RUN: %clang @%t/t2.rsp 2> %t/cached-diags2.txt
 // RUN: diff -u %t/regular-diags2.txt %t/cached-diags2.txt
 
+// RUN: %clang -cc1depscan -fdepscan=inline -fdepscan-include-tree -o %t/terr.rsp -cc1-args \
+// RUN:   -cc1 -triple x86_64-apple-macos12 -fcas-path %t/cas -fdepscan-prefix-map=%t/src=/^src \
+// RUN:     -emit-obj %t/src/main.c -o %t/out/output.o -I %t/src/inc -Rcompile-job-cache -DERROR
+
+// RUN: not %clang @%t/terr.rsp -ferror-limit 1 2> %t/diags_error1.txt
+// RUN: FileCheck %s -check-prefix=ERROR1 -input-file %t/diags_error1.txt
+
+// ERROR1: error: E1
+// ERROR1-NOT: error:
+// ERROR1: fatal error: too many errors emitted
+
+// RUN: not %clang @%t/terr.rsp -ferror-limit 2 2> %t/diags_error2.txt
+// RUN: FileCheck %s -check-prefix=ERROR2 -input-file %t/diags_error2.txt
+
+// ERROR2: error: E1
+// ERROR2: error: E2
+// ERROR2-NOT: error:
+
 //--- main.c
 
 #include "t1.h"
 
+#ifdef ERROR
+#error E1
+#error E2
+#endif
+
 //--- inc/t1.h
 
 #warning some warning

clang/test/ClangScanDeps/modules-include-tree-diag-opts.c

@@ -0,0 +1,28 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json \
+// RUN:   -format experimental-include-tree-full -cas-path %t/cas > %t/deps.json
+
+// RUN: %deps-to-rsp %t/deps.json --module-name Mod > %t/Mod.rsp
+
+// RUN: %clang @%t/Mod.rsp
+// RUN: %clang @%t/Mod.rsp -fmessage-length=8
+// RUN: %clang @%t/Mod.rsp -fcolor-diagnostics
+// RUN: %clang @%t/Mod.rsp -ferror-limit 1
+
+//--- cdb.json.template
+[{
+  "file": "DIR/tu.c",
+  "directory": "DIR",
+  "command": "clang -Xclang -fcache-disable-replay -fsyntax-only DIR/tu.c -fmodules -fimplicit-modules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache"
+}]
+
+//--- module.modulemap
+module Mod { header "Mod.h" }
+
+//--- Mod.h
+
+//--- tu.c
+#include "Mod.h"