[clang-tools-extra] fcf0764 - [AST] Fix an assertion violation in FieldDecl::getParent.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-05-19T15:35:04+02:00
New Revision: fcf0764998b45279cfdf4039c69aec2cd09051a5

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

LOG: [AST] Fix an assertion violation in FieldDecl::getParent.

Summary:
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);

Fixes https://github.com/clangd/clangd/issues/369

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: rsmith, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/Hover.cpp
    clang-tools-extra/clangd/unittests/HoverTests.cpp
    clang/include/clang/AST/Decl.h

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 799d37626489..3d0430b57931 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -662,7 +662,9 @@ void addLayoutInfo(const NamedDecl &ND, HoverInfo &HI) {
   }
 
   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())) {

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 04842ba8d141..e5ff0ee364d8 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1737,6 +1737,19 @@ TEST(Hover, All) {
             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 @@ TEST(Hover, All) {
     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 
diff erent depending on the target triplet, we chose a
     // fixed one to make sure tests passes on 
diff erent platform.

diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 7db74e0803ce..d7136a4cd420 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2920,12 +2920,15 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
 
   /// 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;