https://github.com/QuietMisdreavus created https://github.com/llvm/llvm-project/pull/132297
This PR fixes two crashes in ExtractAPI that occur when decls are requested via libclang: - A null-dereference would sometimes happen in `DeclarationFragmentsBuilder::getFragmentsForClassTemplateSpecialization` when the template being processed was loaded indirectly via a typedef, with parameters filled in. The first commit loads the template parameter locations ahead of time to perform a null check before dereferencing. - An assertion (or another null-dereference) was happening in `CXXRecordDecl::bases` when processing a forward-declaration (i.e. a record without a definition). The second commit guards the use of `bases` in `ExtractAPIVisitorBase::getBases` by first checking that the decl in question has a complete definition. The added test `extract-api-cursor-cpp` adds tests for these two scenarios to protect against the crash in the future. Fixes rdar://140592475, fixes rdar://123430367 >From d83501ff207af0b7576e4297132fd8e702bae2c1 Mon Sep 17 00:00:00 2001 From: Vera Mitchell <v...@apple.com> Date: Thu, 20 Mar 2025 10:22:12 -0600 Subject: [PATCH 1/2] collect template argument locs ahead of time to null-check rdar://140592475 --- clang/lib/ExtractAPI/DeclarationFragments.cpp | 6 +++++- clang/test/Index/extract-api-cursor-cpp.cpp | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 clang/test/Index/extract-api-cursor-cpp.cpp diff --git a/clang/lib/ExtractAPI/DeclarationFragments.cpp b/clang/lib/ExtractAPI/DeclarationFragments.cpp index 66c03863293c2..480e33f607bb0 100644 --- a/clang/lib/ExtractAPI/DeclarationFragments.cpp +++ b/clang/lib/ExtractAPI/DeclarationFragments.cpp @@ -1225,6 +1225,10 @@ DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForClassTemplateSpecialization( const ClassTemplateSpecializationDecl *Decl) { DeclarationFragments Fragments; + std::optional<ArrayRef<TemplateArgumentLoc>> TemplateArgumentLocs = {}; + if (auto *TemplateArgs = Decl->getTemplateArgsAsWritten()) { + TemplateArgumentLocs = TemplateArgs->arguments(); + } return Fragments .append("template", DeclarationFragments::FragmentKind::Keyword) .appendSpace() @@ -1237,7 +1241,7 @@ DeclarationFragmentsBuilder::getFragmentsForClassTemplateSpecialization( .append("<", DeclarationFragments::FragmentKind::Text) .append(getFragmentsForTemplateArguments( Decl->getTemplateArgs().asArray(), Decl->getASTContext(), - Decl->getTemplateArgsAsWritten()->arguments())) + TemplateArgumentLocs)) .append(">", DeclarationFragments::FragmentKind::Text) .appendSemicolon(); } diff --git a/clang/test/Index/extract-api-cursor-cpp.cpp b/clang/test/Index/extract-api-cursor-cpp.cpp new file mode 100644 index 0000000000000..99570bc24462b --- /dev/null +++ b/clang/test/Index/extract-api-cursor-cpp.cpp @@ -0,0 +1,21 @@ +// Test is line- and column-sensitive. Run lines are below + +template <typename T> +class basic_vector { +public: + T x; + T y; +}; + +using my_vec = basic_vector<int>; + +class MyClass { + my_vec myVec; +}; + +// RUN: c-index-test -single-symbol-sgf-at=%s:13:13 local %s | FileCheck --check-prefix=CHECK-MYVEC %s +// CHECK-MYVEC: "parentContexts":[{"kind":"c++.class","name":"MyClass","usr":"c:@S@MyClass"},{"kind":"c++.property","name":"myVec","usr":"c:@S@MyClass@FI@myVec"}] +// CHECK-MYVEC: "identifier":{"interfaceLanguage":"c++","precise":"c:@S@MyClass@FI@myVec"} +// CHECK-MYVEC: "kind":{"displayName":"Instance Property","identifier":"c++.property"} +// CHECK-MYVEC: "title":"myVec" +// CHECK-MYVEC: "pathComponents":["MyClass","myVec"] >From 3e4e2d1b5715bf7bd2aa115757566a45920c25bf Mon Sep 17 00:00:00 2001 From: Vera Mitchell <v...@apple.com> Date: Thu, 20 Mar 2025 10:36:58 -0600 Subject: [PATCH 2/2] don't collect bases for types without definitions rdar://123430367 --- .../clang/ExtractAPI/ExtractAPIVisitor.h | 42 ++++++++++--------- clang/test/Index/extract-api-cursor-cpp.cpp | 12 ++++++ 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h index e60440e14a9fe..13e51413017d5 100644 --- a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h +++ b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h @@ -175,28 +175,30 @@ class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> { SmallVector<SymbolReference> getBases(const CXXRecordDecl *Decl) { // FIXME: store AccessSpecifier given by inheritance SmallVector<SymbolReference> Bases; - for (const auto &BaseSpecifier : Decl->bases()) { - // skip classes not inherited as public - if (BaseSpecifier.getAccessSpecifier() != AccessSpecifier::AS_public) - continue; - if (auto *BaseDecl = BaseSpecifier.getType()->getAsTagDecl()) { - Bases.emplace_back(createSymbolReferenceForDecl(*BaseDecl)); - } else { - SymbolReference BaseClass; - BaseClass.Name = API.copyString(BaseSpecifier.getType().getAsString( - Decl->getASTContext().getPrintingPolicy())); - - if (BaseSpecifier.getType().getTypePtr()->isTemplateTypeParmType()) { - if (auto *TTPTD = BaseSpecifier.getType() - ->getAs<TemplateTypeParmType>() - ->getDecl()) { - SmallString<128> USR; - index::generateUSRForDecl(TTPTD, USR); - BaseClass.USR = API.copyString(USR); - BaseClass.Source = API.copyString(getOwningModuleName(*TTPTD)); + if (Decl->isCompleteDefinition()) { + for (const auto &BaseSpecifier : Decl->bases()) { + // skip classes not inherited as public + if (BaseSpecifier.getAccessSpecifier() != AccessSpecifier::AS_public) + continue; + if (auto *BaseDecl = BaseSpecifier.getType()->getAsTagDecl()) { + Bases.emplace_back(createSymbolReferenceForDecl(*BaseDecl)); + } else { + SymbolReference BaseClass; + BaseClass.Name = API.copyString(BaseSpecifier.getType().getAsString( + Decl->getASTContext().getPrintingPolicy())); + + if (BaseSpecifier.getType().getTypePtr()->isTemplateTypeParmType()) { + if (auto *TTPTD = BaseSpecifier.getType() + ->getAs<TemplateTypeParmType>() + ->getDecl()) { + SmallString<128> USR; + index::generateUSRForDecl(TTPTD, USR); + BaseClass.USR = API.copyString(USR); + BaseClass.Source = API.copyString(getOwningModuleName(*TTPTD)); + } } + Bases.emplace_back(BaseClass); } - Bases.emplace_back(BaseClass); } } return Bases; diff --git a/clang/test/Index/extract-api-cursor-cpp.cpp b/clang/test/Index/extract-api-cursor-cpp.cpp index 99570bc24462b..c3219ddece73a 100644 --- a/clang/test/Index/extract-api-cursor-cpp.cpp +++ b/clang/test/Index/extract-api-cursor-cpp.cpp @@ -13,9 +13,21 @@ class MyClass { my_vec myVec; }; +struct OuterStruct { + struct InnerStruct; + int outer_field; +}; + // RUN: c-index-test -single-symbol-sgf-at=%s:13:13 local %s | FileCheck --check-prefix=CHECK-MYVEC %s // CHECK-MYVEC: "parentContexts":[{"kind":"c++.class","name":"MyClass","usr":"c:@S@MyClass"},{"kind":"c++.property","name":"myVec","usr":"c:@S@MyClass@FI@myVec"}] // CHECK-MYVEC: "identifier":{"interfaceLanguage":"c++","precise":"c:@S@MyClass@FI@myVec"} // CHECK-MYVEC: "kind":{"displayName":"Instance Property","identifier":"c++.property"} // CHECK-MYVEC: "title":"myVec" // CHECK-MYVEC: "pathComponents":["MyClass","myVec"] + +// RUN: c-index-test -single-symbol-sgf-at=%s:17:17 local %s | FileCheck --check-prefix=CHECK-INNER %s +// CHECK-INNER: "parentContexts":[{"kind":"c++.struct","name":"OuterStruct","usr":"c:@S@OuterStruct"},{"kind":"c++.struct","name":"InnerStruct","usr":"c:@S@OuterStruct@S@InnerStruct"}] +// CHECK-INNER: "identifier":{"interfaceLanguage":"c++","precise":"c:@S@OuterStruct@S@InnerStruct"} +// CHECK-INNER: "kind":{"displayName":"Structure","identifier":"c++.struct"} +// CHECK-INNER: "title":"InnerStruct" +// CHECK-INNER: "pathComponents":["OuterStruct","InnerStruct"] _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits