[Lldb-commits] [lldb] 7b00eeb - [lldb] Fix another crash in covariant type handling

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-30T16:00:21+02:00
New Revision: 7b00eeb53de06c7979fd2a88436c3387d0492ee8

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

LOG: [lldb] Fix another crash in covariant type handling

Summary:
D73024 seems to have fixed one set crash, but it introduced another.
Namely, if a class contains a covariant method returning itself, the
logic in MaybeCompleteReturnType could cause us to attempt a recursive
import, which would result in an assertion failure in
clang::DeclContext::removeDecl.

For some reason, this only manifested itself if the class contained at
least two member variables, and the class itself was imported as a
result of a recursive covariant import.

This patch fixes the crash by not attempting to import classes which are
already completed in MaybeCompleteReturnType. However, it's not clear to
me if this is the right fix, or if this should be handled automatically
by functions lower in the stack.

Reviewers: teemperor, shafik

Subscribers: lldb-commits

Tags: #lldb

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

Added: 
    

Modified: 
    lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
    lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
    lldb/test/API/lang/cpp/covariant-return-types/main.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index 479cb64deefd..4b3e237dc62c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -997,6 +997,8 @@ static void MaybeCompleteReturnType(ClangASTImporter &importer,
   clang::RecordDecl *rd = return_type->getPointeeType()->getAsRecordDecl();
   if (!rd)
     return;
+  if (rd->getDefinition())
+    return;
 
   importer.CompleteTagDecl(rd);
 }

diff  --git a/lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py b/lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
index 86a6ddd6e47b..6f2b3eafd2e9 100644
--- a/lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
+++ b/lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
@@ -38,3 +38,5 @@ def test(self):
         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"')
+
+        self.expect_expr("referencing_derived.getOther()->get()->a", result_value='42')

diff  --git a/lldb/test/API/lang/cpp/covariant-return-types/main.cpp b/lldb/test/API/lang/cpp/covariant-return-types/main.cpp
index 2a6fc682c39a..ea05043d6cf9 100644
--- a/lldb/test/API/lang/cpp/covariant-return-types/main.cpp
+++ b/lldb/test/API/lang/cpp/covariant-return-types/main.cpp
@@ -28,6 +28,23 @@ struct Derived : public Base {
   OtherDerived& getOtherRef() override { return other_derived; }
 };
 
+// A regression test for a class with at least two members containing a
+// covariant function, which is referenced through another covariant function.
+struct BaseWithMembers {
+  int a = 42;
+  int b = 47;
+  virtual BaseWithMembers *get() { return this; }
+};
+struct DerivedWithMembers: BaseWithMembers {
+  DerivedWithMembers *get() override { return this; }
+};
+struct ReferencingBase {
+  virtual BaseWithMembers *getOther() { return new BaseWithMembers(); }
+};
+struct ReferencingDerived: ReferencingBase {
+  DerivedWithMembers *getOther() { return new DerivedWithMembers(); }
+};
+
 int main() {
   Derived derived;
   Base base;
@@ -36,5 +53,7 @@ int main() {
   (void)base_ptr_to_derived->getRef();
   (void)base_ptr_to_derived->getOtherPtr();
   (void)base_ptr_to_derived->getOtherRef();
+
+  ReferencingDerived referencing_derived;
   return 0; // break here
 }