Driver: Include driver diagnostics when we --serialize-diagnostics

Currently, when --serialize-diagnostics is passed this only includes
the diagnostics from clang -cc1, and driver diagnostics are
dropped. This causes issues for tools that use the serialized
diagnostics, since stderr is lost and these diagnostics aren't seen at
all.

We handle this by merging the diagnostics from the CC1 process and the
driver diagnostics into a single file when the driver invokes CC1.

Fixes rdar://problem/10585062

llvm-svn: 220525

clang/include/clang/Basic/DiagnosticFrontendKinds.td

@@ -95,9 +95,12 @@ def err_fe_no_pch_in_dir : Error<
 def err_fe_action_not_available : Error<
     "action %0 not compiled in">;
 
+def warn_fe_serialized_diag_merge_failure : Warning<
+    "unable to merge a subprocess's serialized diagnostics">,
+    InGroup<SerializedDiagnostics>;
 def warn_fe_serialized_diag_failure : Warning<
     "unable to open file %0 for serializing diagnostics (%1)">,
-    InGroup<DiagGroup<"serialized-diagnostics">>;
+    InGroup<SerializedDiagnostics>;
 
 def err_verify_missing_line : Error<
     "missing or invalid line number following '@' in expected %0">;

clang/include/clang/Basic/DiagnosticGroups.td

@@ -731,6 +731,9 @@ def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">;
 // AddressSanitizer frontent instrumentation remarks.
 def SanitizeAddressRemarks : DiagGroup<"sanitize-address">;
 
+// Issues with serialized diagnostics.
+def SerializedDiagnostics : DiagGroup<"serialized-diagnostics">;
+
 // A warning group for warnings about code that clang accepts when
 // compiling CUDA C/C++ but which is not compatible with the CUDA spec.
 def CudaCompat : DiagGroup<"cuda-compat">;

clang/include/clang/Frontend/SerializedDiagnosticPrinter.h

@@ -33,8 +33,9 @@ namespace serialized_diags {
 /// This allows wrapper tools for Clang to get diagnostics from Clang
 /// (via libclang) without needing to parse Clang's command line output.
 ///
-std::unique_ptr<DiagnosticConsumer> create(std::unique_ptr<raw_ostream> OS,
-                                           DiagnosticOptions *diags);
+std::unique_ptr<DiagnosticConsumer> create(StringRef OutputFile,
+                                           DiagnosticOptions *Diags,
+                                           bool MergeChildRecords = false);
 
 } // end serialized_diags namespace
 } // end clang namespace

clang/lib/Frontend/CompilerInstance.cpp

