Page MenuHomePhabricator

[lldb] Complete return types of CXXMethodDecls to prevent crashing due to covariant return types
ClosedPublic

Authored by teemperor on Jan 20 2020, 3:18 AM.

Details

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

Diff Detail

Event Timeline

teemperor created this revision.Jan 20 2020, 3:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2020, 3:18 AM
teemperor planned changes to this revision.Jan 20 2020, 4:30 AM

Actually that seems to be more complicated in case we get the method decl by asking the ASTImporter for a definition (which will skip this code).

teemperor updated this revision to Diff 240156.Jan 24 2020, 4:05 AM
  • Made test stricter.
  • Moved code to ASTImporterDelegate.
shafik accepted this revision.Jan 27 2020, 2:21 PM

LGTM

lldb/source/Symbol/ClangASTImporter.cpp
1006

I am curious if we foresee the need to do this for other cases.

This revision is now accepted and ready to land.Jan 27 2020, 2:21 PM
This revision was automatically updated to reflect the committed changes.