[Lldb-commits] [PATCH] D73782: [lldb/DWARF] Don't hold a unique SymbolFileDWARFDwo in a DWARFUnit
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9dc84e9b02d1: [lldb/DWARF] Don't hold a unique
SymbolFileDWARFDwo in a DWARFUnit (authored by labath).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73782/new/
https://reviews.llvm.org/D73782
Files:
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -29,10 +29,7 @@
~SymbolFileDWARFDwo() override = default;
- DWARFCompileUnit *GetCompileUnit();
-
- DWARFUnit *
- GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) override;
+ DWARFCompileUnit *GetDWOCompileUnitForHash(uint64_t hash);
size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
DIEArray &method_die_offsets) override;
@@ -68,10 +65,11 @@
SymbolFileDWARF &GetBaseSymbolFile() { return m_base_symbol_file; }
- DWARFCompileUnit *ComputeCompileUnit();
+ /// If this file contains exactly one compile unit, this function will return
+ /// it. Otherwise it returns nullptr.
+ DWARFCompileUnit *FindSingleCompileUnit();
SymbolFileDWARF &m_base_symbol_file;
- DWARFCompileUnit *m_cu = nullptr;
};
#endif // SymbolFileDWARFDwo_SymbolFileDWARFDwo_h_
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -49,13 +49,17 @@
SymbolFileDWARF::LoadSectionData(sect_type, data);
}
-DWARFCompileUnit *SymbolFileDWARFDwo::GetCompileUnit() {
- if (!m_cu)
-m_cu = ComputeCompileUnit();
- return m_cu;
+DWARFCompileUnit *SymbolFileDWARFDwo::GetDWOCompileUnitForHash(uint64_t hash) {
+ DWARFCompileUnit *cu = FindSingleCompileUnit();
+ if (!cu)
+return nullptr;
+ if (hash !=
+ cu->GetUnitDIEOnly().GetAttributeValueAsUnsigned(DW_AT_GNU_dwo_id, 0))
+return nullptr;
+ return cu;
}
-DWARFCompileUnit *SymbolFileDWARFDwo::ComputeCompileUnit() {
+DWARFCompileUnit *SymbolFileDWARFDwo::FindSingleCompileUnit() {
DWARFDebugInfo *debug_info = DebugInfo();
if (!debug_info)
return nullptr;
@@ -79,11 +83,6 @@
return cu;
}
-DWARFUnit *
-SymbolFileDWARFDwo::GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) {
- return GetCompileUnit();
-}
-
SymbolFileDWARF::DIEToTypePtr &SymbolFileDWARFDwo::GetDIEToType() {
return GetBaseSymbolFile().GetDIEToType();
}
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -270,7 +270,7 @@
lldb::user_id_t GetUID(DIERef ref);
- std::unique_ptr
+ std::shared_ptr
GetDwoSymbolFileForCompileUnit(DWARFUnit &dwarf_cu,
const DWARFDebugInfoEntry &cu_die);
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1556,7 +1556,7 @@
return {};
}
-std::unique_ptr
+std::shared_ptr
SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
DWARFUnit &unit, const DWARFDebugInfoEntry &cu_die) {
// If this is a Darwin-style debug map (non-.dSYM) symbol file,
@@ -1611,7 +1611,7 @@
if (dwo_obj_file == nullptr)
return nullptr;
- return std::make_unique(*this, dwo_obj_file,
+ return std::make_shared(*this, dwo_obj_file,
dwarf_cu->GetID());
}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -277,7 +277,7 @@
}
SymbolFileDWARF &m_dwarf;
- std::unique_ptr m_dwo_symbol_file;
+ std::shared_ptr m_dwo;
DWARFUnitHeader m_header;
const DWARFAbbreviationDeclarationSet *m_abbrevs = nullptr;
void *m_user_data = nullptr;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -188,7 +188
[Lldb-commits] [PATCH] D73782: [lldb/DWARF] Don't hold a unique SymbolFileDWARFDwo in a DWARFUnit
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. LGTM, sorry for the delay! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73782/new/ https://reviews.llvm.org/D73782 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D73782: [lldb/DWARF] Don't hold a unique SymbolFileDWARFDwo in a DWARFUnit
labath added a comment. Do you have any more comments on this patch, Greg? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73782/new/ https://reviews.llvm.org/D73782 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D73782: [lldb/DWARF] Don't hold a unique SymbolFileDWARFDwo in a DWARFUnit
labath updated this revision to Diff 242915.
labath marked 2 inline comments as done.
labath added a comment.
- clang-format
- add some doxygen
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73782/new/
https://reviews.llvm.org/D73782
Files:
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -29,10 +29,7 @@
~SymbolFileDWARFDwo() override = default;
- DWARFCompileUnit *GetCompileUnit();
-
- DWARFUnit *
- GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) override;
+ DWARFCompileUnit *GetDWOCompileUnitForHash(uint64_t hash);
size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
DIEArray &method_die_offsets) override;
@@ -68,10 +65,11 @@
SymbolFileDWARF &GetBaseSymbolFile() { return m_base_symbol_file; }
- DWARFCompileUnit *ComputeCompileUnit();
+ /// If this file contains exactly one compile unit, this function will return
+ /// it. Otherwise it returns nullptr.
+ DWARFCompileUnit *FindSingleCompileUnit();
SymbolFileDWARF &m_base_symbol_file;
- DWARFCompileUnit *m_cu = nullptr;
};
#endif // SymbolFileDWARFDwo_SymbolFileDWARFDwo_h_
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -49,13 +49,17 @@
SymbolFileDWARF::LoadSectionData(sect_type, data);
}
-DWARFCompileUnit *SymbolFileDWARFDwo::GetCompileUnit() {
- if (!m_cu)
-m_cu = ComputeCompileUnit();
- return m_cu;
+DWARFCompileUnit *SymbolFileDWARFDwo::GetDWOCompileUnitForHash(uint64_t hash) {
+ DWARFCompileUnit *cu = FindSingleCompileUnit();
+ if (!cu)
+return nullptr;
+ if (hash !=
+ cu->GetUnitDIEOnly().GetAttributeValueAsUnsigned(DW_AT_GNU_dwo_id, 0))
+return nullptr;
+ return cu;
}
-DWARFCompileUnit *SymbolFileDWARFDwo::ComputeCompileUnit() {
+DWARFCompileUnit *SymbolFileDWARFDwo::FindSingleCompileUnit() {
DWARFDebugInfo *debug_info = DebugInfo();
if (!debug_info)
return nullptr;
@@ -79,11 +83,6 @@
return cu;
}
-DWARFUnit *
-SymbolFileDWARFDwo::GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) {
- return GetCompileUnit();
-}
-
SymbolFileDWARF::DIEToTypePtr &SymbolFileDWARFDwo::GetDIEToType() {
return GetBaseSymbolFile().GetDIEToType();
}
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -270,7 +270,7 @@
lldb::user_id_t GetUID(DIERef ref);
- std::unique_ptr
+ std::shared_ptr
GetDwoSymbolFileForCompileUnit(DWARFUnit &dwarf_cu,
const DWARFDebugInfoEntry &cu_die);
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1552,7 +1552,7 @@
return {};
}
-std::unique_ptr
+std::shared_ptr
SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
DWARFUnit &unit, const DWARFDebugInfoEntry &cu_die) {
// If this is a Darwin-style debug map (non-.dSYM) symbol file,
@@ -1607,7 +1607,7 @@
if (dwo_obj_file == nullptr)
return nullptr;
- return std::make_unique(*this, dwo_obj_file,
+ return std::make_shared(*this, dwo_obj_file,
dwarf_cu->GetID());
}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -277,7 +277,7 @@
}
SymbolFileDWARF &m_dwarf;
- std::unique_ptr m_dwo_symbol_file;
+ std::shared_ptr m_dwo;
DWARFUnitHeader m_header;
const DWARFAbbreviationDeclarationSet *m_abbrevs = nullptr;
void *m_user_data = nullptr;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -188,7 +188,7 @@
// simultaneously. We also don't need
[Lldb-commits] [PATCH] D73782: [lldb/DWARF] Don't hold a unique SymbolFileDWARFDwo in a DWARFUnit
labath marked 4 inline comments as done.
labath added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp:56
+return nullptr;
+ if (hash !=
cu->GetUnitDIEOnly().GetAttributeValueAsUnsigned(DW_AT_GNU_dwo_id, 0))
+return nullptr;
clayborg wrote:
> How often does this function get called? Should we cache the DW_AT_GNU_dwo_id
> in the DWARFCompileUnit to avoid extracting the DW_AT_GNU_dwo_id attribute
> maybe multiple times?
In a dwo file, this function will be called exactly once. With a DWP file (the
next patch) it will get called once for each compile unit, but it will return a
different compile unit each time, so for a single compile unit, the dwo_id
attribute will still be accessed only once.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h:67
SymbolFileDWARF &GetBaseSymbolFile() { return m_base_symbol_file; }
+ DWARFCompileUnit *FindSingleCompileUnit();
clayborg wrote:
> Curious: what is a single compile unit? A bit of a comment in header doc here
> might be nice.
I've added a simple comment. The idea is that a dwo file will normally(*)
contain exactly one compile unit. This function will find it and return it.
(*) This may not be 100% as I believe llvm can produce multiple CUs in a dwo
file under some circumstances, but I'm not sure if this is fully conforming,
and it is definitely not something that works in lldb right now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73782/new/
https://reviews.llvm.org/D73782
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D73782: [lldb/DWARF] Don't hold a unique SymbolFileDWARFDwo in a DWARFUnit
clayborg added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp:56
+return nullptr;
+ if (hash !=
cu->GetUnitDIEOnly().GetAttributeValueAsUnsigned(DW_AT_GNU_dwo_id, 0))
+return nullptr;
How often does this function get called? Should we cache the DW_AT_GNU_dwo_id
in the DWARFCompileUnit to avoid extracting the DW_AT_GNU_dwo_id attribute
maybe multiple times?
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h:67
SymbolFileDWARF &GetBaseSymbolFile() { return m_base_symbol_file; }
+ DWARFCompileUnit *FindSingleCompileUnit();
Curious: what is a single compile unit? A bit of a comment in header doc here
might be nice.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73782/new/
https://reviews.llvm.org/D73782
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D73782: [lldb/DWARF] Don't hold a unique SymbolFileDWARFDwo in a DWARFUnit
labath created this revision.
labath added reviewers: JDevlieghere, aprantl, clayborg.
Herald added a reviewer: jdoerfert.
Herald added a project: LLDB.
labath added a parent revision: D73781: [lldb/DWARF] Don't assume that a
SymbolFileDWARFDwo contains one compile unit.
labath added a child revision: D73783: [lldb/DWARF] Re-enable basic dwp support.
This is the second dwp preparatory patch. When a SymbolFileDWARFDwo will
hold more than one split unit, it will not be able to be uniquely owned
by a single DWARFUnit. I achieve this by changing the
unique_ptr member of DWARFUnit to
shared_ptr. The shared_ptr points to a DWARFUnit, but it is
in fact holding the entire SymbolFileDWARFDwo alive. This is the same
method used by llvm DWARFUnit (except that is uses the DWARFContext
class).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73782
Files:
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -29,10 +29,7 @@
~SymbolFileDWARFDwo() override = default;
- DWARFCompileUnit *GetCompileUnit();
-
- DWARFUnit *
- GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) override;
+ DWARFCompileUnit *GetDWOCompileUnitForHash(uint64_t hash);
size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
DIEArray &method_die_offsets) override;
@@ -68,10 +65,9 @@
SymbolFileDWARF &GetBaseSymbolFile() { return m_base_symbol_file; }
- DWARFCompileUnit *ComputeCompileUnit();
+ DWARFCompileUnit *FindSingleCompileUnit();
SymbolFileDWARF &m_base_symbol_file;
- DWARFCompileUnit *m_cu = nullptr;
};
#endif // SymbolFileDWARFDwo_SymbolFileDWARFDwo_h_
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -49,13 +49,16 @@
SymbolFileDWARF::LoadSectionData(sect_type, data);
}
-DWARFCompileUnit *SymbolFileDWARFDwo::GetCompileUnit() {
- if (!m_cu)
-m_cu = ComputeCompileUnit();
- return m_cu;
+DWARFCompileUnit *SymbolFileDWARFDwo::GetDWOCompileUnitForHash(uint64_t hash) {
+ DWARFCompileUnit *cu = FindSingleCompileUnit();
+ if (!cu)
+return nullptr;
+ if (hash != cu->GetUnitDIEOnly().GetAttributeValueAsUnsigned(DW_AT_GNU_dwo_id, 0))
+return nullptr;
+ return cu;
}
-DWARFCompileUnit *SymbolFileDWARFDwo::ComputeCompileUnit() {
+DWARFCompileUnit *SymbolFileDWARFDwo::FindSingleCompileUnit() {
DWARFDebugInfo *debug_info = DebugInfo();
if (!debug_info)
return nullptr;
@@ -79,11 +82,6 @@
return cu;
}
-DWARFUnit *
-SymbolFileDWARFDwo::GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) {
- return GetCompileUnit();
-}
-
SymbolFileDWARF::DIEToTypePtr &SymbolFileDWARFDwo::GetDIEToType() {
return GetBaseSymbolFile().GetDIEToType();
}
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -270,7 +270,7 @@
lldb::user_id_t GetUID(DIERef ref);
- std::unique_ptr
+ std::shared_ptr
GetDwoSymbolFileForCompileUnit(DWARFUnit &dwarf_cu,
const DWARFDebugInfoEntry &cu_die);
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1554,7 +1554,7 @@
return {};
}
-std::unique_ptr
+std::shared_ptr
SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
DWARFUnit &unit, const DWARFDebugInfoEntry &cu_die) {
// If this is a Darwin-style debug map (non-.dSYM) symbol file,
@@ -1609,7 +1609,7 @@
if (dwo_obj_file == nullptr)
return nullptr;
- return std::make_unique(*this, dwo_obj_file,
+ return std::make_shared(*this, dwo_obj_file,
dwarf_cu->GetID());
}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -281,7 +281,7 @@
}
SymbolFileDWARF &m_dwarf;
- std::unique_ptr m_dwo_symbol_file
