[clang] 76cb4cd - [clang] Fix serialized diagnostics edge-cases

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2022-01-26T11:21:51+01:00
New Revision: 76cb4cd074a6816f3801fd4a2bd8854597748239

URL: https://github.com/llvm/llvm-project/commit/76cb4cd074a6816f3801fd4a2bd8854597748239
DIFF: https://github.com/llvm/llvm-project/commit/76cb4cd074a6816f3801fd4a2bd8854597748239.diff

LOG: [clang] Fix serialized diagnostics edge-cases

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.

Reviewed By: Bigcheese

Differential Revision: https://reviews.llvm.org/D118150

Added: 
    clang/test/Misc/serialized-diags-emit-header-module-misconfig.c
    clang/test/Misc/serialized-diags-unknown-argument.c
    clang/test/Misc/serialized-diags-unknown-target.c

Modified: 
    clang/lib/Frontend/CompilerInstance.cpp
    clang/tools/driver/cc1_main.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 4ef5094931329..2465a7e2453be 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -37,6 +37,7 @@
 #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"
@@ -996,6 +997,11 @@ bool CompilerInstance::ExecuteAction(FrontendAction &Act) {
   // 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))
@@ -1034,9 +1040,6 @@ bool CompilerInstance::ExecuteAction(FrontendAction &Act) {
     }
   }
 
-  // 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.

diff  --git a/clang/test/Misc/serialized-diags-emit-header-module-misconfig.c b/clang/test/Misc/serialized-diags-emit-header-module-misconfig.c
new file mode 100644
index 0000000000000..8629f293b18b9
--- /dev/null
+++ b/clang/test/Misc/serialized-diags-emit-header-module-misconfig.c
@@ -0,0 +1,4 @@
+// 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'

diff  --git a/clang/test/Misc/serialized-diags-unknown-argument.c b/clang/test/Misc/serialized-diags-unknown-argument.c
new file mode 100644
index 0000000000000..7d788e11a51b1
--- /dev/null
+++ b/clang/test/Misc/serialized-diags-unknown-argument.c
@@ -0,0 +1,4 @@
+// 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'

diff  --git a/clang/test/Misc/serialized-diags-unknown-target.c b/clang/test/Misc/serialized-diags-unknown-target.c
new file mode 100644
index 0000000000000..040dfa4b2849e
--- /dev/null
+++ b/clang/test/Misc/serialized-diags-unknown-target.c
@@ -0,0 +1,4 @@
+// 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

diff  --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp
index fd3b25ccb3cb1..f648adeba4834 100644
--- a/clang/tools/driver/cc1_main.cpp
+++ b/clang/tools/driver/cc1_main.cpp
@@ -237,8 +237,10 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
                                   static_cast<void*>(&Clang->getDiagnostics()));
 
   DiagsBuffer->FlushDiagnostics(Clang->getDiagnostics());
-  if (!Success)
+  if (!Success) {
+    Clang->getDiagnosticClient().finish();
     return 1;
+  }
 
   // Execute the frontend actions.
   {