[PATCH] D116037: [clang-include-fixer] Fix incorrect ranking because of dangling references
Danila Kutenin via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 20 05:48:01 PST 2021
danlark created this revision.
danlark added reviewers: alexfh, klimek.
Herald added subscribers: arphaman, mgrang.
danlark requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116037
Files:
clang-tools-extra/clang-include-fixer/SymbolIndexManager.cpp
Index: clang-tools-extra/clang-include-fixer/SymbolIndexManager.cpp
===================================================================
--- clang-tools-extra/clang-include-fixer/SymbolIndexManager.cpp
+++ 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 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 @@
}
// 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>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116037.395425.patch
Type: text/x-patch
Size: 1884 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211220/6df34fc8/attachment.bin>
More information about the cfe-commits
mailing list