augusto2112 created this revision.
augusto2112 added reviewers: labath, clayborg, aprantl.
Herald added a reviewer: shafik.
Herald added a project: All.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Storing raw pointers in DieToTypePtr may cause use-after-frees to occur,
since there are no guarantees that the shared pointers that owns the
underlying pointer to the type are kept around as long as the map.
Change the map to store a shared pointer instead.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D141318
Files:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -56,7 +56,6 @@
class SymbolFileDWARFDwo;
class SymbolFileDWARFDwp;
-#define DIE_IS_BEING_PARSED ((lldb_private::Type *)1)
class SymbolFileDWARF : public lldb_private::SymbolFileCommon,
public lldb_private::UserID {
@@ -233,6 +232,10 @@
static bool SupportedVersion(uint16_t version);
+ /// Returns a sentinel pointer that indicates that a DIE is being
+ /// parsed into a type.
+ static std::shared_ptr<lldb_private::Type> GetSentinelDieBeingParsed();
+
DWARFDIE
GetDeclContextDIEContainingDIE(const DWARFDIE &die);
@@ -348,7 +351,8 @@
lldb_private::ConstString ConstructFunctionDemangledName(const DWARFDIE &die);
protected:
- typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb_private::Type *>
+ typedef llvm::DenseMap<const DWARFDebugInfoEntry *,
+ std::shared_ptr<lldb_private::Type>>
DIEToTypePtr;
typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP>
DIEToVariableSP;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -517,6 +517,13 @@
return version >= 2 && version <= 5;
}
+std::shared_ptr<lldb_private::Type>
+SymbolFileDWARF::GetSentinelDieBeingParsed() {
+ static std::shared_ptr<lldb_private::Type> sentinel =
+ std::make_shared<lldb_private::Type>();
+ return sentinel;
+}
+
uint32_t SymbolFileDWARF::CalculateAbilities() {
uint32_t abilities = 0;
if (m_objfile_sp != nullptr) {
@@ -1602,17 +1609,18 @@
// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done.
GetForwardDeclClangTypeToDie().erase(die_it);
- Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+ auto type_sp = GetDIEToType().lookup(dwarf_die.GetDIE());
Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion);
if (log)
GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
log, "0x%8.8" PRIx64 ": %s '%s' resolving forward declaration...",
dwarf_die.GetID(), dwarf_die.GetTagAsCString(),
- type->GetName().AsCString());
+ type_sp->GetName().AsCString());
assert(compiler_type);
if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
- return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type, compiler_type);
+ return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type_sp.get(),
+ compiler_type);
}
return false;
}
@@ -1624,7 +1632,7 @@
Type *type = GetTypeForDIE(die, resolve_function_context).get();
if (assert_not_being_parsed) {
- if (type != DIE_IS_BEING_PARSED)
+ if (type != SymbolFileDWARF::GetSentinelDieBeingParsed().get())
return type;
GetObjectFile()->GetModule()->ReportError(
@@ -2686,10 +2694,13 @@
TypeSP SymbolFileDWARF::GetTypeForDIE(const DWARFDIE &die,
bool resolve_function_context) {
- TypeSP type_sp;
if (die) {
- Type *type_ptr = GetDIEToType().lookup(die.GetDIE());
- if (type_ptr == nullptr) {
+ if (auto type_sp = GetDIEToType().lookup(die.GetDIE())) {
+ if (type_sp.get() !=
+ SymbolFileDWARF::GetSentinelDieBeingParsed().get()) {
+ return type_sp;
+ }
+ } else {
SymbolContextScope *scope;
if (auto *dwarf_cu = llvm::dyn_cast<DWARFCompileUnit>(die.GetCU()))
scope = GetCompUnitForDWARFCompUnit(*dwarf_cu);
@@ -2708,13 +2719,10 @@
!GetFunction(DWARFDIE(die.GetCU(), parent_die), sc))
sc = sc_backup;
- type_sp = ParseType(sc, die, nullptr);
- } else if (type_ptr != DIE_IS_BEING_PARSED) {
- // Get the original shared pointer for this type
- type_sp = type_ptr->shared_from_this();
+ return ParseType(sc, die, nullptr);
}
}
- return type_sp;
+ return nullptr;
}
DWARFDIE
@@ -2850,7 +2858,9 @@
return true;
Type *resolved_type = ResolveType(type_die, false, true);
- if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
+ if (!resolved_type ||
+ resolved_type ==
+ SymbolFileDWARF::GetSentinelDieBeingParsed().get())
return true;
DEBUG_PRINTF(
@@ -2861,7 +2871,8 @@
type_die.GetID(), type_cu->GetID());
if (die)
- GetDIEToType()[die.GetDIE()] = resolved_type;
+ GetDIEToType()[die.GetDIE()] =
+ std::make_shared<lldb_private::Type>(*resolved_type);
type_sp = resolved_type->shared_from_this();
return false;
});
@@ -3066,8 +3077,10 @@
return true;
Type *resolved_type = ResolveType(type_die, false);
- if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
- return true;
+ if (!resolved_type ||
+ resolved_type ==
+ SymbolFileDWARF::GetSentinelDieBeingParsed().get())
+ return true;
// With -gsimple-template-names, the DIE name may not contain the template
// parameters. If the declaration has template parameters but doesn't
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -206,7 +206,7 @@
TypePayloadClang(GetOwningClangModule(die))));
dwarf->GetTypeList().Insert(type_sp);
- dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+ dwarf->GetDIEToType()[die.GetDIE()] = type_sp;
clang::TagDecl *tag_decl = TypeSystemClang::GetAsTagDecl(type);
if (tag_decl) {
LinkDeclContextToDIE(tag_decl, die);
@@ -434,20 +434,21 @@
die.GetTagAsCString(), die.GetName());
}
- Type *type_ptr = dwarf->GetDIEToType().lookup(die.GetDIE());
- if (type_ptr == DIE_IS_BEING_PARSED)
- return nullptr;
- if (type_ptr)
- return type_ptr->shared_from_this();
+ if (auto type_ptr = dwarf->GetDIEToType().lookup(die.GetDIE())) {
+ if (type_ptr.get() == SymbolFileDWARF::GetSentinelDieBeingParsed().get())
+ return nullptr;
+ return type_ptr;
+ }
// Set a bit that lets us know that we are currently parsing this
- dwarf->GetDIEToType()[die.GetDIE()] = DIE_IS_BEING_PARSED;
+ dwarf->GetDIEToType()[die.GetDIE()] =
+ SymbolFileDWARF::GetSentinelDieBeingParsed();
ParsedDWARFTypeAttributes attrs(die);
if (DWARFDIE signature_die = attrs.signature.Reference()) {
if (TypeSP type_sp =
ParseTypeFromDWARF(sc, signature_die, type_is_new_ptr)) {
- dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+ dwarf->GetDIEToType()[die.GetDIE()] = type_sp;
if (clang::DeclContext *decl_ctx =
GetCachedClangDeclContextForDIE(signature_die))
LinkDeclContextToDIE(decl_ctx, die);
@@ -737,7 +738,7 @@
dwarf->GetUID(attrs.type.Reference()), encoding_data_type, &attrs.decl,
clang_type, resolve_state, TypePayloadClang(GetOwningClangModule(die)));
- dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+ dwarf->GetDIEToType()[die.GetDIE()] = type_sp;
return type_sp;
}
@@ -810,7 +811,7 @@
// We found a real definition for this type elsewhere so lets use
// it and cache the fact that we found a complete type for this
// die
- dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+ dwarf->GetDIEToType()[die.GetDIE()] = type_sp;
clang::DeclContext *defn_decl_ctx =
GetCachedClangDeclContextForDIE(dwarf->GetDIE(type_sp->GetID()));
if (defn_decl_ctx)
@@ -1063,10 +1064,10 @@
// Unfortunately classes don't like having stuff added
// to them after their definitions are complete...
- Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
- if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
- return type_ptr->shared_from_this();
- }
+ if (auto type_sp = dwarf->GetDIEToType()[die.GetDIE()])
+ if (type_sp.get() !=
+ SymbolFileDWARF::GetSentinelDieBeingParsed().get())
+ return type_sp;
}
}
@@ -1179,7 +1180,7 @@
// we need to modify the dwarf->GetDIEToType() so it
// doesn't think we are trying to parse this DIE
// anymore...
- dwarf->GetDIEToType()[die.GetDIE()] = NULL;
+ dwarf->GetDIEToType().erase(die.GetDIE());
// Now we get the full type to force our class type to
// complete itself using the clang::ExternalASTSource
@@ -1189,10 +1190,10 @@
// The type for this DIE should have been filled in the
// function call above
- Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
- if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
- return type_ptr->shared_from_this();
- }
+ if (auto type_sp = dwarf->GetDIEToType()[die.GetDIE()])
+ if (type_sp.get() !=
+ SymbolFileDWARF::GetSentinelDieBeingParsed().get())
+ return type_sp;
// FIXME This is fixing some even uglier behavior but we
// really need to
@@ -1528,7 +1529,7 @@
if (symbol_context_scope != nullptr)
type_sp->SetSymbolContextScope(symbol_context_scope);
- dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+ dwarf->GetDIEToType()[die.GetDIE()] = type_sp;
return type_sp;
}
@@ -1623,7 +1624,7 @@
*unique_ast_entry_up)) {
type_sp = unique_ast_entry_up->m_type_sp;
if (type_sp) {
- dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+ dwarf->GetDIEToType()[die.GetDIE()] = type_sp;
LinkDeclContextToDIE(
GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die);
return type_sp;
@@ -1699,7 +1700,7 @@
// We found a real definition for this type elsewhere so lets use
// it and cache the fact that we found a complete type for this
// die
- dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+ dwarf->GetDIEToType()[die.GetDIE()] = type_sp;
return type_sp;
}
}
@@ -1752,7 +1753,7 @@
// We found a real definition for this type elsewhere so lets use
// it and cache the fact that we found a complete type for this die
- dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+ dwarf->GetDIEToType()[die.GetDIE()] = type_sp;
clang::DeclContext *defn_decl_ctx =
GetCachedClangDeclContextForDIE(dwarf->GetDIE(type_sp->GetID()));
if (defn_decl_ctx)
@@ -2447,15 +2448,17 @@
SymbolFileDWARF *dwarf = die.GetDWARF();
// Supply the type _only_ if it has already been parsed
- Type *func_type = dwarf->GetDIEToType().lookup(die.GetDIE());
+ auto func_type_sp = dwarf->GetDIEToType().lookup(die.GetDIE());
- assert(func_type == nullptr || func_type != DIE_IS_BEING_PARSED);
+ assert(func_type_sp == nullptr ||
+ func_type_sp.get() !=
+ SymbolFileDWARF::GetSentinelDieBeingParsed().get());
const user_id_t func_user_id = die.GetID();
func_sp =
std::make_shared<Function>(&comp_unit,
func_user_id, // UserID is the DIE offset
- func_user_id, func_name, func_type,
+ func_user_id, func_name, func_type_sp.get(),
func_range); // first address range
if (func_sp.get() != nullptr) {
@@ -3603,7 +3606,7 @@
if (dst_decl_ctx)
src_dwarf_ast_parser->LinkDeclContextToDIE(dst_decl_ctx, src);
- if (Type *src_child_type = die_to_type[src.GetDIE()])
+ if (auto src_child_type = die_to_type[src.GetDIE()])
die_to_type[dst.GetDIE()] = src_child_type;
};
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
