Fix OOM in FormatDiagnostic (2nd attempt) (#108866)

Resolves: #70930 (and probably latest comments from clangd/clangd#251)
by fixing racing for the shared DiagStorage value which caused messing with args inside the storage and then formatting the following message with getArgSInt(1) == 2:

def err_module_odr_violation_function : Error<
  "%q0 has different definitions in different modules; "
  "%select{definition in module '%2'|defined here}1 "
  "first difference is "

which causes HandleSelectModifier to go beyond the ArgumentLen so the recursive call to FormatDiagnostic was made with DiagStr > DiagEnd that leads to infinite while (DiagStr != DiagEnd).

The Main Idea:
Reuse the existing DiagStorageAllocator logic to make all DiagnosticBuilders having independent states.
Also, encapsulating the rest of state (e.g. ID and Loc) into DiagnosticBuilder.

The last attempt failed -
https://github.com/llvm/llvm-project/pull/108187#issuecomment-2353122096
so was reverted - #108838

1 files changed, 15 insertions, 4 deletions

diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp
index 7469019..691d74f 100644
--- a/clang/unittests/Basic/DiagnosticTest.cpp
+++ b/clang/unittests/Basic/DiagnosticTest.cpp
@@ -17,9 +17,6 @@ using namespace llvm;
 using namespace clang;
 
 void clang::DiagnosticsTestHelper(DiagnosticsEngine &diag) {
-  unsigned delayedDiagID = 0U;
-
-  EXPECT_EQ(diag.DelayedDiagID, delayedDiagID);
   EXPECT_FALSE(diag.DiagStates.empty());
   EXPECT_TRUE(diag.DiagStatesByLoc.empty());
   EXPECT_TRUE(diag.DiagStateOnPushStack.empty());
@@ -83,6 +80,21 @@ TEST(DiagnosticTest, fatalsAsError) {
   }
 }
 
+TEST(DiagnosticTest, tooManyErrorsIsAlwaysFatal) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
+                          new IgnoringDiagConsumer());
+  Diags.setFatalsAsError(true);
+
+  // Report a fatal_too_many_errors diagnostic to ensure that still
+  // acts as a fatal error despite downgrading fatal errors to errors.
+  Diags.Report(diag::fatal_too_many_errors);
+  EXPECT_TRUE(Diags.hasFatalErrorOccurred());
+
+  // Ensure that the severity of that diagnostic is really "fatal".
+  EXPECT_EQ(Diags.getDiagnosticLevel(diag::fatal_too_many_errors, {}),
+            DiagnosticsEngine::Level::Fatal);
+}
+
 // Check that soft RESET works as intended
 TEST(DiagnosticTest, softReset) {
   DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
@@ -104,7 +116,6 @@ TEST(DiagnosticTest, softReset) {
   // Check for private variables of DiagnosticsEngine differentiating soft reset
   DiagnosticsTestHelper(Diags);
 
-  EXPECT_FALSE(Diags.isDiagnosticInFlight());
   EXPECT_TRUE(Diags.isLastDiagnosticIgnored());
 }