Skip to content

Commit 7b00eeb

Browse files
committed
[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
1 parent 135709a commit 7b00eeb

File tree

3 files changed

+23
-0
lines changed

3 files changed

+23
-0
lines changed

lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -997,6 +997,8 @@ static void MaybeCompleteReturnType(ClangASTImporter &importer,
997997
clang::RecordDecl *rd = return_type->getPointeeType()->getAsRecordDecl();
998998
if (!rd)
999999
return;
1000+
if (rd->getDefinition())
1001+
return;
10001002

10011003
importer.CompleteTagDecl(rd);
10021004
}

lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,5 @@ def test(self):
3838
self.expect_expr("derived.getOtherRef().value()", result_summary='"derived"')
3939
self.expect_expr("base_ptr_to_derived->getOtherRef().value()", result_summary='"derived"')
4040
self.expect_expr("base.getOtherRef().value()", result_summary='"base"')
41+
42+
self.expect_expr("referencing_derived.getOther()->get()->a", result_value='42')

lldb/test/API/lang/cpp/covariant-return-types/main.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,23 @@ struct Derived : public Base {
2828
OtherDerived& getOtherRef() override { return other_derived; }
2929
};
3030

31+
// A regression test for a class with at least two members containing a
32+
// covariant function, which is referenced through another covariant function.
33+
struct BaseWithMembers {
34+
int a = 42;
35+
int b = 47;
36+
virtual BaseWithMembers *get() { return this; }
37+
};
38+
struct DerivedWithMembers: BaseWithMembers {
39+
DerivedWithMembers *get() override { return this; }
40+
};
41+
struct ReferencingBase {
42+
virtual BaseWithMembers *getOther() { return new BaseWithMembers(); }
43+
};
44+
struct ReferencingDerived: ReferencingBase {
45+
DerivedWithMembers *getOther() { return new DerivedWithMembers(); }
46+
};
47+
3148
int main() {
3249
Derived derived;
3350
Base base;
@@ -36,5 +53,7 @@ int main() {
3653
(void)base_ptr_to_derived->getRef();
3754
(void)base_ptr_to_derived->getOtherPtr();
3855
(void)base_ptr_to_derived->getOtherRef();
56+
57+
ReferencingDerived referencing_derived;
3958
return 0; // break here
4059
}

0 commit comments

Comments
 (0)