[Lldb-commits] [lldb] a5fb2e3 - [lldb] Complete return types of CXXMethodDecls to prevent crashing due to covariant return types
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 29 00:09:08 PST 2020
Author: Raphael Isemann
Date: 2020-01-29T09:08:35+01:00
New Revision: a5fb2e371ec2b585ca56cbc1a116912aabe347d3
URL: https://github.com/llvm/llvm-project/commit/a5fb2e371ec2b585ca56cbc1a116912aabe347d3
DIFF: https://github.com/llvm/llvm-project/commit/a5fb2e371ec2b585ca56cbc1a116912aabe347d3.diff
LOG: [lldb] Complete return types of CXXMethodDecls to prevent crashing due to covariant return types
Summary:
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
Reviewers: lhames, shafik
Reviewed By: shafik
Subscribers: abidh, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73024
Added:
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
Modified:
lldb/source/Symbol/ClangASTImporter.cpp
Removed:
################################################################################
diff --git a/lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/Makefile b/lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/Makefile
new file mode 100644
index 000000000000..99998b20bcb0
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py b/lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
new file mode 100644
index 000000000000..86a6ddd6e47b
--- /dev/null
+++ b/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"')
diff --git a/lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/main.cpp b/lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/main.cpp
new file mode 100644
index 000000000000..2a6fc682c39a
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/covariant-return-types/main.cpp
@@ -0,0 +1,40 @@
+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"; }
+};
+
+// Those have to be globals as they would be completed if they
+// are members (which would make this test always pass).
+OtherBase other_base;
+OtherDerived other_derived;
+
+struct 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
diff erent class.
+ virtual OtherBase* getOtherPtr() { return &other_base; }
+ virtual OtherBase& getOtherRef() { return other_base; }
+};
+
+struct Derived : public Base {
+ 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
+}
diff --git a/lldb/source/Symbol/ClangASTImporter.cpp b/lldb/source/Symbol/ClangASTImporter.cpp
index 518157dc1c15..d031ab721573 100644
--- a/lldb/source/Symbol/ClangASTImporter.cpp
+++ b/lldb/source/Symbol/ClangASTImporter.cpp
@@ -976,6 +976,25 @@ void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(
}
}
+/// Takes a CXXMethodDecl and completes the return type if necessary. This
+/// is currently only necessary for virtual functions with covariant return
+/// types where Clang's CodeGen expects that the underlying records are already
+/// completed.
+static void MaybeCompleteReturnType(ClangASTImporter &importer,
+ CXXMethodDecl *to_method) {
+ if (!to_method->isVirtual())
+ return;
+ QualType return_type = to_method->getReturnType();
+ if (!return_type->isPointerType() && !return_type->isReferenceType())
+ return;
+
+ clang::RecordDecl *rd = return_type->getPointeeType()->getAsRecordDecl();
+ if (!rd)
+ return;
+
+ importer.CompleteTagDecl(rd);
+}
+
void ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from,
clang::Decl *to) {
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
@@ -1121,6 +1140,9 @@ void ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from,
}
}
}
+
+ if (clang::CXXMethodDecl *to_method = dyn_cast<CXXMethodDecl>(to))
+ MaybeCompleteReturnType(m_master, to_method);
}
clang::Decl *
More information about the lldb-commits
mailing list