[Lldb-commits] [PATCH] D40466: DWZ 03/12: DWARFCompileUnit split to DWARFCompileUnitData
jankratochvil updated this revision to Diff 132769.
jankratochvil added a comment.
Herald added subscribers: hintonda, mgorny.
DWARFCompileUnitData is no more, there is now DWARFUnit with virtual method
Data() to reference its inherited DWARFCompileUnit. I think it will make it
more mergeable with LLVM DWARFUnit.
https://reviews.llvm.org/D40466
Files:
source/Plugins/SymbolFile/DWARF/CMakeLists.txt
source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
source/Plugins/SymbolFile/DWARF/DWARFUnit.h
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- /dev/null
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -0,0 +1,159 @@
+//===-- DWARFUnit.h -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef SymbolFileDWARF_DWARFUnit_h_
+#define SymbolFileDWARF_DWARFUnit_h_
+
+#include "DWARFDIE.h"
+#include "DWARFDebugInfoEntry.h"
+#include "lldb/lldb-enumerations.h"
+
+class DWARFUnit;
+class DWARFCompileUnit;
+class NameToDIE;
+class SymbolFileDWARF;
+class SymbolFileDWARFDwo;
+
+typedef std::shared_ptr DWARFUnitSP;
+
+enum DWARFProducer {
+ eProducerInvalid = 0,
+ eProducerClang,
+ eProducerGCC,
+ eProducerLLVMGCC,
+ eProcucerOther
+};
+
+class DWARFUnit {
+public:
+ virtual ~DWARFUnit();
+
+ size_t ExtractDIEsIfNeeded(bool cu_die_only);
+ DWARFDIE LookupAddress(const dw_addr_t address);
+ size_t AppendDIEsWithTag(const dw_tag_t tag,
+ DWARFDIECollection &matching_dies,
+ uint32_t depth = UINT32_MAX) const;
+ bool Verify(lldb_private::Stream *s) const;
+ void Dump(lldb_private::Stream *s) const;
+ // Offset of the initial length field.
+ dw_offset_t GetOffset() const { return m_offset; }
+ lldb::user_id_t GetID() const;
+ // Size in bytes of the initial length + compile unit header.
+ uint32_t Size() const;
+ bool ContainsDIEOffset(dw_offset_t die_offset) const {
+return die_offset >= GetFirstDIEOffset() &&
+ die_offset < GetNextCompileUnitOffset();
+ }
+ dw_offset_t GetFirstDIEOffset() const { return m_offset + Size(); }
+ dw_offset_t GetNextCompileUnitOffset() const;
+ // Size of the CU data (without initial length and without header).
+ size_t GetDebugInfoSize() const;
+ // Size of the CU data incl. header but without initial length.
+ uint32_t GetLength() const;
+ uint16_t GetVersion() const;
+ const DWARFAbbreviationDeclarationSet *GetAbbreviations() const;
+ dw_offset_t GetAbbrevOffset() const;
+ uint8_t GetAddressByteSize() const;
+ dw_addr_t GetBaseAddress() const;
+ dw_addr_t GetAddrBase() const;
+ dw_addr_t GetRangesBase() const;
+ void SetAddrBase(dw_addr_t addr_base, dw_addr_t ranges_base, dw_offset_t base_obj_offset);
+ void ClearDIEs(bool keep_compile_unit_die);
+ void BuildAddressRangeTable(SymbolFileDWARF *dwarf2Data,
+ DWARFDebugAranges *debug_aranges);
+
+ lldb::ByteOrder GetByteOrder() const;
+
+ lldb_private::TypeSystem *GetTypeSystem();
+
+ DWARFFormValue::FixedFormSizes GetFixedFormSizes();
+
+ void SetBaseAddress(dw_addr_t base_addr);
+
+ DWARFDIE
+ GetCompileUnitDIEOnly();
+
+ DWARFDIE
+ DIE();
+
+ bool HasDIEsParsed() const;
+
+ DWARFDIE GetDIE(dw_offset_t die_offset);
+
+ static uint8_t GetAddressByteSize(const DWARFUnit *cu);
+
+ static bool IsDWARF64(const DWARFUnit *cu);
+
+ static uint8_t GetDefaultAddressSize();
+
+ static void SetDefaultAddressSize(uint8_t addr_size);
+
+ void *GetUserData() const;
+
+ void SetUserData(void *d);
+
+ bool Supports_DW_AT_APPLE_objc_complete_type();
+
+ bool DW_AT_decl_file_attributes_are_invalid();
+
+ bool Supports_unnamed_objc_bitfields();
+
+ void Index(NameToDIE &func_basenames, NameToDIE &func_fullnames,
+ NameToDIE &func_methods, NameToDIE &func_selectors,
+ NameToDIE &objc_class_selectors, NameToDIE &globals,
+ NameToDIE &types, NameToDIE &namespaces);
+
+ SymbolFileDWARF *GetSymbolFileDWARF() const;
+
+ DWARFProducer GetProducer();
+
+ uint32_t GetProducerVersionMajor();
+
+ uint32_t GetProducerVersionMinor();
+
+ uint32_t GetProducerVersionUpdate();
+
+ static lldb::LanguageType LanguageTypeFromDWARF(uint64_t val);
+
+ lldb::LanguageType GetLanguageType();
+
+ bool IsDWARF64() const;
+
+ bool GetIsOptimized();
+
+ SymbolFileDWARFDwo *GetDwoSymbolFile() const;
+
+ dw_offset_t GetBaseObjOffset() const;
+
+protected:
+ virtual DWARFCompileUnit &Data() = 0;
+ virtual const DWARFCompileUnit &Data() const = 0;
+
+ DWARFUnit();
+
+ static void
+ IndexPrivate(DWARFUnit *dwarf_cu, const lldb::Lang
[Lldb-commits] [PATCH] D40466: DWZ 03/12: DWARFCompileUnit split to DWARFCompileUnitData
jankratochvil added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:216 + DWARFCompileUnitData *m_data; + jankratochvil wrote: > clayborg wrote: > > jankratochvil wrote: > > > clayborg wrote: > > > > Is there a reason this is a member variable that I am not seeing? Seems > > > > we could have this class inherit from DWARFCompileUnitData. I am > > > > guessing this will be needed for a future patch? > > > Yes, future patch D40474 contains a new constructor so multiple > > > `DWARFCompileUnit` then point to single `DWARFCompileUnitData`. Sure > > > that happens only in the case ot `DW_TAG_partial_unit` (one > > > `DWARFCompileUnit` is read from a file while other `DWARFCompileUnit` are > > > remapped instances with unique offset as used from units which did use > > > `DW_TAG_imported_unit` for them). > > > `DWARFCompileUnit(DWARFCompileUnitData *data, DWARFCompileUnit *main_cu);` > > > > > We should just have an instance of this in this class and add a virtual > > function to retrieve it. Then we make a DWARFPartialUnit that subclasses > > this and has its own extra member variable and can use either one when it > > makes sense. Not a fan of just having a dangling pointer with no clear > > ownership. There is no "delete m_data" anywhere? > The `DWARFCompileUnitData *m_data` content is being held in: > `std::forward_list > SymbolFileDWARF::m_compile_unit_data_list` as I hope/believe a > `DWARFCompileUnit` is never deleted until whole `SymbolFileDWARF` is deleted. > I did not use `std::shared_ptr > DWARFCompileUnit::m_data` as `shared_ptr` is both memory and > lock-instruction-performance expensive. > > Then we make a DWARFPartialUnit that subclasses this We cannot because DWARFCompileUnit is constructed by `DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded`->`DWARFCompileUnit::Extract` and it currently does not know whether it is `DW_TAG_compile_unit` or `DW_TAG_partial_unit`. But then `DWARFCompileUnit::Extract` could read even the very first DIE and save it for later use. Then `DWARFCompileUnit::ExtractDIEsIfNeeded` would not need the `bool cu_die_only` parameter and altogether I could also drop my D40471. Do you agree? (This whole DWARF units parsing across the file is not too efficient anyway, there should be some index in use; and I would prefer the DWARF-5 one over the Apple one.) https://reviews.llvm.org/D40466 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40466: DWZ 03/12: DWARFCompileUnit split to DWARFCompileUnitData
jankratochvil marked an inline comment as done. jankratochvil added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:216 + DWARFCompileUnitData *m_data; + clayborg wrote: > jankratochvil wrote: > > clayborg wrote: > > > Is there a reason this is a member variable that I am not seeing? Seems > > > we could have this class inherit from DWARFCompileUnitData. I am guessing > > > this will be needed for a future patch? > > Yes, future patch D40474 contains a new constructor so multiple > > `DWARFCompileUnit` then point to single `DWARFCompileUnitData`. Sure that > > happens only in the case ot `DW_TAG_partial_unit` (one `DWARFCompileUnit` > > is read from a file while other `DWARFCompileUnit` are remapped instances > > with unique offset as used from units which did use `DW_TAG_imported_unit` > > for them). > > `DWARFCompileUnit(DWARFCompileUnitData *data, DWARFCompileUnit *main_cu);` > > > We should just have an instance of this in this class and add a virtual > function to retrieve it. Then we make a DWARFPartialUnit that subclasses this > and has its own extra member variable and can use either one when it makes > sense. Not a fan of just having a dangling pointer with no clear ownership. > There is no "delete m_data" anywhere? The `DWARFCompileUnitData *m_data` content is being held in: `std::forward_list SymbolFileDWARF::m_compile_unit_data_list` as I hope/believe a `DWARFCompileUnit` is never deleted until whole `SymbolFileDWARF` is deleted. I did not use `std::shared_ptr DWARFCompileUnit::m_data` as `shared_ptr` is both memory and lock-instruction-performance expensive. https://reviews.llvm.org/D40466 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40466: DWZ 03/12: DWARFCompileUnit split to DWARFCompileUnitData
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Take a look how the LLVM DWARF parser handles its units. It makes a DWARFUnit
base class that the compile unit inherits from. That can then be used for type
units and compile units.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:23-32
+class DWARFCompileUnitDecls {
public:
enum Producer {
eProducerInvalid = 0,
eProducerClang,
eProducerGCC,
eProducerLLVMGCC,
Just make this enum DWARFProducer and get rid of the DWARFCompileUnitDecls
class.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:34
+
+class DWARFCompileUnitData : public DWARFCompileUnitDecls {
+public:
Remove DWARFCompileUnitDecls and just use DWARFProducer
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:34
+
+class DWARFCompileUnitData : public DWARFCompileUnitDecls {
+public:
clayborg wrote:
> Remove DWARFCompileUnitDecls and just use DWARFProducer
The LLVM DWARF parser calls this class a DWARFUnit. We should probably do the
same.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:77
+class DWARFCompileUnit : public DWARFCompileUnitDecls {
+public:
remove DWARFCompileUnitDecls and just use DWARFProducer
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:216
+ DWARFCompileUnitData *m_data;
+
jankratochvil wrote:
> clayborg wrote:
> > Is there a reason this is a member variable that I am not seeing? Seems we
> > could have this class inherit from DWARFCompileUnitData. I am guessing this
> > will be needed for a future patch?
> Yes, future patch D40474 contains a new constructor so multiple
> `DWARFCompileUnit` then point to single `DWARFCompileUnitData`. Sure that
> happens only in the case ot `DW_TAG_partial_unit` (one `DWARFCompileUnit` is
> read from a file while other `DWARFCompileUnit` are remapped instances with
> unique offset as used from units which did use `DW_TAG_imported_unit` for
> them).
> `DWARFCompileUnit(DWARFCompileUnitData *data, DWARFCompileUnit *main_cu);`
>
We should just have an instance of this in this class and add a virtual
function to retrieve it. Then we make a DWARFPartialUnit that subclasses this
and has its own extra member variable and can use either one when it makes
sense. Not a fan of just having a dangling pointer with no clear ownership.
There is no "delete m_data" anywhere?
https://reviews.llvm.org/D40466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40466: DWZ 03/12: DWARFCompileUnit split to DWARFCompileUnitData
jankratochvil marked an inline comment as done. jankratochvil added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:216 + DWARFCompileUnitData *m_data; + clayborg wrote: > Is there a reason this is a member variable that I am not seeing? Seems we > could have this class inherit from DWARFCompileUnitData. I am guessing this > will be needed for a future patch? Yes, future patch D40474 contains a new constructor so multiple `DWARFCompileUnit` then point to single `DWARFCompileUnitData`. Sure that happens only in the case ot `DW_TAG_partial_unit` (one `DWARFCompileUnit` is read from a file while other `DWARFCompileUnit` are remapped instances with unique offset as used from units which did use `DW_TAG_imported_unit` for them). `DWARFCompileUnit(DWARFCompileUnitData *data, DWARFCompileUnit *main_cu);` https://reviews.llvm.org/D40466 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40466: DWZ 03/12: DWARFCompileUnit split to DWARFCompileUnitData
clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:216 + DWARFCompileUnitData *m_data; + Is there a reason this is a member variable that I am not seeing? Seems we could have this class inherit from DWARFCompileUnitData. I am guessing this will be needed for a future patch? https://reviews.llvm.org/D40466 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
