[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 12 11:27:08 PDT 2024


================
@@ -571,8 +571,7 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
   }
 
   // If explicitly requested, map fatal errors to errors.
-  if (Result == diag::Severity::Fatal &&
-      Diag.CurDiagID != diag::fatal_too_many_errors && Diag.FatalsAsError)
----------------
AaronBallman wrote:

> Well, for now I would do it in a simple/hopeful way - just use the DiagID argument. But, if it doesn't seem safe to reviewers - I could add smth like size_t count_in_flight__fatal_too_many_errors to the DiagnosticsEngine to mimic the original behavior.

I think it should be reasonably safe to use `DiagID`. We could add some test coverage near https://github.com/llvm/llvm-project/blob/2670565afc4ec855fa406b8f582dde44ce1739fb/clang/unittests/Basic/DiagnosticTest.cpp#L61 as it seems we can only downgrade fatal diagnostics to errors as part of internal APIs rather than user-facing flags. The test could look something like (I've not tried this out myself):
```
TEST(DiagnosticTest, fatalsAsError) {
  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 really is "fatal".
  EXPECT_EQ(Diags.getDiagnosticIDs()->getDiagnosticSeverity(diag::fatal_too_many_errors, {}, Diags), diags::Severity::Fatal);
}
```


https://github.com/llvm/llvm-project/pull/108187


More information about the cfe-commits mailing list