[all-commits] [llvm/llvm-project] eda72f: Fix OOM in FormatDiagnostic (2nd attempt) (#108866)

Vakhurin Sergei via All-commits all-commits at lists.llvm.org
Wed Sep 18 08:46:47 PDT 2024


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: eda72fac548f317cec997967494763e9a7bafa27
      https://github.com/llvm/llvm-project/commit/eda72fac548f317cec997967494763e9a7bafa27
  Author: Vakhurin Sergei <igelbox at gmail.com>
  Date:   2024-09-18 (Wed, 18 Sep 2024)

  Changed paths:
    M clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
    M clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
    M clang/include/clang/Basic/Diagnostic.h
    M clang/include/clang/Basic/DiagnosticIDs.h
    M clang/include/clang/Basic/PartialDiagnostic.h
    M clang/include/clang/Sema/Sema.h
    M clang/lib/Basic/Diagnostic.cpp
    M clang/lib/Basic/DiagnosticIDs.cpp
    M clang/lib/Basic/SourceManager.cpp
    M clang/lib/Frontend/Rewrite/FixItRewriter.cpp
    M clang/lib/Frontend/TextDiagnosticPrinter.cpp
    M clang/lib/Sema/Sema.cpp
    M clang/lib/Sema/SemaBase.cpp
    M clang/lib/Serialization/ASTReader.cpp
    A clang/test/PCH/race-condition.cpp
    M clang/unittests/Basic/DiagnosticTest.cpp
    M clang/unittests/Driver/DXCModeTest.cpp
    M flang/lib/Frontend/TextDiagnosticPrinter.cpp

  Log Message:
  -----------
  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



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications


More information about the All-commits mailing list