date:20191124

[Lldb-commits] [PATCH] D70645: RFC 1/3: Unify src<->dst DWARFASTParser for CopyUniqueClassMethodTypes()

2019-11-24 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil created this revision.
jankratochvil added a reviewer: labath.
jankratochvil added a project: LLDB.
Herald added a subscriber: aprantl.
Herald added a reviewer: shafik.

This patchset is removing non-DWARF code from `DWARFUnit` as discussed with 
@labath.
For removing the dependency on `DWARFDIE.GetCU()` (further patch) I have found 
I can no longer find two different parsers in this case.
IIUC it would mean `DW_AT_language` is different between the defining and 
declaring DIE tree - which can happen (and `lldbassert` in this patch would 
fail then - this `lldbassert` is removed in the future patch anyway) but it 
would be a buggy DWARF in such case.
I do not plan to check it in yet. I plan to finish it for use by DWZ patchset 
first (D40474  et al.). Asking whether this is 
a good way forward.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70645

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3774,10 +3774,10 @@
 }
   }
 
-  DWARFASTParserClang *src_dwarf_ast_parser =
+  DWARFASTParserClang *dwarf_ast_parser =
   (DWARFASTParserClang *)src_die.GetDWARFParser();
-  DWARFASTParserClang *dst_dwarf_ast_parser =
-  (DWARFASTParserClang *)dst_die.GetDWARFParser();
+  lldbassert(dwarf_ast_parser ==
+ (DWARFASTParserClang *)dst_die.GetDWARFParser());
 
   // Now do the work of linking the DeclContexts and Types.
   if (fast_path) {
@@ -3788,12 +3788,12 @@
   dst_die = dst_name_to_die.GetValueAtIndexUnchecked(idx);
 
   clang::DeclContext *src_decl_ctx =
-  src_dwarf_ast_parser->m_die_to_decl_ctx[src_die.GetDIE()];
+  dwarf_ast_parser->m_die_to_decl_ctx[src_die.GetDIE()];
   if (src_decl_ctx) {
 LLDB_LOGF(log, "uniquing decl context %p from 0x%8.8x for 0x%8.8x",
   static_cast(src_decl_ctx), src_die.GetOffset(),
   dst_die.GetOffset());
-dst_dwarf_ast_parser->LinkDeclContextToDIE(src_decl_ctx, dst_die);
+dwarf_ast_parser->LinkDeclContextToDIE(src_decl_ctx, dst_die);
   } else {
 LLDB_LOGF(log,
   "warning: tried to unique decl context from 0x%8.8x for "
@@ -3832,12 +3832,12 @@
 
 if (src_die && (src_die.Tag() == dst_die.Tag())) {
   clang::DeclContext *src_decl_ctx =
-  src_dwarf_ast_parser->m_die_to_decl_ctx[src_die.GetDIE()];
+  dwarf_ast_parser->m_die_to_decl_ctx[src_die.GetDIE()];
   if (src_decl_ctx) {
 LLDB_LOGF(log, "uniquing decl context %p from 0x%8.8x for 0x%8.8x",
   static_cast(src_decl_ctx), src_die.GetOffset(),
   dst_die.GetOffset());
-dst_dwarf_ast_parser->LinkDeclContextToDIE(src_decl_ctx, dst_die);
+dwarf_ast_parser->LinkDeclContextToDIE(src_decl_ctx, dst_die);
   } else {
 LLDB_LOGF(log,
   "warning: tried to unique decl context from 0x%8.8x "
@@ -3887,12 +3887,12 @@
   if (dst_die) {
 // Both classes have the artificial types, link them
 clang::DeclContext *src_decl_ctx =
-src_dwarf_ast_parser->m_die_to_decl_ctx[src_die.GetDIE()];
+dwarf_ast_parser->m_die_to_decl_ctx[src_die.GetDIE()];
 if (src_decl_ctx) {
   LLDB_LOGF(log, "uniquing decl context %p from 0x%8.8x for 0x%8.8x",
 static_cast(src_decl_ctx), src_die.GetOffset(),
 dst_die.GetOffset());
-  dst_dwarf_ast_parser->LinkDeclContextToDIE(src_decl_ctx, dst_die);
+  dwarf_ast_parser->LinkDeclContextToDIE(src_decl_ctx, dst_die);
 } else {
   LLDB_LOGF(log,
 "warning: tried to unique decl context from 0x%8.8x "


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3774,10 +3774,10 @@
 }
   }
 
-  DWARFASTParserClang *src_dwarf_ast_parser =
+  DWARFASTParserClang *dwarf_ast_parser =
   (DWARFASTParserClang *)src_die.GetDWARFParser();
-  DWARFASTParserClang *dst_dwarf_ast_parser =
-  (DWARFASTParserClang *)dst_die.GetDWARFParser();
+  lldbassert(dwarf_ast_parser ==
+ (DWARFASTParserClang *)dst_die.GetDWARFParser());
 
   // Now do the work of linking the DeclContexts and Types.
   if (fast_path) {
@@ -3788,12 +3788,12 @@
   dst_die = dst_name_to_die.GetValueAtIndexUnchecked(idx);
 
   clang::DeclContext *src_decl_ctx =
-  src_dwarf_ast_parser->m_die_to_decl_ctx[src_die.GetDIE()];
+  dwar

[Lldb-commits] [PATCH] D70646: RFC 2/3: Move non-DWARF code: `DWARFUnit` -> `SymbolFileDWARF`

2019-11-24 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil created this revision.
jankratochvil added a reviewer: labath.
jankratochvil added a project: LLDB.
Herald added subscribers: arphaman, aprantl.
Herald added a reviewer: shafik.
jankratochvil added a parent revision: D70645: RFC 1/3: Unify src<->dst 
DWARFASTParser for CopyUniqueClassMethodTypes().

This patchset is removing non-DWARF code from `DWARFUnit` as discussed with 
@labath.
@labath, maybe this is the only patch you did mean? But without the 
next/further patches it isn't really useful for DWZ as it still expects 
`DWARFDIE` itself can reach `DW_AT_language` - which it cannot as it does not 
contain `DWARFUnit *main_cu`.
I do not plan to check it in yet. I plan to finish it for use by DWZ patchset 
first (D40474  et al.). Asking whether this is 
a good way forward.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70646

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.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
@@ -318,6 +318,27 @@
 
   lldb_private::FileSpec GetFile(DWARFUnit &unit, size_t file_idx);
 
+  static llvm::Expected
+  GetTypeSystem(DWARFUnit &unit);
+
+  static DWARFASTParser *GetDWARFParser(DWARFUnit &unit);
+
+  // CompilerDecl related functions
+
+  static lldb_private::CompilerDecl GetDecl(const DWARFDIE &die);
+
+  static lldb_private::CompilerDeclContext GetDeclContext(const DWARFDIE &die);
+
+  static lldb_private::CompilerDeclContext
+  GetContainingDeclContext(const DWARFDIE &die);
+
+  static void GetDWARFDeclContext(const DWARFDIE &die,
+  DWARFDeclContext &dwarf_decl_ctx);
+
+  static lldb::LanguageType LanguageTypeFromDWARF(uint64_t val);
+
+  static lldb::LanguageType GetLanguage(DWARFUnit &unit);
+
 protected:
   typedef llvm::DenseMap
   DIEToTypePtr;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -722,7 +722,7 @@
   cu_file_spec.SetFile(remapped_file, FileSpec::Style::native);
   }
 
-  LanguageType cu_language = DWARFUnit::LanguageTypeFromDWARF(
+  LanguageType cu_language = SymbolFileDWARF::LanguageTypeFromDWARF(
   cu_die.GetAttributeValueAsUnsigned(DW_AT_language, 0));
 
   bool is_optimized = dwarf_cu.GetIsOptimized();
@@ -800,8 +800,7 @@
   if (!die.IsValid())
 return nullptr;
 
-  auto type_system_or_err =
-  GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
+  auto type_system_or_err = GetTypeSystemForLanguage(GetLanguage(*die.GetCU()));
   if (auto err = type_system_or_err.takeError()) {
 LLDB_LOG_ERROR(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_SYMBOLS),
std::move(err), "Unable to parse function");
@@ -826,7 +825,7 @@
   std::lock_guard guard(GetModuleMutex());
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
   if (dwarf_cu)
-return dwarf_cu->GetLanguageType();
+return GetLanguage(*dwarf_cu);
   else
 return eLanguageTypeUnknown;
 }
@@ -1309,7 +1308,7 @@
   // SymbolFileDWARF::GetDIE(). See comments inside the
   // SymbolFileDWARF::GetDIE() for details.
   if (DWARFDIE die = GetDIE(type_uid))
-return die.GetDecl();
+return GetDecl(die);
   return CompilerDecl();
 }
 
@@ -1322,7 +1321,7 @@
   // SymbolFileDWARF::GetDIE(). See comments inside the
   // SymbolFileDWARF::GetDIE() for details.
   if (DWARFDIE die = GetDIE(type_uid))
-return die.GetDeclContext();
+return GetDeclContext(die);
   return CompilerDeclContext();
 }
 
@@ -1333,7 +1332,7 @@
   // SymbolFileDWARF::GetDIE(). See comments inside the
   // SymbolFileDWARF::GetDIE() for details.
   if (DWARFDIE die = GetDIE(type_uid))
-return die.GetContainingDeclContext();
+return GetContainingDeclContext(die);
   return CompilerDeclContext();
 }
 
@@ -1464,7 +1463,7 @@
   dwarf_die.GetID(), dwarf_die.GetTagAsCString(),
   type->GetName().AsCString());
 assert(compiler_type);
-DWARFASTParser *dwarf_ast = dwarf_die.GetDWARFParser();
+DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU());
 if (dwarf_ast)
   r

[Lldb-commits] [PATCH] D70647: RFC 3/3: Remove DWARFDIE dependency from functions moved by D70646

2019-11-24 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil created this revision.
jankratochvil added a reviewer: labath.
jankratochvil added a project: LLDB.
Herald added subscribers: arphaman, aprantl.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: shafik.
jankratochvil added a parent revision: D70646: RFC 2/3: Move non-DWARF code: 
`DWARFUnit` -> `SymbolFileDWARF`.

This patchset is removing dependency on `DWARFDIE`'s `DWARFUnit *m_cu` for 
stuff which is unavailable in `DW_TAG_partial_unit` without accessing its 
parent `DW_TAG_compile_unit` (which includes `DW_TAG_partial_unit` by 
`DW_AT_import`).
This patch has no regressions but to really make it usable for DWZ one still 
needs to adjust `DWARFBaseDIE::GetDIERef()` as one cannot generate `DIERef` 
(containing main unit idenitifier for DWZ) purely from `DWARFDIE` (not 
containing `DWARFUnit *main_cu`).
Refactored `DWARFBaseDIE::GetDIERef()` (adding some parameter) would then need 
adjustment also of functions calling it directly - `DWARFBaseDIE::GetID()` and 
`SymbolFileDWARF::GetUID(const DWARFBaseDIE &die)` (and many functions calling 
these).
I do not plan to check it in yet. I plan to finish it for use by DWZ patchset 
first (D40474  et al.). Asking whether this is 
a good way forward.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70647

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  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
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -48,7 +48,7 @@
 TEST_F(DWARFASTParserClangTests,
EnsureAllDIEsInDeclContextHaveBeenParsedParsesOnlyMatchingEntries) {
   ClangASTContext ast_ctx;
-  DWARFASTParserClangStub ast_parser(ast_ctx);
+  DWARFASTParserClangStub ast_parser(ast_ctx, *(SymbolFileDWARF *)1LL);
 
   DWARFUnit *unit = nullptr;
   std::vector dies = {DWARFDIE(unit, (DWARFDebugInfoEntry *)1LL),
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -9811,9 +9811,9 @@
   }
 }
 
-DWARFASTParser *ClangASTContext::GetDWARFParser() {
+DWARFASTParser *ClangASTContext::GetDWARFParser(SymbolFileDWARF &dwarf) {
   if (!m_dwarf_ast_parser_up)
-m_dwarf_ast_parser_up.reset(new DWARFASTParserClang(*this));
+m_dwarf_ast_parser_up.reset(new DWARFASTParserClang(*this, dwarf));
   return m_dwarf_ast_parser_up.get();
 }
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -44,8 +44,7 @@
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  DWARFDIE
-  GetDIE(const DIERef &die_ref) override;
+  DWARFUnit *GetUnit(DIERef die_ref) override;
 
   std::unique_ptr
   GetDwoSymbolFileForCompileUnit(DWARFUnit &dwarf_cu,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -147,9 +147,8 @@
   return GetBaseSymbolFile().GetTypeSystemForLanguage(language);
 }
 
-DWARFDIE
-SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
+DWARFUnit *SymbolFileDWARFDwo::GetUnit(DIERef die_ref) {
   if (*die_ref.dwo_num() == GetDwoNum())
-return DebugInfo()->GetDIE(die_ref);
-  return GetBaseSymbolFile().GetDIE(die_ref);
+return DebugInfo()->GetUnit(die_ref);
+  return GetBaseSymbolFile().GetUnit(die_ref);
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -275,7 +275,9 @@
   return m_external_type_modules;
   }
 
-  virtual DWARFDIE GetDIE(const DIERef &die_ref);
+  virtual DWARFUnit *GetUnit(DIERef die_ref);
+
+  DWARFDIE GetDIE(DIERef die_ref);
 
   DWARFDIE GetDI

[Lldb-commits] [PATCH] D70393: [lldb] Fix NSURL data formatter truncation issue in Swift

2019-11-24 Thread Martin Svensson via Phabricator via lldb-commits

poya removed a reviewer: teemperor.
poya added a comment.
This revision is now accepted and ready to land.

Removing Raphael as reviewer hoping that it will allow me to close this already 
merged revision.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70393/new/

https://reviews.llvm.org/D70393



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70645: RFC 1/3: Unify src<->dst DWARFASTParser for CopyUniqueClassMethodTypes()

[Lldb-commits] [PATCH] D70646: RFC 2/3: Move non-DWARF code: `DWARFUnit` -> `SymbolFileDWARF`

[Lldb-commits] [PATCH] D70647: RFC 3/3: Remove DWARFDIE dependency from functions moved by D70646

[Lldb-commits] [PATCH] D70393: [lldb] Fix NSURL data formatter truncation issue in Swift

4 matches

Site Navigation

Mail list logo

Footer information