Made the ClangASTImporter into a shared pointer, eliminating a race condition.
It used to be a unique pointer, and there could be a case where ClangASTSource
held onto a copy of the pointer but Target::Destroy destroyed the unique pointer
in the mean time.
I also ensured that there is a validity check on the target (which confirms that
a ClangASTImporter can be generated) before the target's shared pointer is
copied into ClangASTSource.
This race condition caused a crash if Target::Destroy was called and then later
the target objecct was deleted.
llvm-svn: 252665
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 6708c6c..995da9c 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1287,7 +1287,7 @@
ClangASTContext *
GetScratchClangASTContext(bool create_on_demand=true);
- ClangASTImporter *
+ lldb::ClangASTImporterSP
GetClangASTImporter();
//----------------------------------------------------------------------
@@ -1568,7 +1568,7 @@
typedef std::map<lldb::LanguageType, lldb::REPLSP> REPLMap;
REPLMap m_repl_map;
- lldb::ClangASTImporterUP m_ast_importer_ap;
+ lldb::ClangASTImporterSP m_ast_importer_sp;
lldb::ClangModulesDeclVendorUP m_clang_modules_decl_vendor_ap;
lldb::SourceManagerUP m_source_manager_ap;
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index 5faaab4..e494b29 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -310,7 +310,7 @@
typedef std::shared_ptr<lldb_private::BreakpointResolver> BreakpointResolverSP;
typedef std::shared_ptr<lldb_private::Broadcaster> BroadcasterSP;
typedef std::unique_ptr<lldb_private::ClangASTContext> ClangASTContextUP;
- typedef std::unique_ptr<lldb_private::ClangASTImporter> ClangASTImporterUP;
+ typedef std::shared_ptr<lldb_private::ClangASTImporter> ClangASTImporterSP;
typedef std::unique_ptr<lldb_private::ClangModulesDeclVendor> ClangModulesDeclVendorUP;
typedef std::unique_ptr<lldb_private::ClangPersistentVariables> ClangPersistentVariablesUP;
typedef std::shared_ptr<lldb_private::UserExpression> UserExpressionSP;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 82ffdf1..d29cb3e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -57,7 +57,7 @@
ClangASTSource::~ClangASTSource()
{
- m_ast_importer->ForgetDestination(m_ast_context);
+ m_ast_importer_sp->ForgetDestination(m_ast_context);
// We are in the process of destruction, don't create clang ast context on demand
// by passing false to Target::GetScratchClangASTContext(create_on_demand).
@@ -72,7 +72,7 @@
return;
if (m_ast_context != scratch_ast_context)
- m_ast_importer->ForgetSource(scratch_ast_context, m_ast_context);
+ m_ast_importer_sp->ForgetSource(scratch_ast_context, m_ast_context);
}
void
@@ -221,7 +221,7 @@
m_active_lexical_decls.insert(tag_decl);
ScopedLexicalDeclEraser eraser(m_active_lexical_decls, tag_decl);
- if (!m_ast_importer->CompleteTagDecl (tag_decl))
+ if (!m_ast_importer_sp->CompleteTagDecl (tag_decl))
{
// We couldn't complete the type. Maybe there's a definition
// somewhere else that can be completed.
@@ -235,7 +235,7 @@
if (const NamespaceDecl *namespace_context = dyn_cast<NamespaceDecl>(decl_ctx))
{
- ClangASTImporter::NamespaceMapSP namespace_map = m_ast_importer->GetNamespaceMap(namespace_context);
+ ClangASTImporter::NamespaceMapSP namespace_map = m_ast_importer_sp->GetNamespaceMap(namespace_context);
if (log && log->GetVerbose())
log->Printf(" CTD[%u] Inspecting namespace map %p (%d entries)",
@@ -283,7 +283,7 @@
TagDecl *candidate_tag_decl = const_cast<TagDecl*>(tag_type->getDecl());
- if (m_ast_importer->CompleteTagDeclWithOrigin (tag_decl, candidate_tag_decl))
+ if (m_ast_importer_sp->CompleteTagDeclWithOrigin (tag_decl, candidate_tag_decl))
found = true;
}
}
@@ -322,7 +322,7 @@
TagDecl *candidate_tag_decl = const_cast<TagDecl*>(tag_type->getDecl());
- if (m_ast_importer->CompleteTagDeclWithOrigin (tag_decl, candidate_tag_decl))
+ if (m_ast_importer_sp->CompleteTagDeclWithOrigin (tag_decl, candidate_tag_decl))
found = true;
}
}
@@ -354,7 +354,7 @@
Decl *original_decl = NULL;
ASTContext *original_ctx = NULL;
- if (m_ast_importer->ResolveDeclOrigin(interface_decl, &original_decl, &original_ctx))
+ if (m_ast_importer_sp->ResolveDeclOrigin(interface_decl, &original_decl, &original_ctx))
{
if (ObjCInterfaceDecl *original_iface_decl = dyn_cast<ObjCInterfaceDecl>(original_decl))
{
@@ -362,12 +362,12 @@
if (complete_iface_decl && (complete_iface_decl != original_iface_decl))
{
- m_ast_importer->SetDeclOrigin(interface_decl, original_iface_decl);
+ m_ast_importer_sp->SetDeclOrigin(interface_decl, original_iface_decl);
}
}
}
- m_ast_importer->CompleteObjCInterfaceDecl (interface_decl);
+ m_ast_importer_sp->CompleteObjCInterfaceDecl (interface_decl);
if (interface_decl->getSuperClass() &&
interface_decl->getSuperClass() != interface_decl)
@@ -462,7 +462,7 @@
Decl *original_decl = NULL;
ASTContext *original_ctx = NULL;
- if (!m_ast_importer->ResolveDeclOrigin(context_decl, &original_decl, &original_ctx))
+ if (!m_ast_importer_sp->ResolveDeclOrigin(context_decl, &original_decl, &original_ctx))
return;
if (log)
@@ -482,7 +482,7 @@
original_decl = complete_iface_decl;
original_ctx = &complete_iface_decl->getASTContext();
- m_ast_importer->SetDeclOrigin(context_decl, original_iface_decl);
+ m_ast_importer_sp->SetDeclOrigin(context_decl, original_iface_decl);
}
}
@@ -516,7 +516,7 @@
log->Printf(" FELD[%d] Adding lexical %sDecl %s", current_id, decl->getDeclKindName(), ast_dumper.GetCString());
}
- Decl *copied_decl = m_ast_importer->CopyDecl(m_ast_context, original_ctx, decl);
+ Decl *copied_decl = m_ast_importer_sp->CopyDecl(m_ast_context, original_ctx, decl);
if (!copied_decl)
continue;
@@ -525,7 +525,7 @@
{
QualType copied_field_type = copied_field->getType();
- m_ast_importer->RequireCompleteType(copied_field_type);
+ m_ast_importer_sp->RequireCompleteType(copied_field_type);
}
DeclContext *decl_context_non_const = const_cast<DeclContext *>(decl_context);
@@ -581,7 +581,7 @@
if (const NamespaceDecl *namespace_context = dyn_cast<NamespaceDecl>(context.m_decl_context))
{
- ClangASTImporter::NamespaceMapSP namespace_map = m_ast_importer->GetNamespaceMap(namespace_context);
+ ClangASTImporter::NamespaceMapSP namespace_map = m_ast_importer_sp->GetNamespaceMap(namespace_context);
if (log && log->GetVerbose())
log->Printf(" CAS::FEVD[%u] Inspecting namespace map %p (%d entries)",
@@ -813,7 +813,7 @@
llvm::isa<clang::ObjCContainerDecl>(decl_from_modules) ||
llvm::isa<clang::EnumConstantDecl>(decl_from_modules))
{
- clang::Decl *copied_decl = m_ast_importer->CopyDecl(m_ast_context, &decl_from_modules->getASTContext(), decl_from_modules);
+ clang::Decl *copied_decl = m_ast_importer_sp->CopyDecl(m_ast_context, &decl_from_modules->getASTContext(), decl_from_modules);
clang::NamedDecl *copied_named_decl = copied_decl ? dyn_cast<clang::NamedDecl>(copied_decl) : nullptr;
if (!copied_named_decl)
@@ -871,7 +871,7 @@
name.GetCString());
}
- clang::Decl *copied_decl = m_ast_importer->CopyDecl(m_ast_context, &decls[0]->getASTContext(), decls[0]);
+ clang::Decl *copied_decl = m_ast_importer_sp->CopyDecl(m_ast_context, &decls[0]->getASTContext(), decls[0]);
clang::NamedDecl *copied_named_decl = copied_decl ? dyn_cast<clang::NamedDecl>(copied_decl) : nullptr;
if (!copied_named_decl)
@@ -1066,7 +1066,7 @@
Decl *original_decl = NULL;
ASTContext *original_ctx = NULL;
- m_ast_importer->ResolveDeclOrigin(interface_decl, &original_decl, &original_ctx);
+ m_ast_importer_sp->ResolveDeclOrigin(interface_decl, &original_decl, &original_ctx);
if (!original_decl)
break;
@@ -1077,7 +1077,7 @@
context,
original_interface_decl,
m_ast_context,
- m_ast_importer,
+ m_ast_importer_sp.get(),
"at origin"))
return; // found it, no need to look any further
} while (0);
@@ -1224,7 +1224,7 @@
if (found_interface_decl->getName() == interface_decl->getName())
{
- Decl *copied_decl = m_ast_importer->CopyDecl(m_ast_context, &method_decl->getASTContext(), method_decl);
+ Decl *copied_decl = m_ast_importer_sp->CopyDecl(m_ast_context, &method_decl->getASTContext(), method_decl);
if (!copied_decl)
continue;
@@ -1272,7 +1272,7 @@
context,
complete_interface_decl,
m_ast_context,
- m_ast_importer,
+ m_ast_importer_sp.get(),
"in debug info");
return;
@@ -1305,7 +1305,7 @@
context,
interface_decl_from_modules,
m_ast_context,
- m_ast_importer,
+ m_ast_importer_sp.get(),
"in modules"))
return;
}
@@ -1351,7 +1351,7 @@
context,
runtime_interface_decl,
m_ast_context,
- m_ast_importer,
+ m_ast_importer_sp.get(),
"in runtime");
}
while(0);
@@ -1423,7 +1423,7 @@
unsigned int current_id = invocation_id++;
DeclFromParser<const ObjCInterfaceDecl> parser_iface_decl(cast<ObjCInterfaceDecl>(context.m_decl_context));
- DeclFromUser<const ObjCInterfaceDecl> origin_iface_decl(parser_iface_decl.GetOrigin(m_ast_importer));
+ DeclFromUser<const ObjCInterfaceDecl> origin_iface_decl(parser_iface_decl.GetOrigin(m_ast_importer_sp.get()));
ConstString class_name(parser_iface_decl->getNameAsString().c_str());
@@ -1436,7 +1436,7 @@
if (FindObjCPropertyAndIvarDeclsWithOrigin(current_id,
context,
*m_ast_context,
- m_ast_importer,
+ m_ast_importer_sp.get(),
origin_iface_decl))
return;
@@ -1471,7 +1471,7 @@
FindObjCPropertyAndIvarDeclsWithOrigin(current_id,
context,
*m_ast_context,
- m_ast_importer,
+ m_ast_importer_sp.get(),
complete_iface_decl);
return;
@@ -1511,7 +1511,7 @@
if (FindObjCPropertyAndIvarDeclsWithOrigin(current_id,
context,
*m_ast_context,
- m_ast_importer,
+ m_ast_importer_sp.get(),
interface_decl_from_modules))
return;
}
@@ -1561,7 +1561,7 @@
if (FindObjCPropertyAndIvarDeclsWithOrigin(current_id,
context,
*m_ast_context,
- m_ast_importer,
+ m_ast_importer_sp.get(),
interface_decl_from_runtime))
return;
}
@@ -1664,7 +1664,7 @@
record->getNameAsString().c_str());
DeclFromParser <const RecordDecl> parser_record(record);
- DeclFromUser <const RecordDecl> origin_record(parser_record.GetOrigin(m_ast_importer));
+ DeclFromUser <const RecordDecl> origin_record(parser_record.GetOrigin(m_ast_importer_sp.get()));
if (origin_record.IsInvalid())
return false;
@@ -1706,9 +1706,9 @@
return false;
}
- if (!ImportOffsetMap(field_offsets, origin_field_offsets, m_ast_importer, parser_ast_context) ||
- !ImportOffsetMap(base_offsets, origin_base_offsets, m_ast_importer, parser_ast_context) ||
- !ImportOffsetMap(virtual_base_offsets, origin_virtual_base_offsets, m_ast_importer, parser_ast_context))
+ if (!ImportOffsetMap(field_offsets, origin_field_offsets, m_ast_importer_sp.get(), parser_ast_context) ||
+ !ImportOffsetMap(base_offsets, origin_base_offsets, m_ast_importer_sp.get(), parser_ast_context) ||
+ !ImportOffsetMap(virtual_base_offsets, origin_virtual_base_offsets, m_ast_importer_sp.get(), parser_ast_context))
return false;
size = record_layout.getSize().getQuantity() * m_ast_context->getCharWidth();
@@ -1870,7 +1870,7 @@
if (!src_namespace_decl)
return nullptr;
- Decl *copied_decl = m_ast_importer->CopyDecl(m_ast_context, src_ast, src_namespace_decl);
+ Decl *copied_decl = m_ast_importer_sp->CopyDecl(m_ast_context, src_ast, src_namespace_decl);
if (!copied_decl)
return nullptr;
@@ -1882,7 +1882,7 @@
context.m_decls.push_back(copied_namespace_decl);
- m_ast_importer->RegisterNamespaceMap(copied_namespace_decl, namespace_decls);
+ m_ast_importer_sp->RegisterNamespaceMap(copied_namespace_decl, namespace_decls);
return dyn_cast<NamespaceDecl>(copied_decl);
}
@@ -1898,7 +1898,7 @@
SetImportInProgress(true);
- QualType copied_qual_type = m_ast_importer->CopyType (m_ast_context, src_ast->getASTContext(), ClangASTContext::GetQualType(src_type));
+ QualType copied_qual_type = m_ast_importer_sp->CopyType (m_ast_context, src_ast->getASTContext(), ClangASTContext::GetQualType(src_type));
SetImportInProgress(false);
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
index 9f4d238..bb63847 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -54,7 +54,7 @@
m_active_lexical_decls (),
m_active_lookups ()
{
- m_ast_importer = m_target->GetClangASTImporter();
+ m_ast_importer_sp = m_target->GetClangASTImporter();
}
//------------------------------------------------------------------
@@ -77,7 +77,7 @@
void InstallASTContext (clang::ASTContext *ast_context)
{
m_ast_context = ast_context;
- m_ast_importer->InstallMapCompleter(ast_context, *this);
+ m_ast_importer_sp->InstallMapCompleter(ast_context, *this);
}
//
@@ -405,7 +405,7 @@
const lldb::TargetSP m_target; ///< The target to use in finding variables and types.
clang::ASTContext *m_ast_context; ///< The AST context requests are coming in for.
- ClangASTImporter *m_ast_importer; ///< The target's AST importer.
+ lldb::ClangASTImporterSP m_ast_importer_sp; ///< The target's AST importer.
std::set<const clang::Decl *> m_active_lexical_decls;
std::set<const char *> m_active_lookups;
};
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index a2e8d72..a367b44 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -215,9 +215,9 @@
ClangASTContext *context(target->GetScratchClangASTContext());
- TypeFromUser user_type(m_ast_importer->DeportType(context->getASTContext(),
- ast->getASTContext(),
- parser_type.GetOpaqueQualType()),
+ TypeFromUser user_type(m_ast_importer_sp->DeportType(context->getASTContext(),
+ ast->getASTContext(),
+ parser_type.GetOpaqueQualType()),
context);
uint32_t offset = m_parser_vars->m_materializer->AddResultVariable(user_type,
@@ -258,9 +258,9 @@
ClangASTContext *context(target->GetScratchClangASTContext());
- TypeFromUser user_type(m_ast_importer->DeportType(context->getASTContext(),
- ast->getASTContext(),
- parser_type.GetOpaqueQualType()),
+ TypeFromUser user_type(m_ast_importer_sp->DeportType(context->getASTContext(),
+ ast->getASTContext(),
+ parser_type.GetOpaqueQualType()),
context);
if (!user_type.GetOpaqueQualType())
@@ -971,7 +971,7 @@
if (const NamespaceDecl *namespace_context = dyn_cast<NamespaceDecl>(context.m_decl_context))
{
- ClangASTImporter::NamespaceMapSP namespace_map = m_ast_importer->GetNamespaceMap(namespace_context);
+ ClangASTImporter::NamespaceMapSP namespace_map = m_ast_importer_sp->GetNamespaceMap(namespace_context);
if (log && log->GetVerbose())
log->Printf(" CEDM::FEVD[%u] Inspecting (NamespaceMap*)%p (%d entries)",
@@ -1291,7 +1291,7 @@
if (!ptype_type_decl)
break;
- Decl *parser_ptype_decl = m_ast_importer->CopyDecl(m_ast_context, scratch_ast_context, ptype_type_decl);
+ Decl *parser_ptype_decl = m_ast_importer_sp->CopyDecl(m_ast_context, scratch_ast_context, ptype_type_decl);
if (!parser_ptype_decl)
break;
@@ -1473,7 +1473,7 @@
{
if (llvm::isa<clang::FunctionDecl>(decl))
{
- clang::NamedDecl *copied_decl = llvm::cast<FunctionDecl>(m_ast_importer->CopyDecl(m_ast_context, &decl->getASTContext(), decl));
+ clang::NamedDecl *copied_decl = llvm::cast<FunctionDecl>(m_ast_importer_sp->CopyDecl(m_ast_context, &decl->getASTContext(), decl));
context.AddNamedDecl(copied_decl);
context.m_found.function_with_type_info = true;
}
@@ -1524,7 +1524,7 @@
name.GetCString());
}
- clang::Decl *copied_decl = m_ast_importer->CopyDecl(m_ast_context, &decl_from_modules->getASTContext(), decl_from_modules);
+ clang::Decl *copied_decl = m_ast_importer_sp->CopyDecl(m_ast_context, &decl_from_modules->getASTContext(), decl_from_modules);
clang::FunctionDecl *copied_function_decl = copied_decl ? dyn_cast<clang::FunctionDecl>(copied_decl) : nullptr;
if (!copied_function_decl)
@@ -1556,7 +1556,7 @@
name.GetCString());
}
- clang::Decl *copied_decl = m_ast_importer->CopyDecl(m_ast_context, &decl_from_modules->getASTContext(), decl_from_modules);
+ clang::Decl *copied_decl = m_ast_importer_sp->CopyDecl(m_ast_context, &decl_from_modules->getASTContext(), decl_from_modules);
clang::VarDecl *copied_var_decl = copied_decl ? dyn_cast_or_null<clang::VarDecl>(copied_decl) : nullptr;
if (!copied_var_decl)
@@ -1929,7 +1929,7 @@
QualType var_type = var_decl->getType();
TypeFromParser parser_type(var_type.getAsOpaquePtr(), ClangASTContext::GetASTContext(&var_decl->getASTContext()));
- lldb::opaque_compiler_type_t copied_type = m_ast_importer->CopyType(scratch_ast_context->getASTContext(), &var_decl->getASTContext(), var_type.getAsOpaquePtr());
+ lldb::opaque_compiler_type_t copied_type = m_ast_importer_sp->CopyType(scratch_ast_context->getASTContext(), &var_decl->getASTContext(), var_type.getAsOpaquePtr());
if (!copied_type)
{
diff --git a/lldb/source/Symbol/ClangASTContext.cpp b/lldb/source/Symbol/ClangASTContext.cpp
index 70f7901..d259c77 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -410,7 +410,7 @@
}
return ast_sp;
}
- else if (target)
+ else if (target && target->IsValid())
{
std::shared_ptr<ClangASTContextForExpressions> ast_sp(new ClangASTContextForExpressions(*target));
if (ast_sp)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index edec59c..86b6b99 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -83,7 +83,7 @@
m_process_sp (),
m_search_filter_sp (),
m_image_search_paths (ImageSearchPathsChanged, this),
- m_ast_importer_ap (),
+ m_ast_importer_sp (),
m_source_manager_ap(),
m_stop_hooks (),
m_stop_hook_next_id (0),
@@ -1178,7 +1178,7 @@
m_section_load_history.Clear();
m_images.Clear();
m_scratch_type_system_map.Clear();
- m_ast_importer_ap.reset();
+ m_ast_importer_sp.reset();
}
void
@@ -2107,21 +2107,18 @@
return nullptr;
}
-ClangASTImporter *
+ClangASTImporterSP
Target::GetClangASTImporter()
{
if (m_valid)
{
- ClangASTImporter *ast_importer = m_ast_importer_ap.get();
-
- if (!ast_importer)
+ if (!m_ast_importer_sp)
{
- ast_importer = new ClangASTImporter();
- m_ast_importer_ap.reset(ast_importer);
+ m_ast_importer_sp.reset(new ClangASTImporter());
}
- return ast_importer;
+ return m_ast_importer_sp;
}
- return nullptr;
+ return ClangASTImporterSP();
}
void