ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: ioeric, jkorous-apple, klimek.
Found by asan. Fiddling with code completion AST after
FrontendAction::Exceute can lead to errors.
Calling the callback in ProcessCodeCompleteResults to make sure we
don't access uninitialized state.
This particular issue comes from the fact that Sema::TUScope is
deleted when destructor of ~Parser runs, but still present in
Sema::TUScope and accessed when building completion items.
I'm still struggling to come up with a small repro. The relevant
stackframes reported by asan are:
ERROR: AddressSanitizer: heap-use-after-free on address
READ of size 8 at 0x61400020d090 thread T175
#0 0x5632dff7821b in llvm::SmallPtrSetImplBase::isSmall() const
include/llvm/ADT/SmallPtrSet.h:195:33
#1 0x5632e0335901 in llvm::SmallPtrSetImplBase::insert_imp(void const*)
include/llvm/ADT/SmallPtrSet.h:127:9
#2 0x5632e067347d in
llvm::SmallPtrSetImpl<clang::Decl*>::insert(clang::Decl*)
include/llvm/ADT/SmallPtrSet.h:372:14
#3 0x5632e065df80 in clang::Scope::AddDecl(clang::Decl*)
tools/clang/include/clang/Sema/Scope.h:287:18
#4 0x5632e0623eea in
clang::ASTReader::pushExternalDeclIntoScope(clang::NamedDecl*,
clang::DeclarationName) clang/lib/Serialization/ASTReader.cpp
#5 0x5632e062ce74 in clang::ASTReader::finishPendingActions()
tools/clang/lib/Serialization/ASTReader.cpp:9164:9
....
#30 0x5632e02009c4 in clang::index::generateUSRForDecl(clang::Decl const*,
llvm::SmallVectorImpl<char>&) tools/clang/lib/Index/USRGeneration.cpp:1037:6
#31 0x5632dff73eab in clang::clangd::(anonymous
namespace)::getSymbolID(clang::CodeCompletionResult const&)
tools/clang/tools/extra/clangd/CodeComplete.cpp:326:20
#32 0x5632dff6fe91 in
clang::clangd::CodeCompleteFlow::mergeResults(std::vector<clang::CodeCompletionResult,
std::allocator<clang::CodeCompletionResult> > const&,
clang::clangd::SymbolSlab const&)::'lambda'(clang::CodeCompletionResult
const&)::operator()(clang::CodeCompletionResult const&)
tools/clang/tools/extra/clangd/CodeComplete.cpp:938:24
#33 0x5632dff6e426 in
clang::clangd::CodeCompleteFlow::mergeResults(std::vector<clang::CodeCompletionResult,
std::allocator<clang::CodeCompletionResult> > const&,
clang::clangd::SymbolSlab const&)
third_party/llvm/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:949:38
#34 0x5632dff7a34d in clang::clangd::CodeCompleteFlow::runWithSema()
llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:894:16
#35 0x5632dff6df6a in
clang::clangd::CodeCompleteFlow::run(clang::clangd::(anonymous
namespace)::SemaCompleteInput const&) &&::'lambda'()::operator()() const
third_party/llvm/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:858:35
#36 0x5632dff6cd42 in clang::clangd::(anonymous
namespace)::semaCodeComplete(std::unique_ptr<clang::CodeCompleteConsumer,
std::default_delete<clang::CodeCompleteConsumer> >, clang::CodeCompleteOptions
const&, clang::clangd::(anonymous namespace)::SemaCompleteInput const&,
llvm::function_ref<void ()>)
tools/clang/tools/extra/clangd/CodeComplete.cpp:735:5
0x61400020d090 is located 80 bytes inside of 432-byte region
[0x61400020d040,0x61400020d1f0)
freed by thread T175 here:
#0 0x5632df74e115 in operator delete(void*, unsigned long)
projects/compiler-rt/lib/asan/asan_new_delete.cc:161:3
#1 0x5632e0b06973 in clang::Parser::~Parser()
tools/clang/lib/Parse/Parser.cpp:410:3
#2 0x5632e0b06ddd in clang::Parser::~Parser()
clang/lib/Parse/Parser.cpp:408:19
#3 0x5632e0b03286 in std::unique_ptr<clang::Parser,
std::default_delete<clang::Parser> >::~unique_ptr() .../bits/unique_ptr.h:236:4
#4 0x5632e0b021c4 in clang::ParseAST(clang::Sema&, bool, bool)
tools/clang/lib/Parse/ParseAST.cpp:182:1
#5 0x5632e0726544 in clang::FrontendAction::Execute()
tools/clang/lib/Frontend/FrontendAction.cpp:904:8
#6 0x5632dff6cd05 in clang::clangd::(anonymous
namespace)::semaCodeComplete(std::unique_ptr<clang::CodeCompleteConsumer,
std::default_delete<clang::CodeCompleteConsumer> >, clang::CodeCompleteOptions
const&, clang::clangd::(anonymous namespace)::SemaCompleteInput const&,
llvm::function_ref<void ()>)
tools/clang/tools/extra/clangd/CodeComplete.cpp:728:15
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44000
Files:
clangd/CodeComplete.cpp
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -425,30 +425,32 @@
return Info.scopesForIndexQuery();
}
+struct SemaCtx {
+ Sema &CCSema;
+ CodeCompletionContext &CCContext;
+ GlobalCodeCompletionAllocator &CCAllocator;
+ CodeCompletionTUInfo &CCTUInfo;
+};
+
// The CompletionRecorder captures Sema code-complete output, including context.
// 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.
struct CompletionRecorder : public CodeCompleteConsumer {
- CompletionRecorder(const CodeCompleteOptions &Opts)
+ using ResultsFn =
+ UniqueFunction<void(SemaCtx, llvm::ArrayRef<CodeCompletionResult>)>;
+
+ CompletionRecorder(const CodeCompleteOptions &Opts, ResultsFn ResultsCallback)
: CodeCompleteConsumer(Opts.getClangCompleteOpts(),
/*OutputIsBinary=*/false),
- CCContext(CodeCompletionContext::CCC_Other), Opts(Opts),
+ Opts(Opts),
CCAllocator(std::make_shared<GlobalCodeCompletionAllocator>()),
- CCTUInfo(CCAllocator) {}
- std::vector<CodeCompletionResult> Results;
- CodeCompletionContext CCContext;
- Sema *CCSema = nullptr; // Sema that created the results.
- // FIXME: Sema is scary. Can we store ASTContext and Preprocessor, instead?
+ CCTUInfo(CCAllocator), ResultsCallback(std::move(ResultsCallback)) {}
void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
CodeCompletionResult *InResults,
unsigned NumResults) override final {
- // Record the completion context.
- assert(!CCSema && "ProcessCodeCompleteResults called multiple times!");
- CCSema = &S;
- CCContext = Context;
-
+ std::vector<CodeCompletionResult> Results;
// Retain the results we might want.
for (unsigned I = 0; I < NumResults; ++I) {
auto &Result = InResults[I];
@@ -466,45 +468,51 @@
continue;
Results.push_back(Result);
}
+ if (ResultsCallback)
+ ResultsCallback(SemaCtx{S, Context, *CCAllocator, CCTUInfo}, Results);
}
CodeCompletionAllocator &getAllocator() override { return *CCAllocator; }
CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
- // Returns the filtering/sorting name for Result, which must be from Results.
- // Returned string is owned by this recorder (or the AST).
- llvm::StringRef getName(const CodeCompletionResult &Result) {
- switch (Result.Kind) {
- case CodeCompletionResult::RK_Declaration:
- if (auto *ID = Result.Declaration->getIdentifier())
- return ID->getName();
- break;
- case CodeCompletionResult::RK_Keyword:
- return Result.Keyword;
- case CodeCompletionResult::RK_Macro:
- return Result.Macro->getName();
- case CodeCompletionResult::RK_Pattern:
- return Result.Pattern->getTypedText();
- }
- auto *CCS = codeCompletionString(Result, /*IncludeBriefComments=*/false);
- return CCS->getTypedText();
- }
-
- // Build a CodeCompletion string for R, which must be from Results.
- // The CCS will be owned by this recorder.
- CodeCompletionString *codeCompletionString(const CodeCompletionResult &R,
- bool IncludeBriefComments) {
- // CodeCompletionResult doesn't seem to be const-correct. We own it, anyway.
- return const_cast<CodeCompletionResult &>(R).CreateCodeCompletionString(
- *CCSema, CCContext, *CCAllocator, CCTUInfo, IncludeBriefComments);
- }
-
private:
CodeCompleteOptions Opts;
std::shared_ptr<GlobalCodeCompletionAllocator> CCAllocator;
CodeCompletionTUInfo CCTUInfo;
+ ResultsFn ResultsCallback;
};
+// Build a CodeCompletion string for R, which must be from Results.
+// The CCS will be owned by this recorder.
+CodeCompletionString *codeCompletionString(SemaCtx &Ctx,
+ const CodeCompletionResult &R,
+ bool IncludeBriefComments) {
+ // CodeCompletionResult doesn't seem to be const-correct. We own it, anyway.
+ return const_cast<CodeCompletionResult &>(R).CreateCodeCompletionString(
+ Ctx.CCSema, Ctx.CCContext, Ctx.CCAllocator, Ctx.CCTUInfo,
+ IncludeBriefComments);
+}
+
+// Returns the filtering/sorting name for Result, which must be from Results.
+// Returned string is owned by this recorder (or the AST).
+llvm::StringRef getName(SemaCtx &Ctx, const CodeCompletionResult &Result) {
+ switch (Result.Kind) {
+ case CodeCompletionResult::RK_Declaration:
+ if (auto *ID = Result.Declaration->getIdentifier())
+ return ID->getName();
+ break;
+ case CodeCompletionResult::RK_Keyword:
+ return Result.Keyword;
+ case CodeCompletionResult::RK_Macro:
+ return Result.Macro->getName();
+ case CodeCompletionResult::RK_Pattern:
+ return Result.Pattern->getTypedText();
+ }
+ auto *CCS = codeCompletionString(Ctx, Result,
+ /*IncludeBriefComments=*/false);
+ return CCS->getTypedText();
+}
+
// Tracks a bounded number of candidates with the best scores.
class TopN {
public:
@@ -660,8 +668,7 @@
// 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) {
+ const SemaCompleteInput &Input) {
auto Tracer = llvm::make_unique<trace::Span>("Sema completion");
std::vector<const char *> ArgStrs;
for (const auto &S : Input.Command.CommandLine)
@@ -680,7 +687,7 @@
&DummyDiagsConsumer, false),
Input.VFS);
if (!CI) {
- log("Couldn't create CompilerInvocation");;
+ log("Couldn't create CompilerInvocation");
return false;
}
CI->getFrontendOpts().DisableFree = false;
@@ -729,11 +736,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();
@@ -833,35 +835,35 @@
class CodeCompleteFlow {
PathRef FileName;
const CodeCompleteOptions &Opts;
- // Sema takes ownership of Recorder. Recorder is valid until Sema cleanup.
- std::unique_ptr<CompletionRecorder> RecorderOwner;
- CompletionRecorder &Recorder;
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.
// - partial identifier and context. We need these for the index query.
CompletionList Output;
- 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");
- });
+ bool SemaCalled = false;
+ auto Recorder = llvm::make_unique<CompletionRecorder>(
+ Opts, [&](SemaCtx Ctx, llvm::ArrayRef<CodeCompletionResult> Results) {
+ assert(!SemaCalled && "Sema callback called twice");
+ SemaCalled = true;
+
+ Output = runWithSema(Ctx, Results);
+ SPAN_ATTACH(Tracer, "sema_completion_kind",
+ getCompletionKindString(Ctx.CCContext.getKind()));
+ });
+ semaCodeComplete(std::move(Recorder), Opts.getClangCompleteOpts(),
+ SemaCCInput);
+ if (!SemaCalled)
+ log("Code complete: no Sema callback, 0 results");
SPAN_ATTACH(Tracer, "sema_results", NSema);
SPAN_ATTACH(Tracer, "index_results", NIndex);
@@ -881,27 +883,28 @@
private:
// This is called by run() once Sema code completion is done, but before the
// Sema data structures are torn down. It does all the real work.
- CompletionList runWithSema() {
- Filter = FuzzyMatcher(
- Recorder.CCSema->getPreprocessor().getCodeCompletionFilter());
+ CompletionList runWithSema(SemaCtx Ctx,
+ llvm::ArrayRef<CodeCompletionResult> Results) {
+ Filter =
+ FuzzyMatcher(Ctx.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();
+ auto IndexResults = queryIndex(Ctx);
// Merge Sema and Index results, score them, and pick the winners.
- auto Top = mergeResults(Recorder.Results, IndexResults);
+ auto Top = mergeResults(Ctx, Results, IndexResults);
// Convert the results to the desired LSP structs.
CompletionList Output;
for (auto &C : Top)
- Output.items.push_back(toCompletionItem(C.first, C.second));
+ Output.items.push_back(toCompletionItem(Ctx, C.first, C.second));
Output.isIncomplete = Incomplete;
return Output;
}
- SymbolSlab queryIndex() {
- if (!Opts.Index || !allowIndex(Recorder.CCContext.getKind()))
+ SymbolSlab queryIndex(SemaCtx &Ctx) {
+ if (!Opts.Index || !allowIndex(Ctx.CCContext.getKind()))
return SymbolSlab();
trace::Span Tracer("Query index");
SPAN_ATTACH(Tracer, "limit", Opts.Limit);
@@ -912,8 +915,7 @@
if (Opts.Limit)
Req.MaxCandidateCount = Opts.Limit;
Req.Query = Filter->pattern();
- Req.Scopes =
- getQueryScopes(Recorder.CCContext, Recorder.CCSema->getSourceManager());
+ Req.Scopes = getQueryScopes(Ctx.CCContext, Ctx.CCSema.getSourceManager());
log(llvm::formatv("Code complete: fuzzyFind(\"{0}\", scopes=[{1}])",
Req.Query,
llvm::join(Req.Scopes.begin(), Req.Scopes.end(), ",")));
@@ -927,7 +929,7 @@
// Merges the Sema and Index results where possible, scores them, and
// returns the top results from best to worst.
std::vector<std::pair<CompletionCandidate, CompletionItemScores>>
- mergeResults(const std::vector<CodeCompletionResult> &SemaResults,
+ mergeResults(SemaCtx &Ctx, llvm::ArrayRef<CodeCompletionResult> SemaResults,
const SymbolSlab &IndexResults) {
trace::Span Tracer("Merge and score results");
// We only keep the best N results at any time, in "native" format.
@@ -945,24 +947,25 @@
return nullptr;
};
// Emit all Sema results, merging them with Index results if possible.
- for (auto &SemaResult : Recorder.Results)
- addCandidate(Top, &SemaResult, CorrespondingIndexResult(SemaResult));
+ for (auto &SemaResult : SemaResults)
+ addCandidate(Ctx, Top, &SemaResult, CorrespondingIndexResult(SemaResult));
// Now emit any Index-only results.
for (const auto &IndexResult : IndexResults) {
if (UsedIndexResults.count(&IndexResult))
continue;
- addCandidate(Top, /*SemaResult=*/nullptr, &IndexResult);
+ addCandidate(Ctx, Top, /*SemaResult=*/nullptr, &IndexResult);
}
return std::move(Top).items();
}
// Scores a candidate and adds it to the TopN structure.
- void addCandidate(TopN &Candidates, const CodeCompletionResult *SemaResult,
+ void addCandidate(SemaCtx &Ctx, TopN &Candidates,
+ const CodeCompletionResult *SemaResult,
const Symbol *IndexResult) {
CompletionCandidate C;
C.SemaResult = SemaResult;
C.IndexResult = IndexResult;
- C.Name = IndexResult ? IndexResult->Name : Recorder.getName(*SemaResult);
+ C.Name = IndexResult ? IndexResult->Name : getName(Ctx, *SemaResult);
CompletionItemScores Scores;
if (auto FuzzyScore = Filter->match(C.Name))
@@ -982,11 +985,12 @@
Incomplete = true;
}
- CompletionItem toCompletionItem(const CompletionCandidate &Candidate,
+ CompletionItem toCompletionItem(SemaCtx &Ctx,
+ const CompletionCandidate &Candidate,
const CompletionItemScores &Scores) {
CodeCompletionString *SemaCCS = nullptr;
if (auto *SR = Candidate.SemaResult)
- SemaCCS = Recorder.codeCompletionString(*SR, Opts.IncludeBriefComments);
+ SemaCCS = codeCompletionString(Ctx, *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
