Author: Danila Kutenin Date: 2021-12-20T15:56:57+01:00 New Revision: cff192739bb6dd67b2ec58f055af2ae2834c4348
URL: https://github.com/llvm/llvm-project/commit/cff192739bb6dd67b2ec58f055af2ae2834c4348 DIFF: https://github.com/llvm/llvm-project/commit/cff192739bb6dd67b2ec58f055af2ae2834c4348.diff LOG: [clang-include-fixer] Fix incorrect ranking because of dangling references SymbolAndSignals stores SymbolInfo which stores two std::strings. Then the values are stored in a llvm::DenseMap<llvm::StringRef, double>. When the sorting is happening, SymbolAndSignals are swapped and thus because of small string optimization some strings may become invalid. This results in incorrect ranking. This was detected when running new std::sort algorithm against llvm toolchain. This could have been prevented with running llvm::sort and EXPENSIVE_CHECKS. Unfortunately, no sanitizer yelled. I don't have commit rights, kutdan...@yandex.ru Danila Kutenin Reviewed By: bkramer Differential Revision: https://reviews.llvm.org/D116037 Added: Modified: clang-tools-extra/clang-include-fixer/SymbolIndexManager.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-include-fixer/SymbolIndexManager.cpp b/clang-tools-extra/clang-include-fixer/SymbolIndexManager.cpp index cbd79b64eae09..ebb4a70c1f8db 100644 --- a/clang-tools-extra/clang-include-fixer/SymbolIndexManager.cpp +++ b/clang-tools-extra/clang-include-fixer/SymbolIndexManager.cpp @@ -8,8 +8,9 @@ #include "SymbolIndexManager.h" #include "find-all-symbols/SymbolInfo.h" -#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Path.h" @@ -47,7 +48,7 @@ static double similarityScore(llvm::StringRef FileName, static void rank(std::vector<SymbolAndSignals> &Symbols, llvm::StringRef FileName) { - llvm::DenseMap<llvm::StringRef, double> Score; + llvm::StringMap<double> Score; for (const auto &Symbol : Symbols) { // Calculate a score from the similarity of the header the symbol is in // with the current file and the popularity of the symbol. @@ -58,14 +59,14 @@ static void rank(std::vector<SymbolAndSignals> &Symbols, } // Sort by the gathered scores. Use file name as a tie breaker so we can // deduplicate. - std::sort(Symbols.begin(), Symbols.end(), - [&](const SymbolAndSignals &A, const SymbolAndSignals &B) { - auto AS = Score[A.Symbol.getFilePath()]; - auto BS = Score[B.Symbol.getFilePath()]; - if (AS != BS) - return AS > BS; - return A.Symbol.getFilePath() < B.Symbol.getFilePath(); - }); + llvm::sort(Symbols.begin(), Symbols.end(), + [&](const SymbolAndSignals &A, const SymbolAndSignals &B) { + auto AS = Score[A.Symbol.getFilePath()]; + auto BS = Score[B.Symbol.getFilePath()]; + if (AS != BS) + return AS > BS; + return A.Symbol.getFilePath() < B.Symbol.getFilePath(); + }); } std::vector<find_all_symbols::SymbolInfo> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits