Skip to content

Conversation

@5chmidti
Copy link
Contributor

In the added testcase, which is invalid code, the result of getDecl() called on a TemplateTypeParmTypeLoc was
a nullptr. However, IndexingContext::handleReference expects the parameter D to not be a nullptr and
passed it to isa<> without checking it. Similarly MakeCursorTypeRef expects D to not be a nullptr as well.

Fixes: clangd/clangd#2016, #89389

…ullptr` in `TypeIndexer` and `CursorVisitor`

In the added testcase, which is invalid code, the result of `getDecl()` called on a `TemplateTypeParmTypeLoc` was
a `nullptr`. However, `IndexingContext::handleReference` expects the parameter `D` to not be a `nullptr` and
passed it to `isa<>` without checking it. Similarly `MakeCursorTypeRef` expects `D` to not be a `nullptr` as well.

Fixes: clangd/clangd#2016, llvm#89389
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Apr 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 19, 2024

@llvm/pr-subscribers-clang

Author: Julian Schmidt (5chmidti)

Changes

In the added testcase, which is invalid code, the result of getDecl() called on a TemplateTypeParmTypeLoc was
a nullptr. However, IndexingContext::handleReference expects the parameter D to not be a nullptr and
passed it to isa&lt;&gt; without checking it. Similarly MakeCursorTypeRef expects D to not be a nullptr as well.

Fixes: clangd/clangd#2016, #89389


Full diff: https://github.com/llvm/llvm-project/pull/89392.diff

4 Files Affected:

  • (modified) clang/lib/Index/IndexTypeSourceInfo.cpp (+3)
  • (added) clang/test/Index/gh89389.cpp (+16)
  • (modified) clang/tools/libclang/CIndex.cpp (+6-2)
  • (modified) clang/unittests/Index/IndexTests.cpp (+27)
diff --git a/clang/lib/Index/IndexTypeSourceInfo.cpp b/clang/lib/Index/IndexTypeSourceInfo.cpp
index b986ccde574525..58ad0799062ca3 100644
--- a/clang/lib/Index/IndexTypeSourceInfo.cpp
+++ b/clang/lib/Index/IndexTypeSourceInfo.cpp
@@ -52,6 +52,9 @@ class TypeIndexer : public RecursiveASTVisitor<TypeIndexer> {
   bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TTPL) {
     SourceLocation Loc = TTPL.getNameLoc();
     TemplateTypeParmDecl *TTPD = TTPL.getDecl();
+    if (!TTPD)
+      return false;
+
     return IndexCtx.handleReference(TTPD, Loc, Parent, ParentDC,
                                     SymbolRoleSet());
   }
diff --git a/clang/test/Index/gh89389.cpp b/clang/test/Index/gh89389.cpp
new file mode 100644
index 00000000000000..0458d0a64083d8
--- /dev/null
+++ b/clang/test/Index/gh89389.cpp
@@ -0,0 +1,16 @@
+// RUN: c-index-test -test-load-source all %s -std=gnu++20 -fno-delayed-template-parsing
+
+namespace test18 {
+template<typename T>
+concept False = false;
+
+template <typename T> struct Foo { T t; };
+
+template<typename T> requires False<T>
+Foo(T) -> Foo<int>;
+
+template <typename U>
+using Bar = Foo<U>;
+
+Bar s = {1};
+} // namespace test18
diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp
index cbc1d85bb33dfc..8fd723a90e5565 100644
--- a/clang/tools/libclang/CIndex.cpp
+++ b/clang/tools/libclang/CIndex.cpp
@@ -1693,12 +1693,16 @@ bool CursorVisitor::VisitTagTypeLoc(TagTypeLoc TL) {
 }
 
 bool CursorVisitor::VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TL) {
-  if (const auto *TC = TL.getDecl()->getTypeConstraint()) {
+  TemplateTypeParmDecl *D = TL.getDecl();
+  if (!D)
+    return true;
+
+  if (const auto *TC = D->getTypeConstraint()) {
     if (VisitTypeConstraint(*TC))
       return true;
   }
 
-  return Visit(MakeCursorTypeRef(TL.getDecl(), TL.getNameLoc(), TU));
+  return Visit(MakeCursorTypeRef(D, TL.getNameLoc(), TU));
 }
 
 bool CursorVisitor::VisitObjCInterfaceTypeLoc(ObjCInterfaceTypeLoc TL) {
diff --git a/clang/unittests/Index/IndexTests.cpp b/clang/unittests/Index/IndexTests.cpp
index 8e9a1c6bf88245..0db8a4b98c2025 100644
--- a/clang/unittests/Index/IndexTests.cpp
+++ b/clang/unittests/Index/IndexTests.cpp
@@ -453,6 +453,33 @@ TEST(IndexTest, ReadWriteRoles) {
                            WrittenAt(Position(6, 17))))));
 }
 
+TEST(IndexTest, gh89389) {
+  std::string Code = R"cpp(
+    namespace test18 {
+    template<typename T>
+    concept False = false;
+
+    template <typename T> struct Foo { T t; };
+
+    template<typename T> requires False<T>
+    Foo(T) -> Foo<int>;
+
+    template <typename U>
+    using Bar = Foo<U>;
+
+    Bar s = {1};
+    } // namespace test18
+  )cpp";
+  auto Index = std::make_shared<Indexer>();
+  IndexingOptions Opts;
+  Opts.IndexTemplateParameters = true;
+
+  // This test case is invalid code, so the expected return value is `false`.
+  // What is being tested is that there is no crash.
+  EXPECT_FALSE(tooling::runToolOnCodeWithArgs(
+      std::make_unique<IndexAction>(Index, Opts), Code, {"-std=c++20"}));
+}
+
 } // namespace
 } // namespace index
 } // namespace clang

@5chmidti 5chmidti changed the title [clang][Index] check TemplateTypeParmTypeLoc ::getDecl() against nullptr in TypeIndexer and CursorVisitor [clang][Index] check TemplateTypeParmTypeLoc::getDecl() against nullptr in TypeIndexer and CursorVisitor Apr 19, 2024
@5chmidti
Copy link
Contributor Author

CC @hokein

@5chmidti
Copy link
Contributor Author

Fixed by c7bfc41

@5chmidti 5chmidti closed this Oct 27, 2024
@5chmidti 5chmidti deleted the clang_index_isa_nullptr branch October 27, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clangd index crashes on SemaCXX/cxx20-ctad-type-alias.cpp

2 participants