Skip to content

Commit e5d2556

Browse files
authored
Fix OOM in FormatDiagnostic (#108187)
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 `DiagnosticBuilder`s having independent states. Also, encapsulating the rest of state (e.g. ID and Loc) into `DiagnosticBuilder`. **TODO (if it will be requested by reviewer):** - [x] 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](#70930 (comment)) instead) - [x] [`Diag.CurDiagID != diag::fatal_too_many_errors`](#108187 (review)) - [ ] ? 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
1 parent e6618aa commit e5d2556

File tree

16 files changed

+226
-300
lines changed

16 files changed

+226
-300
lines changed

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
380380
++Context.Stats.ErrorsIgnoredNOLINT;
381381
// Ignored a warning, should ignore related notes as well
382382
LastErrorWasIgnored = true;
383-
Context.DiagEngine->Clear();
384383
for (const auto &Error : SuppressionErrors)
385384
Context.diag(Error);
386385
return;
@@ -457,7 +456,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
457456
if (Info.hasSourceManager())
458457
checkFilters(Info.getLocation(), Info.getSourceManager());
459458

460-
Context.DiagEngine->Clear();
461459
for (const auto &Error : SuppressionErrors)
462460
Context.diag(Error);
463461
}

0 commit comments

Comments
 (0)