[clang] 721476e - [clang] Fix a crash during code completion

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2021-06-07T13:29:58+02:00
New Revision: 721476e6b2119a93033903109b54f429b6e8c91b

URL: https://github.com/llvm/llvm-project/commit/721476e6b2119a93033903109b54f429b6e8c91b
DIFF: https://github.com/llvm/llvm-project/commit/721476e6b2119a93033903109b54f429b6e8c91b.diff

LOG: [clang] Fix a crash during code completion

During code completion, lookupInDeclContext() calls
CodeCompletionDeclConsumer::FoundDecl(),which can mutate StoredDeclsMap,
over which lookupInDeclContext() iterates. This can lead to invalidation
of iterators and an assert()-crash.

Example code where this happens:
 #include <list>
 int main() {
   std::list<int>;
   std::^
 }
with code completion on ^ with -std=c++20.

I do not have a repro case that does not need standard library.

This fix stores pointers to NamedDecls in a temporary vector, then
visits them outside of the main loop, when StoredDeclsMap iterators are
gone.

Differential Revision: https://reviews.llvm.org/D103472

Added: 
    

Modified: 
    clang/lib/Sema/SemaLookup.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index db6a01543d76..4b3d7de04bf7 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -3835,6 +3835,7 @@ class LookupVisibleHelper {
     if (CXXRecordDecl *Class = dyn_cast<CXXRecordDecl>(Ctx))
       Result.getSema().ForceDeclarationOfImplicitMembers(Class);
 
+    llvm::SmallVector<NamedDecl *, 4> DeclsToVisit;
     // We sometimes skip loading namespace-level results (they tend to be huge).
     bool Load = LoadExternal ||
                 !(isa<TranslationUnitDecl>(Ctx) || isa<NamespaceDecl>(Ctx));
@@ -3844,12 +3845,21 @@ class LookupVisibleHelper {
               : Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
       for (auto *D : R) {
         if (auto *ND = Result.getAcceptableDecl(D)) {
-          Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
-          Visited.add(ND);
+          // Rather than visit immediatelly, we put ND into a vector and visit
+          // all decls, in order, outside of this loop. The reason is that
+          // Consumer.FoundDecl() may invalidate the iterators used in the two
+          // loops above.
+          DeclsToVisit.push_back(ND);
         }
       }
     }
 
+    for (auto *ND : DeclsToVisit) {
+      Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
+      Visited.add(ND);
+    }
+    DeclsToVisit.clear();
+
     // Traverse using directives for qualified name lookup.
     if (QualifiedNameLookup) {
       ShadowContextRAII Shadow(Visited);