hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
FieldDecl::getParent assumes that the FiledDecl::getDeclContext returns a
RecordDecl, this is true for C/C++, but not for ObjCIvarDecl:
The Decls hierarchy is like following
FieldDecl <-- ObjCIvarDecl
DeclContext <-- ObjCContainerDecl <-- ObjCInterfaceDecl
^
|----- TagDecl <-- RecordDecl
calling getParent() on ObjCIvarDecl will:
1. invoke getDeclContext(), which returns a DeclContext*, which points to an
ObjCInterfaceDecl;
2. then downcast the "DeclContext" pointer to a RecordDecl*, and we will hit
the "is_a<RecordDecl>" assertion in llvm::cast (undefined behavior
in release build without assertion enabled);
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79627
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang/include/clang/AST/Decl.h
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2920,12 +2920,15 @@
/// Returns the parent of this field declaration, which
/// is the struct in which this field is defined.
+ ///
+ /// Returns null if this is not a normal class/struct field declaration, e.g.
+ /// ObjCAtDefsFieldDecl, ObjCIvarDecl.
const RecordDecl *getParent() const {
- return cast<RecordDecl>(getDeclContext());
+ return dyn_cast<RecordDecl>(getDeclContext());
}
RecordDecl *getParent() {
- return cast<RecordDecl>(getDeclContext());
+ return dyn_cast<RecordDecl>(getDeclContext());
}
SourceRange getSourceRange() const override LLVM_READONLY;
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1737,6 +1737,19 @@
HI.Definition = "template <> void foo<int>(const int &)";
HI.NamespaceScope = "";
}},
+ {
+ R"cpp(// should not crash
+ @interface ObjC {
+ char [[da^ta]];
+ }@end
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.Name = "data";
+ HI.Type = "char";
+ HI.Kind = index::SymbolKind::Field;
+ HI.NamespaceScope = "ObjC::"; // FIXME: fix it
+ HI.Definition = "char data";
+ }},
};
// Create a tiny index, so tests above can verify documentation is fetched.
@@ -1753,6 +1766,8 @@
Annotations T(Case.Code);
TestTU TU = TestTU::withCode(T.code());
TU.ExtraArgs.push_back("-std=c++17");
+ TU.ExtraArgs.push_back("-xobjective-c++");
+
TU.ExtraArgs.push_back("-Wno-gnu-designator");
// Types might be different depending on the target triplet, we chose a
// fixed one to make sure tests passes on different platform.
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -662,7 +662,9 @@
}
if (const auto *FD = llvm::dyn_cast<FieldDecl>(&ND)) {
- const auto *Record = FD->getParent()->getDefinition();
+ const auto *Record = FD->getParent();
+ if (Record)
+ Record = Record->getDefinition();
if (Record && !Record->isDependentType()) {
uint64_t OffsetBits = Ctx.getFieldOffset(FD);
if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2920,12 +2920,15 @@
/// Returns the parent of this field declaration, which
/// is the struct in which this field is defined.
+ ///
+ /// Returns null if this is not a normal class/struct field declaration, e.g.
+ /// ObjCAtDefsFieldDecl, ObjCIvarDecl.
const RecordDecl *getParent() const {
- return cast<RecordDecl>(getDeclContext());
+ return dyn_cast<RecordDecl>(getDeclContext());
}
RecordDecl *getParent() {
- return cast<RecordDecl>(getDeclContext());
+ return dyn_cast<RecordDecl>(getDeclContext());
}
SourceRange getSourceRange() const override LLVM_READONLY;
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1737,6 +1737,19 @@
HI.Definition = "template <> void foo<int>(const int &)";
HI.NamespaceScope = "";
}},
+ {
+ R"cpp(// should not crash
+ @interface ObjC {
+ char [[da^ta]];
+ }@end
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.Name = "data";
+ HI.Type = "char";
+ HI.Kind = index::SymbolKind::Field;
+ HI.NamespaceScope = "ObjC::"; // FIXME: fix it
+ HI.Definition = "char data";
+ }},
};
// Create a tiny index, so tests above can verify documentation is fetched.
@@ -1753,6 +1766,8 @@
Annotations T(Case.Code);
TestTU TU = TestTU::withCode(T.code());
TU.ExtraArgs.push_back("-std=c++17");
+ TU.ExtraArgs.push_back("-xobjective-c++");
+
TU.ExtraArgs.push_back("-Wno-gnu-designator");
// Types might be different depending on the target triplet, we chose a
// fixed one to make sure tests passes on different platform.
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -662,7 +662,9 @@
}
if (const auto *FD = llvm::dyn_cast<FieldDecl>(&ND)) {
- const auto *Record = FD->getParent()->getDefinition();
+ const auto *Record = FD->getParent();
+ if (Record)
+ Record = Record->getDefinition();
if (Record && !Record->isDependentType()) {
uint64_t OffsetBits = Ctx.getFieldOffset(FD);
if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
