Skip to content

Commit 1dc978e

Browse files
committed
[lldb][TypeSystemClang] Avoid accessing definition if none is available
We've been hitting cases where LLDB marks a decl as `isCompleteDefinition` but no definition was allocated for it. Then when we access a an API in which Clang assumes a definition exists, and dereferences a nullptr. In the specific case that happens when we've incorrectly been keeping track of the `ImportedDecls` in `clang::ASTImporter` (which manifests as trying to `MapImported` on two different destination decls from the same source decl for the same ClangASTImporterDelegate). The more fundmental issue is that we're failing to complete the type properly. But the fix for that is still in-progress. So we're working around the crash by guarding against failed type completion. rdar://133958782
1 parent 20dbf8d commit 1dc978e

File tree

1 file changed

+21
-4
lines changed

1 file changed

+21
-4
lines changed

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
#include "TypeSystemClang.h"
1010

1111
#include "clang/AST/DeclBase.h"
12-
#include "llvm/ADT/STLForwardCompat.h"
12+
#include "clang/AST/DeclCXX.h"
13+
#include "clang/AST/ExprCXX.h"
1314
#include "llvm/Support/Casting.h"
1415
#include "llvm/Support/FormatAdapters.h"
1516
#include "llvm/Support/FormatVariadic.h"
@@ -3640,6 +3641,15 @@ bool TypeSystemClang::IsPolymorphicClass(lldb::opaque_compiler_type_t type) {
36403641
return false;
36413642
}
36423643

3644+
/// Returns 'true' if \ref decl has been allocated a definition
3645+
/// *and* the definition was marked as completed. There are currently
3646+
/// situations in which LLDB marks a definition as `isCompleteDefinition`
3647+
/// while no definition was allocated. This function guards against those
3648+
/// situations.
3649+
static bool HasCompleteDefinition(clang::CXXRecordDecl *decl) {
3650+
return decl && decl->hasDefinition() && decl->isCompleteDefinition();
3651+
}
3652+
36433653
bool TypeSystemClang::IsPossibleDynamicType(lldb::opaque_compiler_type_t type,
36443654
CompilerType *dynamic_pointee_type,
36453655
bool check_cplusplus,
@@ -3722,8 +3732,9 @@ bool TypeSystemClang::IsPossibleDynamicType(lldb::opaque_compiler_type_t type,
37223732
if (check_cplusplus) {
37233733
clang::CXXRecordDecl *cxx_record_decl =
37243734
pointee_qual_type->getAsCXXRecordDecl();
3735+
37253736
if (cxx_record_decl) {
3726-
bool is_complete = cxx_record_decl->isCompleteDefinition();
3737+
bool is_complete = HasCompleteDefinition(cxx_record_decl);
37273738

37283739
if (is_complete)
37293740
success = cxx_record_decl->isDynamicClass();
@@ -3732,7 +3743,9 @@ bool TypeSystemClang::IsPossibleDynamicType(lldb::opaque_compiler_type_t type,
37323743
if (metadata)
37333744
success = metadata->GetIsDynamicCXXType();
37343745
else {
3735-
is_complete = GetType(pointee_qual_type).GetCompleteType();
3746+
// Make sure completion has actually completed the type.
3747+
is_complete = GetType(pointee_qual_type).GetCompleteType() &&
3748+
HasCompleteDefinition(cxx_record_decl);
37363749
if (is_complete)
37373750
success = cxx_record_decl->isDynamicClass();
37383751
else
@@ -5428,8 +5441,12 @@ TypeSystemClang::GetNumChildren(lldb::opaque_compiler_type_t type,
54285441
llvm::cast<clang::RecordType>(qual_type.getTypePtr());
54295442
const clang::RecordDecl *record_decl = record_type->getDecl();
54305443
assert(record_decl);
5444+
5445+
// CXXBaseSpecifiers are stored on the definition. If we don't have
5446+
// one at this point that means we "completed" the type without actually
5447+
// having allocated a definition.
54315448
const clang::CXXRecordDecl *cxx_record_decl =
5432-
llvm::dyn_cast<clang::CXXRecordDecl>(record_decl);
5449+
llvm::dyn_cast<clang::CXXRecordDecl>(record_decl)->getDefinition();
54335450
if (cxx_record_decl) {
54345451
if (omit_empty_base_classes) {
54355452
// Check each base classes to see if it or any of its base classes

0 commit comments

Comments
 (0)