This revision was automatically updated to reflect the committed changes.
Closed by commit rL326569: [clang] Fix use-after-free on code completion
(authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D44000
Files:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -429,13 +429,19 @@
// It filters out ignored results (but doesn't apply fuzzy-filtering yet).
// It doesn't do scoring or conversion to CompletionItem yet, as we want to
// merge with index results first.
+// Generally the fields and methods of this object should only be used from
+// within the callback.
struct CompletionRecorder : public CodeCompleteConsumer {
- CompletionRecorder(const CodeCompleteOptions &Opts)
+ CompletionRecorder(const CodeCompleteOptions &Opts,
+ UniqueFunction<void()> ResultsCallback)
: CodeCompleteConsumer(Opts.getClangCompleteOpts(),
/*OutputIsBinary=*/false),
CCContext(CodeCompletionContext::CCC_Other), Opts(Opts),
CCAllocator(std::make_shared<GlobalCodeCompletionAllocator>()),
- CCTUInfo(CCAllocator) {}
+ CCTUInfo(CCAllocator), ResultsCallback(std::move(ResultsCallback)) {
+ assert(this->ResultsCallback);
+ }
+
std::vector<CodeCompletionResult> Results;
CodeCompletionContext CCContext;
Sema *CCSema = nullptr; // Sema that created the results.
@@ -466,6 +472,7 @@
continue;
Results.push_back(Result);
}
+ ResultsCallback();
}
CodeCompletionAllocator &getAllocator() override { return *CCAllocator; }
@@ -503,6 +510,7 @@
CodeCompleteOptions Opts;
std::shared_ptr<GlobalCodeCompletionAllocator> CCAllocator;
CodeCompletionTUInfo CCTUInfo;
+ UniqueFunction<void()> ResultsCallback;
};
// Tracks a bounded number of candidates with the best scores.
@@ -657,12 +665,10 @@
};
// Invokes Sema code completion on a file.
-// Callback will be invoked once completion is done, but before cleaning up.
bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
const clang::CodeCompleteOptions &Options,
- const SemaCompleteInput &Input,
- llvm::function_ref<void()> Callback = nullptr) {
- auto Tracer = llvm::make_unique<trace::Span>("Sema completion");
+ const SemaCompleteInput &Input) {
+ trace::Span Tracer("Sema completion");
std::vector<const char *> ArgStrs;
for (const auto &S : Input.Command.CommandLine)
ArgStrs.push_back(S.c_str());
@@ -729,12 +735,6 @@
log("Execute() failed when running codeComplete for " + Input.FileName);
return false;
}
- Tracer.reset();
-
- if (Callback)
- Callback();
-
- Tracer = llvm::make_unique<trace::Span>("Sema completion cleanup");
Action.EndSourceFile();
return true;
@@ -813,7 +813,7 @@
// other arenas, which must stay alive for us to build CompletionItems.
// - we may get duplicate results from Sema and the Index, we need to merge.
//
-// So we start Sema completion first, but defer its cleanup until we're done.
+// So we start Sema completion first, and do all our work in its callback.
// We use the Sema context information to query the index.
// Then we merge the two result sets, producing items that are Sema/Index/Both.
// These items are scored, and the top N are synthesized into the LSP response.
@@ -834,34 +834,32 @@
PathRef FileName;
const CodeCompleteOptions &Opts;
// Sema takes ownership of Recorder. Recorder is valid until Sema cleanup.
- std::unique_ptr<CompletionRecorder> RecorderOwner;
- CompletionRecorder &Recorder;
+ CompletionRecorder *Recorder = nullptr;
int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
bool Incomplete = false; // Would more be available with a higher limit?
llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
public:
// A CodeCompleteFlow object is only useful for calling run() exactly once.
CodeCompleteFlow(PathRef FileName, const CodeCompleteOptions &Opts)
- : FileName(FileName), Opts(Opts),
- RecorderOwner(new CompletionRecorder(Opts)), Recorder(*RecorderOwner) {}
+ : FileName(FileName), Opts(Opts) {}
CompletionList run(const SemaCompleteInput &SemaCCInput) && {
trace::Span Tracer("CodeCompleteFlow");
// We run Sema code completion first. It builds an AST and calculates:
- // - completion results based on the AST. These are saved for merging.
+ // - completion results based on the AST.
// - partial identifier and context. We need these for the index query.
CompletionList Output;
+ auto RecorderOwner = llvm::make_unique<CompletionRecorder>(Opts, [&]() {
+ assert(Recorder && "Recorder is not set");
+ Output = runWithSema();
+ SPAN_ATTACH(Tracer, "sema_completion_kind",
+ getCompletionKindString(Recorder->CCContext.getKind()));
+ });
+
+ Recorder = RecorderOwner.get();
semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
- SemaCCInput, [&] {
- if (Recorder.CCSema) {
- Output = runWithSema();
- SPAN_ATTACH(
- Tracer, "sema_completion_kind",
- getCompletionKindString(Recorder.CCContext.getKind()));
- } else
- log("Code complete: no Sema callback, 0 results");
- });
+ SemaCCInput);
SPAN_ATTACH(Tracer, "sema_results", NSema);
SPAN_ATTACH(Tracer, "index_results", NIndex);
@@ -883,15 +881,15 @@
// Sema data structures are torn down. It does all the real work.
CompletionList runWithSema() {
Filter = FuzzyMatcher(
- Recorder.CCSema->getPreprocessor().getCodeCompletionFilter());
+ Recorder->CCSema->getPreprocessor().getCodeCompletionFilter());
// Sema provides the needed context to query the index.
// FIXME: in addition to querying for extra/overlapping symbols, we should
// explicitly request symbols corresponding to Sema results.
// We can use their signals even if the index can't suggest them.
// We must copy index results to preserve them, but there are at most Limit.
auto IndexResults = queryIndex();
// Merge Sema and Index results, score them, and pick the winners.
- auto Top = mergeResults(Recorder.Results, IndexResults);
+ auto Top = mergeResults(Recorder->Results, IndexResults);
// Convert the results to the desired LSP structs.
CompletionList Output;
for (auto &C : Top)
@@ -901,7 +899,7 @@
}
SymbolSlab queryIndex() {
- if (!Opts.Index || !allowIndex(Recorder.CCContext.getKind()))
+ if (!Opts.Index || !allowIndex(Recorder->CCContext.getKind()))
return SymbolSlab();
trace::Span Tracer("Query index");
SPAN_ATTACH(Tracer, "limit", Opts.Limit);
@@ -912,8 +910,8 @@
if (Opts.Limit)
Req.MaxCandidateCount = Opts.Limit;
Req.Query = Filter->pattern();
- Req.Scopes =
- getQueryScopes(Recorder.CCContext, Recorder.CCSema->getSourceManager());
+ Req.Scopes = getQueryScopes(Recorder->CCContext,
+ Recorder->CCSema->getSourceManager());
log(llvm::formatv("Code complete: fuzzyFind(\"{0}\", scopes=[{1}])",
Req.Query,
llvm::join(Req.Scopes.begin(), Req.Scopes.end(), ",")));
@@ -945,7 +943,7 @@
return nullptr;
};
// Emit all Sema results, merging them with Index results if possible.
- for (auto &SemaResult : Recorder.Results)
+ for (auto &SemaResult : Recorder->Results)
addCandidate(Top, &SemaResult, CorrespondingIndexResult(SemaResult));
// Now emit any Index-only results.
for (const auto &IndexResult : IndexResults) {
@@ -962,7 +960,7 @@
CompletionCandidate C;
C.SemaResult = SemaResult;
C.IndexResult = IndexResult;
- C.Name = IndexResult ? IndexResult->Name : Recorder.getName(*SemaResult);
+ C.Name = IndexResult ? IndexResult->Name : Recorder->getName(*SemaResult);
CompletionItemScores Scores;
if (auto FuzzyScore = Filter->match(C.Name))
@@ -986,7 +984,7 @@
const CompletionItemScores &Scores) {
CodeCompletionString *SemaCCS = nullptr;
if (auto *SR = Candidate.SemaResult)
- SemaCCS = Recorder.codeCompletionString(*SR, Opts.IncludeBriefComments);
+ SemaCCS = Recorder->codeCompletionString(*SR, Opts.IncludeBriefComments);
return Candidate.build(FileName, Scores, Opts, SemaCCS);
}
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
