[clang-tools-extra] cff1927 - [clang-include-fixer] Fix incorrect ranking because of dangling references

[Benjamin Kramer via cfe-commits]

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, kutdanila at 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>