[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox created
https://github.com/llvm/llvm-project/pull/108187
Resolves: #70930 (and probably latest comments from #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
`DiagnosticBuilder`s having independent states.
Also, encapsulating the rest of state (e.g. ID and Loc) into
`DiagnosticBuilder`.
**TODO (or not todo):**
- [ ] add a test (I have no idea how to turn a whole bunch of my proprietary
code which leads `clangd` to OOM into a small public example.. probably I must
try using
[this](https://github.com/llvm/llvm-project/issues/70930#issuecomment-2209872975)
instead)
- [ ] figure `Diag.CurDiagID != diag::fatal_too_many_errors` out
- [ ] ? get rid of `DiagStorageAllocator` at all and make `DiagnosticBuilder`
having they own `DiagnosticStorage` coz it seems pretty small so should fit the
stack for short-living `DiagnosticBuilder` instances
>From f26c4f33c838f173cd330a18b66eb94ffa7fcc87 Mon Sep 17 00:00:00 2001
From: Sergei
Date: Tue, 10 Sep 2024 16:19:05 +
Subject: [PATCH] fix quick OOM in FormatDiagnostic
---
.../ClangTidyDiagnosticConsumer.cpp | 2 -
clang/include/clang/Basic/Diagnostic.h| 269 ++
clang/include/clang/Basic/DiagnosticIDs.h | 7 +-
clang/include/clang/Basic/PartialDiagnostic.h | 5 +-
clang/include/clang/Sema/Sema.h | 6 +-
clang/lib/Basic/Diagnostic.cpp| 86 +++---
clang/lib/Basic/DiagnosticIDs.cpp | 22 +-
clang/lib/Basic/SourceManager.cpp | 23 +-
clang/lib/Frontend/Rewrite/FixItRewriter.cpp | 4 +-
clang/lib/Frontend/TextDiagnosticPrinter.cpp | 2 +-
clang/lib/Sema/Sema.cpp | 19 +-
clang/lib/Sema/SemaBase.cpp | 2 +-
clang/lib/Serialization/ASTReader.cpp | 15 +-
clang/unittests/Basic/DiagnosticTest.cpp | 4 -
clang/unittests/Driver/DXCModeTest.cpp| 5 -
15 files changed, 174 insertions(+), 297 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 200bb87a5ac3cb..4c75b422701148 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -380,7 +380,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
LastErrorWasIgnored = true;
-Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
return;
@@ -457,7 +456,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
- Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
}
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index 0c7836c2ea569c..1eecab4f6e49a2 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -183,6 +183,41 @@ struct DiagnosticStorage {
DiagnosticStorage() = default;
};
+/// An allocator for DiagnosticStorage objects, which uses a small cache to
+/// objects, used to reduce malloc()/free() traffic for partial diagnostics.
+class DiagStorageAllocator {
+ static const unsigned NumCached = 16;
+ DiagnosticStorage Cached[NumCached];
+ DiagnosticStorage *FreeList[NumCached];
+ unsigned NumFreeListEntries;
+
+public:
+ DiagStorageAllocator();
+ ~DiagStorageAllocator();
+
+ /// Allocate new storage.
+ DiagnosticStorage *Allocate() {
+if (NumFreeListEntries == 0)
+ return new DiagnosticStorage;
+
+DiagnosticStorage *Result = FreeList[--NumFreeListEntries];
+Result->NumDiagArgs = 0;
+Result->DiagRanges.clear();
+Result->FixItHints.clear();
+return Result;
+ }
+
+ /// Free the given storage object.
+ void Deallocate(DiagnosticStorage *S) {
+if (S >= Cached && S <= Cached + NumCached) {
+ FreeList[NumFreeListEntries++] = S;
+ return;
+}
+
+delete S;
+ }
+};
+
/// Concrete class used by the front-end to report problems and issues.
///
/// This massages the diagnostics (e.g. handling things like "report warnings
@@ -520,27 +555,6 @@ class DiagnosticsEngine : public
Ref
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -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) igelbox wrote: I'm not sure why do we checked for `Diag.CurDiagID != diag::fatal_too_many_errors` here. Moreover even without this piece of code unit-tests succeed. https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -1333,13 +1263,7 @@ class DiagnosticBuilder : public StreamingDiagnostic {
public:
/// Copy constructor. When copied, this "takes" the diagnostic info from the
/// input and neuters it.
- DiagnosticBuilder(const DiagnosticBuilder &D) : StreamingDiagnostic() {
-DiagObj = D.DiagObj;
-DiagStorage = D.DiagStorage;
igelbox wrote:
Well well, `DiagStorage` might be allocated via `DiagStorageAllocator` so this
looked like a potential memory manager mess. Now, I fixed this (see cpp).
https://github.com/llvm/llvm-project/pull/108187
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -538,24 +511,51 @@ bool DiagnosticsEngine::EmitCurrentDiagnostic(bool Force)
{
Emitted = (DiagLevel != DiagnosticIDs::Ignored);
if (Emitted) {
// Emit the diagnostic regardless of suppression level.
- Diags->EmitDiag(*this, DiagLevel);
+ Diags->EmitDiag(*this, DB, DiagLevel);
}
} else {
// Process the diagnostic, sending the accumulated information to the
// DiagnosticConsumer.
-Emitted = ProcessDiag();
+Emitted = ProcessDiag(DB);
}
- // Clear out the current diagnostic object.
- Clear();
+ return Emitted;
+}
- // If there was a delayed diagnostic, emit it now.
- if (!Force && DelayedDiagID)
-ReportDelayed();
+DiagnosticBuilder::DiagnosticBuilder(DiagnosticsEngine *diagObj,
+ SourceLocation CurDiagLoc,
+ unsigned CurDiagID)
+: StreamingDiagnostic(diagObj->DiagAllocator), DiagObj(diagObj),
+ CurDiagLoc(CurDiagLoc), CurDiagID(CurDiagID), IsActive(true) {
+ assert(diagObj && "DiagnosticBuilder requires a valid DiagnosticsEngine!");
+}
- return Emitted;
+DiagnosticBuilder::DiagnosticBuilder(const DiagnosticBuilder &D)
+: StreamingDiagnostic() {
+ CurDiagLoc = D.CurDiagLoc;
+ CurDiagID = D.CurDiagID;
+ FlagValue = D.FlagValue;
+ DiagObj = D.DiagObj;
+ DiagStorage = D.DiagStorage;
+ D.DiagStorage = nullptr;
igelbox wrote:
Prevent potential memory manager mess if it was allocated by
`DiagStorageAllocator`
https://github.com/llvm/llvm-project/pull/108187
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox updated
https://github.com/llvm/llvm-project/pull/108187
>From aee4cf70dedaa3c8b7b6508238e01f92d60c631c Mon Sep 17 00:00:00 2001
From: Sergei
Date: Tue, 10 Sep 2024 16:19:05 +
Subject: [PATCH] fix quick OOM in FormatDiagnostic
---
.../ClangTidyDiagnosticConsumer.cpp | 2 -
clang/include/clang/Basic/Diagnostic.h| 269 ++
clang/include/clang/Basic/DiagnosticIDs.h | 7 +-
clang/include/clang/Basic/PartialDiagnostic.h | 5 +-
clang/include/clang/Sema/Sema.h | 6 +-
clang/lib/Basic/Diagnostic.cpp| 86 +++---
clang/lib/Basic/DiagnosticIDs.cpp | 22 +-
clang/lib/Basic/SourceManager.cpp | 23 +-
clang/lib/Frontend/Rewrite/FixItRewriter.cpp | 4 +-
clang/lib/Frontend/TextDiagnosticPrinter.cpp | 2 +-
clang/lib/Sema/Sema.cpp | 19 +-
clang/lib/Sema/SemaBase.cpp | 2 +-
clang/lib/Serialization/ASTReader.cpp | 15 +-
clang/unittests/Basic/DiagnosticTest.cpp | 4 -
clang/unittests/Driver/DXCModeTest.cpp| 5 -
15 files changed, 174 insertions(+), 297 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 200bb87a5ac3cb..4c75b422701148 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -380,7 +380,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
LastErrorWasIgnored = true;
-Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
return;
@@ -457,7 +456,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
- Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
}
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index 0c7836c2ea569c..1eecab4f6e49a2 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -183,6 +183,41 @@ struct DiagnosticStorage {
DiagnosticStorage() = default;
};
+/// An allocator for DiagnosticStorage objects, which uses a small cache to
+/// objects, used to reduce malloc()/free() traffic for partial diagnostics.
+class DiagStorageAllocator {
+ static const unsigned NumCached = 16;
+ DiagnosticStorage Cached[NumCached];
+ DiagnosticStorage *FreeList[NumCached];
+ unsigned NumFreeListEntries;
+
+public:
+ DiagStorageAllocator();
+ ~DiagStorageAllocator();
+
+ /// Allocate new storage.
+ DiagnosticStorage *Allocate() {
+if (NumFreeListEntries == 0)
+ return new DiagnosticStorage;
+
+DiagnosticStorage *Result = FreeList[--NumFreeListEntries];
+Result->NumDiagArgs = 0;
+Result->DiagRanges.clear();
+Result->FixItHints.clear();
+return Result;
+ }
+
+ /// Free the given storage object.
+ void Deallocate(DiagnosticStorage *S) {
+if (S >= Cached && S <= Cached + NumCached) {
+ FreeList[NumFreeListEntries++] = S;
+ return;
+}
+
+delete S;
+ }
+};
+
/// Concrete class used by the front-end to report problems and issues.
///
/// This massages the diagnostics (e.g. handling things like "report warnings
@@ -520,27 +555,6 @@ class DiagnosticsEngine : public
RefCountedBase {
void *ArgToStringCookie = nullptr;
ArgToStringFnTy ArgToStringFn;
- /// ID of the "delayed" diagnostic, which is a (typically
- /// fatal) diagnostic that had to be delayed because it was found
- /// while emitting another diagnostic.
- unsigned DelayedDiagID;
-
- /// First string argument for the delayed diagnostic.
- std::string DelayedDiagArg1;
-
- /// Second string argument for the delayed diagnostic.
- std::string DelayedDiagArg2;
-
- /// Third string argument for the delayed diagnostic.
- std::string DelayedDiagArg3;
-
- /// Optional flag value.
- ///
- /// Some flags accept values, for instance: -Wframe-larger-than= and
- /// -Rpass=. The content of this string is emitted after the flag name
- /// and '='.
- std::string FlagValue;
-
public:
explicit DiagnosticsEngine(IntrusiveRefCntPtr Diags,
IntrusiveRefCntPtr DiagOpts,
@@ -945,50 +959,11 @@ class DiagnosticsEngine : public
RefCountedBase {
void Report(const StoredDiagnostic &storedDiag);
- /// Determine whethere there is already a diagnostic in flight.
- bool isDiagnosticInFlight() const {
-return CurDiagID != std::numeric_limits::max();
- }
-
- /// Set the "delayed" diagnostic that will be emitted once
- /// the current diagnostic completes.
- ///
- /// If a diagnostic is already in-flight but the front end must
- /// report a
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox ready_for_review https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -538,24 +511,51 @@ bool DiagnosticsEngine::EmitCurrentDiagnostic(bool Force)
{
Emitted = (DiagLevel != DiagnosticIDs::Ignored);
if (Emitted) {
// Emit the diagnostic regardless of suppression level.
- Diags->EmitDiag(*this, DiagLevel);
+ Diags->EmitDiag(*this, DB, DiagLevel);
}
} else {
// Process the diagnostic, sending the accumulated information to the
// DiagnosticConsumer.
-Emitted = ProcessDiag();
+Emitted = ProcessDiag(DB);
}
- // Clear out the current diagnostic object.
- Clear();
+ return Emitted;
+}
- // If there was a delayed diagnostic, emit it now.
- if (!Force && DelayedDiagID)
-ReportDelayed();
+DiagnosticBuilder::DiagnosticBuilder(DiagnosticsEngine *diagObj,
+ SourceLocation CurDiagLoc,
+ unsigned CurDiagID)
+: StreamingDiagnostic(diagObj->DiagAllocator), DiagObj(diagObj),
+ CurDiagLoc(CurDiagLoc), CurDiagID(CurDiagID), IsActive(true) {
+ assert(diagObj && "DiagnosticBuilder requires a valid DiagnosticsEngine!");
+}
- return Emitted;
+DiagnosticBuilder::DiagnosticBuilder(const DiagnosticBuilder &D)
+: StreamingDiagnostic() {
+ CurDiagLoc = D.CurDiagLoc;
+ CurDiagID = D.CurDiagID;
+ FlagValue = D.FlagValue;
+ DiagObj = D.DiagObj;
+ DiagStorage = D.DiagStorage;
+ D.DiagStorage = nullptr;
igelbox wrote:
> and be caught off-guard by the copy constructor mutating the original object
Yeah. But this is the original behavior (see the original "and neuters it"
comment above the constructor declaration) even before my patch.
I'll try to declare it as a moving constructor.. and spend another hour to
re-build the code.. soo slow and resource-demanding operation)
https://github.com/llvm/llvm-project/pull/108187
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
igelbox wrote: > Have you run your changes with address and ub sanitizers to see if there are > some uncaught edge cases? Not quire sure I get the point about UB-sanitizers. I tested this changes against my proprietary code which caused the issue with OOM/infinite-loop and it works just fine. I can and will build the `clangd` with assertions enabled and test it against my codebase to try catching other cases. https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
igelbox wrote: > I'm not certain that there is a reasonable way to add test coverage for the > changes, but if you can devise a test, that would be appreciated. I wonder there should be some tests like: - input = some source files - output = expected it compiles well and has no assertions fired but not sure yet where to find such a tests-folder. I'm just a newbie in clang development workflow, so I wonder is someone more experienced would point me that. https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox updated
https://github.com/llvm/llvm-project/pull/108187
>From aee4cf70dedaa3c8b7b6508238e01f92d60c631c Mon Sep 17 00:00:00 2001
From: Sergei
Date: Tue, 10 Sep 2024 16:19:05 +
Subject: [PATCH 1/2] fix quick OOM in FormatDiagnostic
---
.../ClangTidyDiagnosticConsumer.cpp | 2 -
clang/include/clang/Basic/Diagnostic.h| 269 ++
clang/include/clang/Basic/DiagnosticIDs.h | 7 +-
clang/include/clang/Basic/PartialDiagnostic.h | 5 +-
clang/include/clang/Sema/Sema.h | 6 +-
clang/lib/Basic/Diagnostic.cpp| 86 +++---
clang/lib/Basic/DiagnosticIDs.cpp | 22 +-
clang/lib/Basic/SourceManager.cpp | 23 +-
clang/lib/Frontend/Rewrite/FixItRewriter.cpp | 4 +-
clang/lib/Frontend/TextDiagnosticPrinter.cpp | 2 +-
clang/lib/Sema/Sema.cpp | 19 +-
clang/lib/Sema/SemaBase.cpp | 2 +-
clang/lib/Serialization/ASTReader.cpp | 15 +-
clang/unittests/Basic/DiagnosticTest.cpp | 4 -
clang/unittests/Driver/DXCModeTest.cpp| 5 -
15 files changed, 174 insertions(+), 297 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 200bb87a5ac3cb..4c75b422701148 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -380,7 +380,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
LastErrorWasIgnored = true;
-Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
return;
@@ -457,7 +456,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
- Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
}
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index 0c7836c2ea569c..1eecab4f6e49a2 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -183,6 +183,41 @@ struct DiagnosticStorage {
DiagnosticStorage() = default;
};
+/// An allocator for DiagnosticStorage objects, which uses a small cache to
+/// objects, used to reduce malloc()/free() traffic for partial diagnostics.
+class DiagStorageAllocator {
+ static const unsigned NumCached = 16;
+ DiagnosticStorage Cached[NumCached];
+ DiagnosticStorage *FreeList[NumCached];
+ unsigned NumFreeListEntries;
+
+public:
+ DiagStorageAllocator();
+ ~DiagStorageAllocator();
+
+ /// Allocate new storage.
+ DiagnosticStorage *Allocate() {
+if (NumFreeListEntries == 0)
+ return new DiagnosticStorage;
+
+DiagnosticStorage *Result = FreeList[--NumFreeListEntries];
+Result->NumDiagArgs = 0;
+Result->DiagRanges.clear();
+Result->FixItHints.clear();
+return Result;
+ }
+
+ /// Free the given storage object.
+ void Deallocate(DiagnosticStorage *S) {
+if (S >= Cached && S <= Cached + NumCached) {
+ FreeList[NumFreeListEntries++] = S;
+ return;
+}
+
+delete S;
+ }
+};
+
/// Concrete class used by the front-end to report problems and issues.
///
/// This massages the diagnostics (e.g. handling things like "report warnings
@@ -520,27 +555,6 @@ class DiagnosticsEngine : public
RefCountedBase {
void *ArgToStringCookie = nullptr;
ArgToStringFnTy ArgToStringFn;
- /// ID of the "delayed" diagnostic, which is a (typically
- /// fatal) diagnostic that had to be delayed because it was found
- /// while emitting another diagnostic.
- unsigned DelayedDiagID;
-
- /// First string argument for the delayed diagnostic.
- std::string DelayedDiagArg1;
-
- /// Second string argument for the delayed diagnostic.
- std::string DelayedDiagArg2;
-
- /// Third string argument for the delayed diagnostic.
- std::string DelayedDiagArg3;
-
- /// Optional flag value.
- ///
- /// Some flags accept values, for instance: -Wframe-larger-than= and
- /// -Rpass=. The content of this string is emitted after the flag name
- /// and '='.
- std::string FlagValue;
-
public:
explicit DiagnosticsEngine(IntrusiveRefCntPtr Diags,
IntrusiveRefCntPtr DiagOpts,
@@ -945,50 +959,11 @@ class DiagnosticsEngine : public
RefCountedBase {
void Report(const StoredDiagnostic &storedDiag);
- /// Determine whethere there is already a diagnostic in flight.
- bool isDiagnosticInFlight() const {
-return CurDiagID != std::numeric_limits::max();
- }
-
- /// Set the "delayed" diagnostic that will be emitted once
- /// the current diagnostic completes.
- ///
- /// If a diagnostic is already in-flight but the front end must
- /// repo
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -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) igelbox wrote: I researched over the codebase and found many easy to prove usage patterns: - `DiagID` comes from the same `Diag` object, e.g. https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Diagnostic.cpp#L536 => they're interchangeable - code compares the result level with `> DiagnosticsEngine::Warning`, e.g. https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/Driver.cpp#L277 or `>= DiagnosticsEngine::Error` - https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDecl.cpp#L16351 or `if (CurLevel < DiagnosticsEngine::Error)` - https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L498 but both `Fatal` and `Error` are greater than `Warning` (or not less than `Error`) => these comparisons aren't affected at all as well as some not so easy to prove: - https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/ParsedAST.cpp#L578 but it checks levels of a particular DiagIDs => probably we shouldn't rely on how many errors we faced before this checks (many of not so) - https://github.com/llvm/llvm-project/blob/main/clang/tools/diagtool/ShowEnabledWarnings.cpp#L138 ! here I'm not sure should we prevent downgrading some `Fatal`s to `Error`s here if the current diagnostic is in the `fatal_too_many_errors` "state" - https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h#L217 !! the hardest one to prove coz it has so many usages 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. https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -538,24 +511,51 @@ bool DiagnosticsEngine::EmitCurrentDiagnostic(bool Force)
{
Emitted = (DiagLevel != DiagnosticIDs::Ignored);
if (Emitted) {
// Emit the diagnostic regardless of suppression level.
- Diags->EmitDiag(*this, DiagLevel);
+ Diags->EmitDiag(*this, DB, DiagLevel);
}
} else {
// Process the diagnostic, sending the accumulated information to the
// DiagnosticConsumer.
-Emitted = ProcessDiag();
+Emitted = ProcessDiag(DB);
}
- // Clear out the current diagnostic object.
- Clear();
+ return Emitted;
+}
- // If there was a delayed diagnostic, emit it now.
- if (!Force && DelayedDiagID)
-ReportDelayed();
+DiagnosticBuilder::DiagnosticBuilder(DiagnosticsEngine *diagObj,
+ SourceLocation CurDiagLoc,
+ unsigned CurDiagID)
+: StreamingDiagnostic(diagObj->DiagAllocator), DiagObj(diagObj),
+ CurDiagLoc(CurDiagLoc), CurDiagID(CurDiagID), IsActive(true) {
+ assert(diagObj && "DiagnosticBuilder requires a valid DiagnosticsEngine!");
+}
- return Emitted;
+DiagnosticBuilder::DiagnosticBuilder(const DiagnosticBuilder &D)
+: StreamingDiagnostic() {
+ CurDiagLoc = D.CurDiagLoc;
+ CurDiagID = D.CurDiagID;
+ FlagValue = D.FlagValue;
+ DiagObj = D.DiagObj;
+ DiagStorage = D.DiagStorage;
+ D.DiagStorage = nullptr;
igelbox wrote:
Just tried to turn copy-constructor into a moving one and got a whole bunch of
errors like:
```
/home/igelbox/llvm-project/clang/lib/Basic/SourceMgrAdapter.cpp: In member
function ‘void clang::SourceMgrAdapter::handleDiag(const llvm::SMDiagnostic&)’:
/home/igelbox/llvm-project/clang/lib/Basic/SourceMgrAdapter.cpp:120:66: error:
use of deleted function ‘clang::DiagnosticBuilder::DiagnosticBuilder(const
clang::DiagnosticBuilder&)’
120 | DiagnosticBuilder Builder = Diagnostics.Report(Loc, DiagID) <<
Message;
| ^~~
In file included from
/home/igelbox/llvm-project/clang/include/clang/Basic/SourceManager.h:37,
from
/home/igelbox/llvm-project/clang/include/clang/Basic/SourceMgrAdapter.h:17,
from
/home/igelbox/llvm-project/clang/lib/Basic/SourceMgrAdapter.cpp:14:
/home/igelbox/llvm-project/clang/include/clang/Basic/Diagnostic.h:1192:7: note:
‘clang::DiagnosticBuilder::DiagnosticBuilder(const clang::DiagnosticBuilder&)’
is implicitly declared as deleted because ‘clang::DiagnosticBuilder’ declares a
move constructor or move assignment operator
1192 | class DiagnosticBuilder : public StreamingDiagnostic {
| ^
[ 14%] Building R600GenCallingConv.inc...
make[3]: ***
[tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/build.make:530:
tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/SourceMgrAdapter.cpp.o]
Error 1
make[2]: *** [CMakeFiles/Makefile2:41994:
tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/all] Error 2
```
I think they caused by `<<` operators which take `const Builder&` and return
`const Builder&` as well that cannot match the moving constructor argument
(which is a mutable value of course). Then tried to make them all non-const but
it's still failing to compile in several places. I believe it's doable but
afraid it would blow up the amount of changes in this PR. So, keeping in mind
the original behavior was the same ("and neuters it" original comment) I
suggest leaving this surprise-included copy-constructor as it is (like as it
was).
https://github.com/llvm/llvm-project/pull/108187
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox updated
https://github.com/llvm/llvm-project/pull/108187
>From aee4cf70dedaa3c8b7b6508238e01f92d60c631c Mon Sep 17 00:00:00 2001
From: Sergei
Date: Tue, 10 Sep 2024 16:19:05 +
Subject: [PATCH 1/3] fix quick OOM in FormatDiagnostic
---
.../ClangTidyDiagnosticConsumer.cpp | 2 -
clang/include/clang/Basic/Diagnostic.h| 269 ++
clang/include/clang/Basic/DiagnosticIDs.h | 7 +-
clang/include/clang/Basic/PartialDiagnostic.h | 5 +-
clang/include/clang/Sema/Sema.h | 6 +-
clang/lib/Basic/Diagnostic.cpp| 86 +++---
clang/lib/Basic/DiagnosticIDs.cpp | 22 +-
clang/lib/Basic/SourceManager.cpp | 23 +-
clang/lib/Frontend/Rewrite/FixItRewriter.cpp | 4 +-
clang/lib/Frontend/TextDiagnosticPrinter.cpp | 2 +-
clang/lib/Sema/Sema.cpp | 19 +-
clang/lib/Sema/SemaBase.cpp | 2 +-
clang/lib/Serialization/ASTReader.cpp | 15 +-
clang/unittests/Basic/DiagnosticTest.cpp | 4 -
clang/unittests/Driver/DXCModeTest.cpp| 5 -
15 files changed, 174 insertions(+), 297 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 200bb87a5ac3cb..4c75b422701148 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -380,7 +380,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
LastErrorWasIgnored = true;
-Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
return;
@@ -457,7 +456,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
- Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
}
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index 0c7836c2ea569c..1eecab4f6e49a2 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -183,6 +183,41 @@ struct DiagnosticStorage {
DiagnosticStorage() = default;
};
+/// An allocator for DiagnosticStorage objects, which uses a small cache to
+/// objects, used to reduce malloc()/free() traffic for partial diagnostics.
+class DiagStorageAllocator {
+ static const unsigned NumCached = 16;
+ DiagnosticStorage Cached[NumCached];
+ DiagnosticStorage *FreeList[NumCached];
+ unsigned NumFreeListEntries;
+
+public:
+ DiagStorageAllocator();
+ ~DiagStorageAllocator();
+
+ /// Allocate new storage.
+ DiagnosticStorage *Allocate() {
+if (NumFreeListEntries == 0)
+ return new DiagnosticStorage;
+
+DiagnosticStorage *Result = FreeList[--NumFreeListEntries];
+Result->NumDiagArgs = 0;
+Result->DiagRanges.clear();
+Result->FixItHints.clear();
+return Result;
+ }
+
+ /// Free the given storage object.
+ void Deallocate(DiagnosticStorage *S) {
+if (S >= Cached && S <= Cached + NumCached) {
+ FreeList[NumFreeListEntries++] = S;
+ return;
+}
+
+delete S;
+ }
+};
+
/// Concrete class used by the front-end to report problems and issues.
///
/// This massages the diagnostics (e.g. handling things like "report warnings
@@ -520,27 +555,6 @@ class DiagnosticsEngine : public
RefCountedBase {
void *ArgToStringCookie = nullptr;
ArgToStringFnTy ArgToStringFn;
- /// ID of the "delayed" diagnostic, which is a (typically
- /// fatal) diagnostic that had to be delayed because it was found
- /// while emitting another diagnostic.
- unsigned DelayedDiagID;
-
- /// First string argument for the delayed diagnostic.
- std::string DelayedDiagArg1;
-
- /// Second string argument for the delayed diagnostic.
- std::string DelayedDiagArg2;
-
- /// Third string argument for the delayed diagnostic.
- std::string DelayedDiagArg3;
-
- /// Optional flag value.
- ///
- /// Some flags accept values, for instance: -Wframe-larger-than= and
- /// -Rpass=. The content of this string is emitted after the flag name
- /// and '='.
- std::string FlagValue;
-
public:
explicit DiagnosticsEngine(IntrusiveRefCntPtr Diags,
IntrusiveRefCntPtr DiagOpts,
@@ -945,50 +959,11 @@ class DiagnosticsEngine : public
RefCountedBase {
void Report(const StoredDiagnostic &storedDiag);
- /// Determine whethere there is already a diagnostic in flight.
- bool isDiagnosticInFlight() const {
-return CurDiagID != std::numeric_limits::max();
- }
-
- /// Set the "delayed" diagnostic that will be emitted once
- /// the current diagnostic completes.
- ///
- /// If a diagnostic is already in-flight but the front end must
- /// repo
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -0,0 +1,41 @@ +// RUN: %clang_cc1 -fallow-pch-with-compiler-errors -std=c++20 -x c++-header -emit-pch %s -o %t -verify +// RUN: %clang_cc1 -fallow-pch-with-compiler-errors -std=c++20 -include-pch %t %s -verify igelbox wrote: In the `main` branch it didn't fail but simply hangs. Better than nothing though. https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
igelbox wrote: Whew, managed to add both tests. https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox updated
https://github.com/llvm/llvm-project/pull/108187
>From aee4cf70dedaa3c8b7b6508238e01f92d60c631c Mon Sep 17 00:00:00 2001
From: Sergei
Date: Tue, 10 Sep 2024 16:19:05 +
Subject: [PATCH 1/4] fix quick OOM in FormatDiagnostic
---
.../ClangTidyDiagnosticConsumer.cpp | 2 -
clang/include/clang/Basic/Diagnostic.h| 269 ++
clang/include/clang/Basic/DiagnosticIDs.h | 7 +-
clang/include/clang/Basic/PartialDiagnostic.h | 5 +-
clang/include/clang/Sema/Sema.h | 6 +-
clang/lib/Basic/Diagnostic.cpp| 86 +++---
clang/lib/Basic/DiagnosticIDs.cpp | 22 +-
clang/lib/Basic/SourceManager.cpp | 23 +-
clang/lib/Frontend/Rewrite/FixItRewriter.cpp | 4 +-
clang/lib/Frontend/TextDiagnosticPrinter.cpp | 2 +-
clang/lib/Sema/Sema.cpp | 19 +-
clang/lib/Sema/SemaBase.cpp | 2 +-
clang/lib/Serialization/ASTReader.cpp | 15 +-
clang/unittests/Basic/DiagnosticTest.cpp | 4 -
clang/unittests/Driver/DXCModeTest.cpp| 5 -
15 files changed, 174 insertions(+), 297 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 200bb87a5ac3cb..4c75b422701148 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -380,7 +380,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
LastErrorWasIgnored = true;
-Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
return;
@@ -457,7 +456,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
- Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
}
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index 0c7836c2ea569c..1eecab4f6e49a2 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -183,6 +183,41 @@ struct DiagnosticStorage {
DiagnosticStorage() = default;
};
+/// An allocator for DiagnosticStorage objects, which uses a small cache to
+/// objects, used to reduce malloc()/free() traffic for partial diagnostics.
+class DiagStorageAllocator {
+ static const unsigned NumCached = 16;
+ DiagnosticStorage Cached[NumCached];
+ DiagnosticStorage *FreeList[NumCached];
+ unsigned NumFreeListEntries;
+
+public:
+ DiagStorageAllocator();
+ ~DiagStorageAllocator();
+
+ /// Allocate new storage.
+ DiagnosticStorage *Allocate() {
+if (NumFreeListEntries == 0)
+ return new DiagnosticStorage;
+
+DiagnosticStorage *Result = FreeList[--NumFreeListEntries];
+Result->NumDiagArgs = 0;
+Result->DiagRanges.clear();
+Result->FixItHints.clear();
+return Result;
+ }
+
+ /// Free the given storage object.
+ void Deallocate(DiagnosticStorage *S) {
+if (S >= Cached && S <= Cached + NumCached) {
+ FreeList[NumFreeListEntries++] = S;
+ return;
+}
+
+delete S;
+ }
+};
+
/// Concrete class used by the front-end to report problems and issues.
///
/// This massages the diagnostics (e.g. handling things like "report warnings
@@ -520,27 +555,6 @@ class DiagnosticsEngine : public
RefCountedBase {
void *ArgToStringCookie = nullptr;
ArgToStringFnTy ArgToStringFn;
- /// ID of the "delayed" diagnostic, which is a (typically
- /// fatal) diagnostic that had to be delayed because it was found
- /// while emitting another diagnostic.
- unsigned DelayedDiagID;
-
- /// First string argument for the delayed diagnostic.
- std::string DelayedDiagArg1;
-
- /// Second string argument for the delayed diagnostic.
- std::string DelayedDiagArg2;
-
- /// Third string argument for the delayed diagnostic.
- std::string DelayedDiagArg3;
-
- /// Optional flag value.
- ///
- /// Some flags accept values, for instance: -Wframe-larger-than= and
- /// -Rpass=. The content of this string is emitted after the flag name
- /// and '='.
- std::string FlagValue;
-
public:
explicit DiagnosticsEngine(IntrusiveRefCntPtr Diags,
IntrusiveRefCntPtr DiagOpts,
@@ -945,50 +959,11 @@ class DiagnosticsEngine : public
RefCountedBase {
void Report(const StoredDiagnostic &storedDiag);
- /// Determine whethere there is already a diagnostic in flight.
- bool isDiagnosticInFlight() const {
-return CurDiagID != std::numeric_limits::max();
- }
-
- /// Set the "delayed" diagnostic that will be emitted once
- /// the current diagnostic completes.
- ///
- /// If a diagnostic is already in-flight but the front end must
- /// repo
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -945,50 +959,11 @@ class DiagnosticsEngine : public
RefCountedBase {
void Report(const StoredDiagnostic &storedDiag);
- /// Determine whethere there is already a diagnostic in flight.
- bool isDiagnosticInFlight() const {
-return CurDiagID != std::numeric_limits::max();
- }
-
- /// Set the "delayed" diagnostic that will be emitted once
- /// the current diagnostic completes.
- ///
- /// If a diagnostic is already in-flight but the front end must
- /// report a problem (e.g., with an inconsistent file system
- /// state), this routine sets a "delayed" diagnostic that will be
- /// emitted after the current diagnostic completes. This should
- /// only be used for fatal errors detected at inconvenient
- /// times. If emitting a delayed diagnostic causes a second delayed
- /// diagnostic to be introduced, that second delayed diagnostic
- /// will be ignored.
- ///
- /// \param DiagID The ID of the diagnostic being delayed.
- ///
- /// \param Arg1 A string argument that will be provided to the
- /// diagnostic. A copy of this string will be stored in the
- /// DiagnosticsEngine object itself.
- ///
- /// \param Arg2 A string argument that will be provided to the
- /// diagnostic. A copy of this string will be stored in the
- /// DiagnosticsEngine object itself.
- ///
- /// \param Arg3 A string argument that will be provided to the
- /// diagnostic. A copy of this string will be stored in the
- /// DiagnosticsEngine object itself.
- void SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1 = "",
-StringRef Arg2 = "", StringRef Arg3 = "");
-
- /// Clear out the current diagnostic.
- void Clear() { CurDiagID = std::numeric_limits::max(); }
-
- /// Return the value associated with this diagnostic flag.
- StringRef getFlagValue() const { return FlagValue; }
-
private:
// This is private state used by DiagnosticBuilder. We put it here instead
of
// in DiagnosticBuilder in order to keep DiagnosticBuilder a small
lightweight
- // object. This implementation choice means that we can only have one
- // diagnostic "in flight" at a time, but this seems to be a reasonable
+ // object. This implementation choice means that we can only have a few
+ // diagnostics "in flight" at a time, but this seems to be a reasonable
// tradeoff to keep these objects small. Assertions verify that only one
// diagnostic is in flight at a time.
igelbox wrote:
Okay. Removed misleading `Cur`-prefix (coz it isn't current it's the one per
builder/diagnostic obj) and seemingly meaningless comments as well.
https://github.com/llvm/llvm-project/pull/108187
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
igelbox wrote: > LGTM! Do you need someone to land these changes on your behalf? I suppose so.. or what are my options? The Merge button isn't active of course. I believe someone from maintainers would merge this PR and I will be so proud) https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
https://github.com/igelbox created
https://github.com/llvm/llvm-project/pull/108866
The last attempt failed -
https://github.com/llvm/llvm-project/pull/108187#issuecomment-2353122096
so was reverted - #108838
Not quite sure how to build targets available, so I run the same `cmake` and
the `ninja` w/o arguments:
- [x]
[build-unified-tree](https://lab.llvm.org/buildbot/#/builders/157/builds/7831)
`cmake -DLLVM_TARGETS_TO_BUILD=PowerPC -DLLVM_INSTALL_UTILS=ON
-DCMAKE_CXX_STANDARD=17 '-DLLVM_LIT_ARGS=-vj 256' -DFLANG_ENABLE_WERROR=ON
-DLLVM_ENABLE_ASSERTIONS=ON '-DLLVM_ENABLE_PROJECTS=llvm;mlir;flang;clang'
-DCMAKE_BUILD_TYPE=Release -GNinja ../llvm`
- [ ]
[build-unified-tree](https://lab.llvm.org/buildbot/#/builders/190/builds/5825)
`cmake -DCMAKE_BUILD_TYPE=Release
'-DLLVM_ENABLE_PROJECTS=llvm;cross-project-tests;lld;clang-tools-extra;clang'
../llvm`
- [ ]
[build-unified-tree](https://lab.llvm.org/buildbot/#/builders/89/builds/6426)
- [x]
~[build-unified-tree](https://lab.llvm.org/buildbot/#/builders/153/builds/9025)~
ignored, the same error
- [ ]
[test-build-clangd-clangd-index-server-clangd-indexer-check-clangd](https://lab.llvm.org/buildbot/#/builders/134/builds/5347)
- [ ] [ninja check](https://lab.llvm.org/buildbot/#/builders/65/builds/4601)
- [ ]
[test-build-clangd-clangd-index-server-clangd-indexer-check-clangd](https://lab.llvm.org/buildbot/#/builders/134/builds/5347)
- [ ]
[test-build-unified-tree-check-all](https://lab.llvm.org/buildbot/#/builders/56/builds/7472)
- [ ] [compile-openmp](https://lab.llvm.org/buildbot/#/builders/140/builds/6654)
>From aee4cf70dedaa3c8b7b6508238e01f92d60c631c Mon Sep 17 00:00:00 2001
From: Sergei
Date: Tue, 10 Sep 2024 16:19:05 +
Subject: [PATCH 1/5] fix quick OOM in FormatDiagnostic
---
.../ClangTidyDiagnosticConsumer.cpp | 2 -
clang/include/clang/Basic/Diagnostic.h| 269 ++
clang/include/clang/Basic/DiagnosticIDs.h | 7 +-
clang/include/clang/Basic/PartialDiagnostic.h | 5 +-
clang/include/clang/Sema/Sema.h | 6 +-
clang/lib/Basic/Diagnostic.cpp| 86 +++---
clang/lib/Basic/DiagnosticIDs.cpp | 22 +-
clang/lib/Basic/SourceManager.cpp | 23 +-
clang/lib/Frontend/Rewrite/FixItRewriter.cpp | 4 +-
clang/lib/Frontend/TextDiagnosticPrinter.cpp | 2 +-
clang/lib/Sema/Sema.cpp | 19 +-
clang/lib/Sema/SemaBase.cpp | 2 +-
clang/lib/Serialization/ASTReader.cpp | 15 +-
clang/unittests/Basic/DiagnosticTest.cpp | 4 -
clang/unittests/Driver/DXCModeTest.cpp| 5 -
15 files changed, 174 insertions(+), 297 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 200bb87a5ac3cb..4c75b422701148 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -380,7 +380,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
LastErrorWasIgnored = true;
-Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
return;
@@ -457,7 +456,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
- Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
}
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index 0c7836c2ea569c..1eecab4f6e49a2 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -183,6 +183,41 @@ struct DiagnosticStorage {
DiagnosticStorage() = default;
};
+/// An allocator for DiagnosticStorage objects, which uses a small cache to
+/// objects, used to reduce malloc()/free() traffic for partial diagnostics.
+class DiagStorageAllocator {
+ static const unsigned NumCached = 16;
+ DiagnosticStorage Cached[NumCached];
+ DiagnosticStorage *FreeList[NumCached];
+ unsigned NumFreeListEntries;
+
+public:
+ DiagStorageAllocator();
+ ~DiagStorageAllocator();
+
+ /// Allocate new storage.
+ DiagnosticStorage *Allocate() {
+if (NumFreeListEntries == 0)
+ return new DiagnosticStorage;
+
+DiagnosticStorage *Result = FreeList[--NumFreeListEntries];
+Result->NumDiagArgs = 0;
+Result->DiagRanges.clear();
+Result->FixItHints.clear();
+return Result;
+ }
+
+ /// Free the given storage object.
+ void Deallocate(DiagnosticStorage *S) {
+if (S >= Cached && S <= Cached + NumCached) {
+ FreeList[NumFreeListEntries++] = S;
+ return;
+}
+
+delete S;
+ }
+};
+
/// Concrete class used by the front-end to report problems and issues.
///
/// This massages
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
https://github.com/igelbox updated
https://github.com/llvm/llvm-project/pull/108866
>From aee4cf70dedaa3c8b7b6508238e01f92d60c631c Mon Sep 17 00:00:00 2001
From: Sergei
Date: Tue, 10 Sep 2024 16:19:05 +
Subject: [PATCH 1/5] fix quick OOM in FormatDiagnostic
---
.../ClangTidyDiagnosticConsumer.cpp | 2 -
clang/include/clang/Basic/Diagnostic.h| 269 ++
clang/include/clang/Basic/DiagnosticIDs.h | 7 +-
clang/include/clang/Basic/PartialDiagnostic.h | 5 +-
clang/include/clang/Sema/Sema.h | 6 +-
clang/lib/Basic/Diagnostic.cpp| 86 +++---
clang/lib/Basic/DiagnosticIDs.cpp | 22 +-
clang/lib/Basic/SourceManager.cpp | 23 +-
clang/lib/Frontend/Rewrite/FixItRewriter.cpp | 4 +-
clang/lib/Frontend/TextDiagnosticPrinter.cpp | 2 +-
clang/lib/Sema/Sema.cpp | 19 +-
clang/lib/Sema/SemaBase.cpp | 2 +-
clang/lib/Serialization/ASTReader.cpp | 15 +-
clang/unittests/Basic/DiagnosticTest.cpp | 4 -
clang/unittests/Driver/DXCModeTest.cpp| 5 -
15 files changed, 174 insertions(+), 297 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 200bb87a5ac3cb..4c75b422701148 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -380,7 +380,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
LastErrorWasIgnored = true;
-Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
return;
@@ -457,7 +456,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
- Context.DiagEngine->Clear();
for (const auto &Error : SuppressionErrors)
Context.diag(Error);
}
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index 0c7836c2ea569c..1eecab4f6e49a2 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -183,6 +183,41 @@ struct DiagnosticStorage {
DiagnosticStorage() = default;
};
+/// An allocator for DiagnosticStorage objects, which uses a small cache to
+/// objects, used to reduce malloc()/free() traffic for partial diagnostics.
+class DiagStorageAllocator {
+ static const unsigned NumCached = 16;
+ DiagnosticStorage Cached[NumCached];
+ DiagnosticStorage *FreeList[NumCached];
+ unsigned NumFreeListEntries;
+
+public:
+ DiagStorageAllocator();
+ ~DiagStorageAllocator();
+
+ /// Allocate new storage.
+ DiagnosticStorage *Allocate() {
+if (NumFreeListEntries == 0)
+ return new DiagnosticStorage;
+
+DiagnosticStorage *Result = FreeList[--NumFreeListEntries];
+Result->NumDiagArgs = 0;
+Result->DiagRanges.clear();
+Result->FixItHints.clear();
+return Result;
+ }
+
+ /// Free the given storage object.
+ void Deallocate(DiagnosticStorage *S) {
+if (S >= Cached && S <= Cached + NumCached) {
+ FreeList[NumFreeListEntries++] = S;
+ return;
+}
+
+delete S;
+ }
+};
+
/// Concrete class used by the front-end to report problems and issues.
///
/// This massages the diagnostics (e.g. handling things like "report warnings
@@ -520,27 +555,6 @@ class DiagnosticsEngine : public
RefCountedBase {
void *ArgToStringCookie = nullptr;
ArgToStringFnTy ArgToStringFn;
- /// ID of the "delayed" diagnostic, which is a (typically
- /// fatal) diagnostic that had to be delayed because it was found
- /// while emitting another diagnostic.
- unsigned DelayedDiagID;
-
- /// First string argument for the delayed diagnostic.
- std::string DelayedDiagArg1;
-
- /// Second string argument for the delayed diagnostic.
- std::string DelayedDiagArg2;
-
- /// Third string argument for the delayed diagnostic.
- std::string DelayedDiagArg3;
-
- /// Optional flag value.
- ///
- /// Some flags accept values, for instance: -Wframe-larger-than= and
- /// -Rpass=. The content of this string is emitted after the flag name
- /// and '='.
- std::string FlagValue;
-
public:
explicit DiagnosticsEngine(IntrusiveRefCntPtr Diags,
IntrusiveRefCntPtr DiagOpts,
@@ -945,50 +959,11 @@ class DiagnosticsEngine : public
RefCountedBase {
void Report(const StoredDiagnostic &storedDiag);
- /// Determine whethere there is already a diagnostic in flight.
- bool isDiagnosticInFlight() const {
-return CurDiagID != std::numeric_limits::max();
- }
-
- /// Set the "delayed" diagnostic that will be emitted once
- /// the current diagnostic completes.
- ///
- /// If a diagnostic is already in-flight but the front end must
- /// repo
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
igelbox wrote: Errors were fixed. If there's some other `make` target that could catch even more errors - please lemme know. cc: @AaronBallman I'm gonna remove Draft from this PR after `check-all` target finish with OK-state. https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
igelbox wrote: > Testing Time: 301.48s Total Discovered Tests: 105926 Skipped : 23 (0.02%) Unsupported : 2682 (2.53%) Passed : 103036 (97.27%) Expectedly Failed:185 (0.17%) 1 warning(s) in tests https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
https://github.com/igelbox ready_for_review https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
igelbox wrote: Well, the more it waits the bigger a probability it will have logical conflicts after merging into `main`. https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
igelbox wrote: 30 of 31, this time is much closer) https://github.com/user-attachments/assets/de806772-30df-42bd-9928-19bad35dca02";> it seems like [those](https://lab.llvm.org/buildbot/#/builders/73) builds were broken a couple hours ago: https://github.com/user-attachments/assets/c375276a-d3ac-4e09-a6fd-2a2aced0539c";> https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] Fix OOM in FormatDiagnostic (2nd attempt) (PR #108866)
https://github.com/igelbox edited https://github.com/llvm/llvm-project/pull/108866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
