[clang] Fix serialized diagnostics edge-cases

[jansvoboda11]

Summary

The Clang frontend sometimes fails on the following assertion when launched with -serialize-diagnostic-file <x>:

Assertion failed: (BlockScope.empty() && CurAbbrevs.empty() && "Block imbalance"), function ~BitstreamWriter, file BitstreamWriter.h, line 125.

This was first noticed when passing an unknown command-line argument to -cc1.

It turns out the DiagnosticConsumer::finish() function should be called as soon as processing of all source files ends, but there are some code paths where that doesn't happen:

1. when command line parsing fails in cc1_main(),
2. when !Act.PrepareToExecute(*this) or !createTarget() evaluate to true in CompilerInstance::ExecuteAction and the function returns early.

This patch ensures finish() is called in all those code paths.

[jansvoboda11]

clang/test/Misc/serialized-diags-emit-header-module-misconfig.c

2

This combination of command-line arguments causes GenerateHeaderModuleAction::PrepareToExecuteAction to return false and !Act.PrepareToExecute(*this) == true.

[Bigcheese]

lgtm

clang/lib/Frontend/CompilerInstance.cpp

 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/GlobalModuleIndex.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/LockFileManager.h"
 #include "llvm/Support/MemoryBuffer.h"
bool CompilerInstance::ExecuteAction(FrontendAction &Act) {
   assert(!getFrontendOpts().ShowHelp && "Client must handle '-help'!");
   assert(!getFrontendOpts().ShowVersion && "Client must handle '-version'!");
 
   // Mark this point as the bottom of the stack if we don't have somewhere
   // better. We generally expect frontend actions to be invoked with (nearly)
   // DesiredStackSpace available.
   noteBottomOfStack();
 
+  auto FinishDiagnosticClient = llvm::make_scope_exit([&]() {
+    // Notify the diagnostic client that all files were processed.
+    getDiagnosticClient().finish();
+  });
+
   raw_ostream &OS = getVerboseOutputStream();
 
   if (!Act.PrepareToExecute(*this))
     return false;
 
   if (!createTarget())
     return false;
 
for (const FrontendInputFile &FIF : getFrontendOpts().Inputs) {
     if (Act.BeginSourceFile(*this, FIF)) {
       if (llvm::Error Err = Act.Execute()) {
         consumeError(std::move(Err)); // FIXME this drops errors on the floor.
       }
       Act.EndSourceFile();
     }
   }
 
-  // Notify the diagnostic client that all files were processed.
-  getDiagnostics().getClient()->finish();
-
   if (getDiagnosticOpts().ShowCarets) {
     // We can have multiple diagnostics sharing one diagnostic client.
     // Get the total number of warnings/errors from the client.
     unsigned NumWarnings = getDiagnostics().getClient()->getNumWarnings();
     unsigned NumErrors = getDiagnostics().getClient()->getNumErrors();
 
     if (NumWarnings)
       OS << NumWarnings << " warning" << (NumWarnings == 1 ? "" : "s");

clang/test/Misc/serialized-diags-emit-header-module-misconfig.c

+// RUN: rm -rf %t && mkdir %t
+// RUN: not %clang_cc1 -emit-header-module %s -o %t/out.pcm -serialize-diagnostic-file %t/diag 2>&1 | FileCheck %s
+
+// CHECK: error: header module compilation requires '-fmodules', '-std=c++20', or '-fmodules-ts'

clang/test/Misc/serialized-diags-unknown-argument.c

+// RUN: rm -rf %t && mkdir %t
+// RUN: not %clang_cc1 %s -unknown-argument -serialize-diagnostic-file %t/diag -o /dev/null 2>&1 | FileCheck %s
+
+// CHECK: error: unknown argument: '-unknown-argument'

clang/test/Misc/serialized-diags-unknown-target.c

+// RUN: rm -rf %t && mkdir %t
+// RUN: not %clang_cc1 %s -triple blah-unknown-unknown -serialize-diagnostic-file %t/diag -o /dev/null 2>&1 | FileCheck %s
+
+// CHECK: error: unknown target triple 'blah-unknown-unknown', please use -triple or -arch

clang/tools/driver/cc1_main.cpp

if (!Clang->hasDiagnostics())
     return 1;
 
   // Set an error handler, so that any LLVM backend diagnostics go through our
   // error handler.
   llvm::install_fatal_error_handler(LLVMErrorHandler,
                                   static_cast<void*>(&Clang->getDiagnostics()));
 
   DiagsBuffer->FlushDiagnostics(Clang->getDiagnostics());
-  if (!Success)
+  if (!Success) {
+    Clang->getDiagnosticClient().finish();
     return 1;
+  }
 
   // Execute the frontend actions.
   {
     llvm::TimeTraceScope TimeScope("ExecuteCompiler");
     Success = ExecuteCompilerInvocation(Clang.get());
   }
 
   // If any timers were active but haven't been destroyed yet, print their