@@ -167,18 +167,8 @@ static void SetUpDiagnosticLog(DiagnosticOptions *DiagOpts,
 static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts,
                                        DiagnosticsEngine &Diags,
                                        StringRef OutputFile) {
-  std::error_code EC;
-  auto OS = llvm::make_unique<llvm::raw_fd_ostream>(OutputFile.str(), EC,
-                                                    llvm::sys::fs::F_None);
-
-  if (EC) {
-    Diags.Report(diag::warn_fe_serialized_diag_failure) << OutputFile
-                                                        << EC.message();
-    return;
-  }
-
   auto SerializedConsumer =
-      clang::serialized_diags::create(std::move(OS), DiagOpts);
+      clang::serialized_diags::create(OutputFile, DiagOpts);
 
   assert(Diags.ownsClient());
   Diags.setClient(new ChainedDiagnosticConsumer(

clang/lib/Frontend/CompilerInvocation.cpp

@@ -612,8 +612,9 @@ bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
   bool Success = true;
 
   Opts.DiagnosticLogFile = Args.getLastArgValue(OPT_diagnostic_log_file);
-  Opts.DiagnosticSerializationFile =
-    Args.getLastArgValue(OPT_diagnostic_serialized_file);
+  if (Arg *A =
+          Args.getLastArg(OPT_diagnostic_serialized_file, OPT__serialize_diags))
+    Opts.DiagnosticSerializationFile = A->getValue();
   Opts.IgnoreWarnings = Args.hasArg(OPT_w);
   Opts.NoRewriteMacros = Args.hasArg(OPT_Wno_rewrite_macros);
   Opts.Pedantic = Args.hasArg(OPT_pedantic);

clang/lib/Frontend/SerializedDiagnosticPrinter.cpp

@@ -8,6 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Frontend/SerializedDiagnosticPrinter.h"
+#include "clang/Frontend/SerializedDiagnosticReader.h"
 #include "clang/Frontend/SerializedDiagnostics.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
@@ -15,6 +16,8 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Frontend/DiagnosticRenderer.h"
+#include "clang/Frontend/FrontendDiagnostic.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallString.h"
@@ -87,19 +90,70 @@ protected:
   void endDiagnostic(DiagOrStoredDiag D,
                      DiagnosticsEngine::Level Level) override;
 };
-  
+
+typedef llvm::DenseMap<unsigned, unsigned> AbbrevLookup;
+
+class SDiagsMerger : SerializedDiagnosticReader {
+  SDiagsWriter &Writer;
+  AbbrevLookup FileLookup;
+  AbbrevLookup CategoryLookup;
+  AbbrevLookup DiagFlagLookup;
+
+public:
+  SDiagsMerger(SDiagsWriter &Writer)
+      : SerializedDiagnosticReader(), Writer(Writer) {}
+
+  std::error_code mergeRecordsFromFile(const char *File) {
+    return readDiagnostics(File);
+  }
+
+protected:
+  std::error_code visitStartOfDiagnostic() override;
+  std::error_code visitEndOfDiagnostic() override;
+  std::error_code visitCategoryRecord(unsigned ID, StringRef Name) override;
+  std::error_code visitDiagFlagRecord(unsigned ID, StringRef Name) override;
+  std::error_code visitDiagnosticRecord(
+      unsigned Severity, const serialized_diags::Location &Location,
+      unsigned Category, unsigned Flag, StringRef Message) override;
+  std::error_code visitFilenameRecord(unsigned ID, unsigned Size,
+                                      unsigned Timestamp,
+                                      StringRef Name) override;
+  std::error_code visitFixitRecord(const serialized_diags::Location &Start,
+                                   const serialized_diags::Location &End,
+                                   StringRef CodeToInsert) override;
+  std::error_code
+  visitSourceRangeRecord(const serialized_diags::Location &Start,
+                         const serialized_diags::Location &End) override;
+
+private:
+  std::error_code adjustSourceLocFilename(RecordData &Record,
+                                          unsigned int offset);
+
+  void adjustAbbrevID(RecordData &Record, AbbrevLookup &Lookup,
+                      unsigned NewAbbrev);
+
+  void writeRecordWithAbbrev(unsigned ID, RecordData &Record);
+
+  void writeRecordWithBlob(unsigned ID, RecordData &Record, StringRef Blob);
+};
+
 class SDiagsWriter : public DiagnosticConsumer {
   friend class SDiagsRenderer;
+  friend class SDiagsMerger;
 
   struct SharedState;
 
   explicit SDiagsWriter(IntrusiveRefCntPtr<SharedState> State)
-    : LangOpts(nullptr), OriginalInstance(false), State(State) {}
+      : LangOpts(nullptr), OriginalInstance(false), MergeChildRecords(false),
+        State(State) {}
 
 public:
-  SDiagsWriter(std::unique_ptr<raw_ostream> os, DiagnosticOptions *diags)
+  SDiagsWriter(StringRef File, DiagnosticOptions *Diags, bool MergeChildRecords)
       : LangOpts(nullptr), OriginalInstance(true),
-        State(new SharedState(std::move(os), diags)) {
+        MergeChildRecords(MergeChildRecords),
+        State(new SharedState(File, Diags)) {
+    if (MergeChildRecords)
+      RemoveOldDiagnostics();
     EmitPreamble();
   }
 
@@ -115,6 +169,14 @@ public:
   void finish() override;
 
 private:
+  /// \brief Build a DiagnosticsEngine to emit diagnostics about the diagnostics
+  DiagnosticsEngine *getMetaDiags();
+
+  /// \brief Remove old copies of the serialized diagnostics. This is necessary
+  /// so that we can detect when subprocesses write diagnostics that we should
+  /// merge into our own.
+  void RemoveOldDiagnostics();
+
   /// \brief Emit the preamble for the serialized diagnostics.
   void EmitPreamble();
   
@@ -152,7 +214,9 @@ private:
   /// \brief Emit the string information for diagnostic flags.
   unsigned getEmitDiagnosticFlag(DiagnosticsEngine::Level DiagLevel,
                                  unsigned DiagID = 0);
-  
+
+  unsigned getEmitDiagnosticFlag(StringRef DiagName);
+
   /// \brief Emit (lazily) the file string and retrieved the file identifier.
   unsigned getEmitFile(const char *Filename);
 
@@ -181,11 +245,15 @@ private:
   /// clones), responsible for writing the file at the end.
   bool OriginalInstance;
 
+  /// \brief Whether this instance should aggregate diagnostics that are
+  /// generated from child processes.
+  bool MergeChildRecords;
+
   /// \brief State that is shared among the various clones of this diagnostic
   /// consumer.
   struct SharedState : RefCountedBase<SharedState> {
-    SharedState(std::unique_ptr<raw_ostream> os, DiagnosticOptions *diags)
-        : DiagOpts(diags), Stream(Buffer), OS(std::move(os)),
+    SharedState(StringRef File, DiagnosticOptions *Diags)
+        : DiagOpts(Diags), Stream(Buffer), OutputFile(File.str()),
           EmittedAnyDiagBlocks(false) {}
 
     /// \brief Diagnostic options.
@@ -198,7 +266,7 @@ private:
     llvm::BitstreamWriter Stream;
 
     /// \brief The name of the diagnostics file.
-    std::unique_ptr<raw_ostream> OS;
+    std::string OutputFile;
 
     /// \brief The set of constructed record abbreviations.
     AbbreviationMap Abbrevs;
@@ -225,6 +293,9 @@ private:
     /// this becomes \c true, we never close a DIAG block until we know that we're
     /// starting another one or we're done.
     bool EmittedAnyDiagBlocks;
+
+    /// \brief Engine for emitting diagnostics about the diagnostics.
+    std::unique_ptr<DiagnosticsEngine> MetaDiagnostics;
   };
 
   /// \brief State shared among the various clones of this diagnostic consumer.
@@ -234,10 +305,11 @@ private:
 
 namespace clang {
 namespace serialized_diags {
-std::unique_ptr<DiagnosticConsumer> create(std::unique_ptr<raw_ostream> OS,
-                                           DiagnosticOptions *diags) {
-  return llvm::make_unique<SDiagsWriter>(std::move(OS), diags);
+std::unique_ptr<DiagnosticConsumer>
+create(StringRef OutputFile, DiagnosticOptions *Diags, bool MergeChildRecords) {
+  return llvm::make_unique<SDiagsWriter>(OutputFile, Diags, MergeChildRecords);
 }
+
 } // end namespace serialized_diags
 } // end namespace clang
 
@@ -492,6 +564,10 @@ unsigned SDiagsWriter::getEmitDiagnosticFlag(DiagnosticsEngine::Level DiagLevel,
     return 0; // No flag for notes.
   
   StringRef FlagName = DiagnosticIDs::getWarningOptionForDiag(DiagID);
+  return getEmitDiagnosticFlag(FlagName);
+}
+
+unsigned SDiagsWriter::getEmitDiagnosticFlag(StringRef FlagName) {
   if (FlagName.empty())
     return 0;
 
@@ -686,6 +762,40 @@ void SDiagsRenderer::emitNote(SourceLocation Loc, StringRef Message,
   Writer.ExitDiagBlock();
 }
 
+DiagnosticsEngine *SDiagsWriter::getMetaDiags() {
+  // FIXME: It's slightly absurd to create a new diagnostics engine here, but
+  // the other options that are available today are worse:
+  //
+  // 1. Teach DiagnosticsConsumers to emit diagnostics to the engine they are a
+  //    part of. The DiagnosticsEngine would need to know not to send
+  //    diagnostics back to the consumer that failed. This would require us to
+  //    rework ChainedDiagnosticsConsumer and teach the engine about multiple
+  //    consumers, which is difficult today because most APIs interface with
+  //    consumers rather than the engine itself.
+  //
+  // 2. Pass a DiagnosticsEngine to SDiagsWriter on creation - this would need
+  //    to be distinct from the engine the writer was being added to and would
+  //    normally not be used.
+  if (!State->MetaDiagnostics) {
+    IntrusiveRefCntPtr<DiagnosticIDs> IDs(new DiagnosticIDs());
+    auto Client =
+        new TextDiagnosticPrinter(llvm::errs(), State->DiagOpts.get());
+    State->MetaDiagnostics = llvm::make_unique<DiagnosticsEngine>(
+        IDs, State->DiagOpts.get(), Client);
+  }
+  return State->MetaDiagnostics.get();
+}
+
+void SDiagsWriter::RemoveOldDiagnostics() {
+  if (!llvm::sys::fs::remove(State->OutputFile))
+    return;
+
+  getMetaDiags()->Report(diag::warn_fe_serialized_diag_merge_failure);
+  // Disable merging child records, as whatever is in this file may be
+  // misleading.
+  MergeChildRecords = false;
+}
+
 void SDiagsWriter::finish() {
   // The original instance is responsible for writing the file.
   if (!OriginalInstance)
@@ -695,9 +805,113 @@ void SDiagsWriter::finish() {
   if (State->EmittedAnyDiagBlocks)
     ExitDiagBlock();
 
-  // Write the generated bitstream to "Out".
-  State->OS->write((char *)&State->Buffer.front(), State->Buffer.size());
-  State->OS->flush();
+  if (MergeChildRecords) {
+    if (!State->EmittedAnyDiagBlocks)
+      // We have no diagnostics of our own, so we can just leave the child
+      // process' output alone
+      return;
 
-  State->OS.reset();
+    if (llvm::sys::fs::exists(State->OutputFile))
+      if (SDiagsMerger(*this).mergeRecordsFromFile(State->OutputFile.c_str()))
+        getMetaDiags()->Report(diag::warn_fe_serialized_diag_merge_failure);
+  }
+
+  std::error_code EC;
+  auto OS = llvm::make_unique<llvm::raw_fd_ostream>(State->OutputFile.c_str(),
+                                                    EC, llvm::sys::fs::F_None);
+  if (EC) {
+    getMetaDiags()->Report(diag::warn_fe_serialized_diag_failure)
+        << State->OutputFile << EC.message();
+    return;
+  }
+
+  // Write the generated bitstream to "Out".
+  OS->write((char *)&State->Buffer.front(), State->Buffer.size());
+  OS->flush();
+}
+
+std::error_code SDiagsMerger::visitStartOfDiagnostic() {
+  Writer.EnterDiagBlock();
+  return std::error_code();
+}
+
+std::error_code SDiagsMerger::visitEndOfDiagnostic() {
+  Writer.ExitDiagBlock();
+  return std::error_code();
+}
+
+std::error_code
+SDiagsMerger::visitSourceRangeRecord(const serialized_diags::Location &Start,
+                                     const serialized_diags::Location &End) {
+  RecordData Record;
+  Record.push_back(RECORD_SOURCE_RANGE);
+  Record.push_back(FileLookup[Start.FileID]);
+  Record.push_back(Start.Line);
+  Record.push_back(Start.Col);
+  Record.push_back(Start.Offset);
+  Record.push_back(FileLookup[End.FileID]);
+  Record.push_back(End.Line);
+  Record.push_back(End.Col);
+  Record.push_back(End.Offset);
+
+  Writer.State->Stream.EmitRecordWithAbbrev(
+      Writer.State->Abbrevs.get(RECORD_SOURCE_RANGE), Record);
+  return std::error_code();
+}
+
+std::error_code SDiagsMerger::visitDiagnosticRecord(
+    unsigned Severity, const serialized_diags::Location &Location,
+    unsigned Category, unsigned Flag, StringRef Message) {
+  RecordData MergedRecord;
+  MergedRecord.push_back(RECORD_DIAG);
+  MergedRecord.push_back(Severity);
+  MergedRecord.push_back(FileLookup[Location.FileID]);
+  MergedRecord.push_back(Location.Line);
+  MergedRecord.push_back(Location.Col);
+  MergedRecord.push_back(Location.Offset);
+  MergedRecord.push_back(CategoryLookup[Category]);
+  MergedRecord.push_back(Flag ? DiagFlagLookup[Flag] : 0);
+  MergedRecord.push_back(Message.size());
+
+  Writer.State->Stream.EmitRecordWithBlob(
+      Writer.State->Abbrevs.get(RECORD_DIAG), MergedRecord, Message);
+  return std::error_code();
+}
+
+std::error_code
+SDiagsMerger::visitFixitRecord(const serialized_diags::Location &Start,
+                               const serialized_diags::Location &End,
+                               StringRef Text) {
+  RecordData Record;
+  Record.push_back(RECORD_FIXIT);
+  Record.push_back(FileLookup[Start.FileID]);
+  Record.push_back(Start.Line);
+  Record.push_back(Start.Col);
+  Record.push_back(Start.Offset);
+  Record.push_back(FileLookup[End.FileID]);
+  Record.push_back(End.Line);
+  Record.push_back(End.Col);
+  Record.push_back(End.Offset);
+  Record.push_back(Text.size());
+
+  Writer.State->Stream.EmitRecordWithBlob(
+      Writer.State->Abbrevs.get(RECORD_FIXIT), Record, Text);
+  return std::error_code();
+}
+
+std::error_code SDiagsMerger::visitFilenameRecord(unsigned ID, unsigned Size,
+                                                  unsigned Timestamp,
+                                                  StringRef Name) {
+  FileLookup[ID] = Writer.getEmitFile(Name.str().c_str());
+  return std::error_code();
+}
+
+std::error_code SDiagsMerger::visitCategoryRecord(unsigned ID, StringRef Name) {
+  CategoryLookup[ID] = Writer.getEmitCategory(ID);
+  return std::error_code();
+}
+
+std::error_code SDiagsMerger::visitDiagFlagRecord(unsigned ID, StringRef Name) {
+  DiagFlagLookup[ID] = Writer.getEmitDiagnosticFlag(Name);
+  return std::error_code();
 }

clang/test/Misc/serialized-diags-driver.c

@@ -0,0 +1,20 @@
+// Test that the driver correctly combines its own diagnostics with CC1's in the
+// serialized diagnostics. To test this, we need to trigger diagnostics from
+// both processes, so we compile code that has a warning (with an associated
+// note) and then force the driver to crash. We compile stdin so that the crash
+// doesn't litter the user's system with preprocessed output.
+
+// RUN: rm -f %t
+// RUN: %clang -Wx-unknown-warning -Wall -fsyntax-only --serialize-diagnostics %t.diag %s
+// RUN: c-index-test -read-diagnostics %t.diag 2>&1 | FileCheck %s
+
+// CHECK: warning: unknown warning option '-Wx-unknown-warning' [-Wunknown-warning-option] []
+
+// CHECK: warning: variable 'voodoo' is uninitialized when used here [-Wuninitialized]
+// CHECK: note: initialize the variable 'voodoo' to silence this warning []
+// CHECK: Number of diagnostics: 2
+
+void foo() {
+  int voodoo;
+  voodoo = voodoo + 1;
+}

clang/tools/driver/driver.cpp

@@ -19,6 +19,8 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/ChainedDiagnosticConsumer.h"
+#include "clang/Frontend/SerializedDiagnosticPrinter.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Frontend/Utils.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -445,6 +447,16 @@ int main(int argc_, const char **argv_) {
   IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
 
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagClient);
+
+  if (!DiagOpts->DiagnosticSerializationFile.empty()) {
+    auto SerializedConsumer =
+        clang::serialized_diags::create(DiagOpts->DiagnosticSerializationFile,
+                                        &*DiagOpts, /*MergeChildRecords=*/true);
+    Diags.setClient(new ChainedDiagnosticConsumer(
+        std::unique_ptr<DiagnosticConsumer>(Diags.takeClient()),
+        std::move(SerializedConsumer)));
+  }
+
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
@@ -492,10 +504,12 @@ int main(int argc_, const char **argv_) {
     }
   }
 
+  Diags.getClient()->finish();
+
   // If any timers were active but haven't been destroyed yet, print their
   // results now.  This happens in -disable-free mode.
   llvm::TimerGroup::printAll(llvm::errs());
-  
+
   llvm::llvm_shutdown();
 
 #ifdef LLVM_ON_WIN32