teemperor created this revision.
teemperor added reviewers: lhames, shafik.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.
Herald added a project: LLDB.

Currently we crash in Clang's CodeGen when we call functions with covariant 
return types with this assert:

  Assertion failed: (DD && "queried property of class with no definition"), 
function data, file clang/include/clang/AST/DeclCXX.h, line 433.

when calling `clang::CXXRecordDecl::isDerivedFrom` from the 
`ItaniumVTableBuilder`.

Clang seems to assume that the underlying record decls of covariant return 
types are already completed.
This is true during a normal Clang invocation as there the type checker will 
complete both decls when
checking if the overloaded function is valid (i.e., the return types are 
covariant).

When we minimally import our AST into the expression in LLDB we don't do this 
type checking (which
would complete the record decls) and we end up trying to access the invalid 
record decls from CodeGen
which makes us trigger the assert.

This patch just completes the underlying types of ptr/ref return types of 
virtual function so that the
underlying records are complete and we behave as Clang expects us to do.

Fixes rdar://38048657


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D73024

Files:
  lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/Makefile
  
lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
  lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/main.cpp
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -530,6 +530,22 @@
 
         m_ast_importer_sp->RequireCompleteType(copied_field_type);
       }
+      // The member might be a function with a covariant return type. In this
+      // case Clang's CodeGen expects that the underlying record of the
+      // reference/pointer is complete. This is true when parsing C++ code as
+      // there the type checking will complete those classes to verify that
+      // the user provided actually covariant return types. But when we import
+      // our own AST in LLDB we have to simulate that behavior and also
+      // complete the underlying type of the result type.
+      if (CXXMethodDecl *method_decl = dyn_cast<CXXMethodDecl>(copied_decl)) {
+        QualType return_type = method_decl->getReturnType();
+
+        if (return_type->isReferenceType())
+          m_ast_importer_sp->RequireCompleteType(
+              return_type.getNonReferenceType());
+        if (return_type->isPointerType())
+          m_ast_importer_sp->RequireCompleteType(return_type->getPointeeType());
+      }
     } else {
       SkippedDecls = true;
     }
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2397,7 +2397,7 @@
                 "Unexpected failure with msg: " + eval_result.GetError().GetCString())
 
         if result_type:
-            self.assertEqual(result_type, eval_result.GetTypeName())
+            self.assertEqual(result_type, eval_result.GetTypeName(), "Error when evaluating expr: " + expr)
 
         if result_value:
             self.assertEqual(result_value, eval_result.GetValue())
Index: lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/main.cpp
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/main.cpp
@@ -0,0 +1,37 @@
+struct OtherBase {
+  // Allow checking actual type from the test by giving
+  // this class and the subclass unique values here.
+  virtual const char *value() { return "base"; }
+};
+struct OtherDerived : public OtherBase {
+  const char *value() override { return "derived"; }
+};
+
+struct Base {
+  OtherBase other_base;
+  // Function with covariant return type that is same class.
+  virtual Base* getPtr() { return this; }
+  virtual Base& getRef() { return *this; }
+  // Function with covariant return type that is a different class.
+  virtual OtherBase* getOtherPtr() { return &other_base; }
+  virtual OtherBase& getOtherRef() { return other_base; }
+};
+
+struct Derived : public Base {
+  OtherDerived other_derived;
+  Derived* getPtr() override { return this; }
+  Derived& getRef() override { return *this; }
+  OtherDerived* getOtherPtr() override { return &other_derived; }
+  OtherDerived& getOtherRef() override { return other_derived; }
+};
+
+int main() {
+  Derived derived;
+  Base base;
+  Base *base_ptr_to_derived = &derived;
+  (void)base_ptr_to_derived->getPtr();
+  (void)base_ptr_to_derived->getRef();
+  (void)base_ptr_to_derived->getOtherPtr();
+  (void)base_ptr_to_derived->getOtherRef();
+  return 0; // break here
+}
Index: lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
@@ -0,0 +1,40 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class TestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self,"// break here", lldb.SBFileSpec("main.cpp"))
+
+        # Test covariant return types for pointers to class that contains the called function.
+        self.expect_expr("derived.getPtr()", result_type="Derived *")
+        self.expect_expr("base_ptr_to_derived->getPtr()", result_type="Base *")
+        self.expect_expr("base.getPtr()", result_type="Base *")
+        # The same tests with reference types. LLDB drops the reference when it turns the
+        # result into a SBValue so check for the the underlying type of the result.
+        self.expect_expr("derived.getRef()", result_type="Derived")
+        self.expect_expr("base_ptr_to_derived->getRef()", result_type="Base")
+        self.expect_expr("base.getRef()", result_type="Base")
+
+        # Test covariant return types for pointers to class that does *not* contain the called function.
+        self.expect_expr("derived.getOtherPtr()", result_type="OtherDerived *")
+        self.expect_expr("base_ptr_to_derived->getOtherPtr()", result_type="OtherBase *")
+        self.expect_expr("base.getOtherPtr()", result_type="OtherBase *")
+        # The same tests with reference types. LLDB drops the reference when it turns the
+        # result into a SBValue so check for the the underlying type of the result.
+        self.expect_expr("derived.getOtherRef()", result_type="OtherDerived")
+        self.expect_expr("base_ptr_to_derived->getOtherRef()", result_type="OtherBase")
+        self.expect_expr("base.getOtherRef()", result_type="OtherBase")
+
+        # Test that we call the right function and get the right value back.
+        self.expect_expr("derived.getOtherPtr()->value()", result_summary='"derived"')
+        self.expect_expr("base_ptr_to_derived->getOtherPtr()->value()", result_summary='"derived"')
+        self.expect_expr("base.getOtherPtr()->value()", result_summary='"base"')
+        self.expect_expr("derived.getOtherRef().value()", result_summary='"derived"')
+        self.expect_expr("base_ptr_to_derived->getOtherRef().value()", result_summary='"derived"')
+        self.expect_expr("base.getOtherRef().value()", result_summary='"base"')
Index: lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/Makefile
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to