[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-21 Thread Pavel Labath via lldb-commits 

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/92745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-21 Thread Pavel Labath via lldb-commits 


@@ -9,7 +9,7 @@
 #include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
-#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"

labath wrote:

Not exactly. I wanted to remove (unused) SymbolFileDWARFDwo since it was 
potentially signalling breaking of abstractions, and then I got carried away 
include gardening (my editor now highlights unused headers :/).

Let me put those in separately.

https://github.com/llvm/llvm-project/pull/92745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-21 Thread Pavel Labath via lldb-commits 

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/92745

>From ae72d530c723a8d0fb2eac38224e8cf74cc43e70 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Mon, 20 May 2024 12:30:46 +
Subject: [PATCH 1/2] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf

DWARFDebugInfo only knows how to resolve references in its own file, but
in split dwarf, the index entries will refer to DIEs in the separate
(DWO) file. To resolve the DIERef correctly we'd either need to go
through the SymbolFileDWARF to get the full logic for resolving a
DIERef, or use the fact that ToDIERef already looks up the correct unit
while computing its result.

This patch does the latter.

This bug manifested itself in not being able to find type definitions
for types in namespaces, so I've modified one of our type resolving test
cases to run with debug_names, and added a namespaced class into it (it
originally contained only a top-level class).
---
 .../SymbolFile/DWARF/DWARFDebugInfo.cpp   |  6 ---
 .../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h |  5 ---
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 28 +++--
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  2 +-
 .../API/lang/cpp/limit-debug-info/Makefile|  2 -
 .../TestWithLimitDebugInfo.py | 40 +++
 .../API/lang/cpp/limit-debug-info/base.cpp|  9 ++---
 .../test/API/lang/cpp/limit-debug-info/base.h | 22 +++---
 .../API/lang/cpp/limit-debug-info/derived.cpp | 11 +++--
 .../API/lang/cpp/limit-debug-info/derived.h   | 37 -
 .../API/lang/cpp/limit-debug-info/main.cpp|  6 +--
 .../SymbolFile/DWARF/DWARFDIETest.cpp | 24 +--
 12 files changed, 106 insertions(+), 86 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 44febcfac3b00..d28da728728e5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -259,9 +259,3 @@ DWARFDebugInfo::GetDIE(const DIERef _ref) {
 return cu->GetNonSkeletonUnit().GetDIE(die_ref.die_offset());
   return DWARFDIE(); // Not found
 }
-
-llvm::StringRef DWARFDebugInfo::PeekDIEName(const DIERef _ref) {
-  if (DWARFUnit *cu = GetUnit(die_ref))
-return cu->GetNonSkeletonUnit().PeekDIEName(die_ref.die_offset());
-  return llvm::StringRef();
-}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index c1f0cb0203fb7..456ebd908ccb2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -44,11 +44,6 @@ class DWARFDebugInfo {
   bool ContainsTypeUnits();
   DWARFDIE GetDIE(const DIERef _ref);
 
-  /// Returns the AT_Name of this DIE, if it exists, without parsing the entire
-  /// compile unit. An empty is string is returned upon error or if the
-  /// attribute is not present.
-  llvm::StringRef PeekDIEName(const DIERef _ref);
-
   enum {
 eDumpFlag_Verbose = (1 << 0),  // Verbose dumping
 eDumpFlag_ShowForm = (1 << 1), // Show the DW_form type
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 4da0d56fdcacb..79400e36e04f3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -9,7 +9,7 @@
 #include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
-#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
@@ -48,26 +48,30 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames 
_names) {
   return result;
 }
 
-std::optional
-DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry ) const {
+DWARFUnit *
+DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry ) const 
{
   // Look for a DWARF unit offset (CU offset or local TU offset) as they are
   // both offsets into the .debug_info section.
   std::optional unit_offset = entry.getCUOffset();
   if (!unit_offset) {
 unit_offset = entry.getLocalTUOffset();
 if (!unit_offset)
-  return std::nullopt;
+  return nullptr;
   }
 
   DWARFUnit *cu =
   m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset);
-  if (!cu)
-return std::nullopt;
+  return cu ? >GetNonSkeletonUnit() : nullptr;
+}
 
-  cu = >GetNonSkeletonUnit();
+std::optional
+DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry ) const {
+  DWARFUnit *unit = GetNonSkeletonUnit(entry);
+  if (!unit)
+return std::nullopt;
   if (std::optional die_offset = entry.getDIEUnitOffset())
-return

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-20 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan approved this pull request.

Great catch and fix! Thanks for doing this

https://github.com/llvm/llvm-project/pull/92745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-20 Thread Felipe de Azevedo Piovezan via lldb-commits 


@@ -9,7 +9,7 @@
 #include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
-#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"

felipepiovezan wrote:

Is this actually needed?

https://github.com/llvm/llvm-project/pull/92745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-20 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/92745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-20 Thread Pavel Labath via lldb-commits 

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/92745

>From ae72d530c723a8d0fb2eac38224e8cf74cc43e70 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Mon, 20 May 2024 12:30:46 +
Subject: [PATCH] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf

DWARFDebugInfo only knows how to resolve references in its own file, but
in split dwarf, the index entries will refer to DIEs in the separate
(DWO) file. To resolve the DIERef correctly we'd either need to go
through the SymbolFileDWARF to get the full logic for resolving a
DIERef, or use the fact that ToDIERef already looks up the correct unit
while computing its result.

This patch does the latter.

This bug manifested itself in not being able to find type definitions
for types in namespaces, so I've modified one of our type resolving test
cases to run with debug_names, and added a namespaced class into it (it
originally contained only a top-level class).
---
 .../SymbolFile/DWARF/DWARFDebugInfo.cpp   |  6 ---
 .../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h |  5 ---
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 28 +++--
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  2 +-
 .../API/lang/cpp/limit-debug-info/Makefile|  2 -
 .../TestWithLimitDebugInfo.py | 40 +++
 .../API/lang/cpp/limit-debug-info/base.cpp|  9 ++---
 .../test/API/lang/cpp/limit-debug-info/base.h | 22 +++---
 .../API/lang/cpp/limit-debug-info/derived.cpp | 11 +++--
 .../API/lang/cpp/limit-debug-info/derived.h   | 37 -
 .../API/lang/cpp/limit-debug-info/main.cpp|  6 +--
 .../SymbolFile/DWARF/DWARFDIETest.cpp | 24 +--
 12 files changed, 106 insertions(+), 86 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 44febcfac3b00..d28da728728e5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -259,9 +259,3 @@ DWARFDebugInfo::GetDIE(const DIERef _ref) {
 return cu->GetNonSkeletonUnit().GetDIE(die_ref.die_offset());
   return DWARFDIE(); // Not found
 }
-
-llvm::StringRef DWARFDebugInfo::PeekDIEName(const DIERef _ref) {
-  if (DWARFUnit *cu = GetUnit(die_ref))
-return cu->GetNonSkeletonUnit().PeekDIEName(die_ref.die_offset());
-  return llvm::StringRef();
-}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index c1f0cb0203fb7..456ebd908ccb2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -44,11 +44,6 @@ class DWARFDebugInfo {
   bool ContainsTypeUnits();
   DWARFDIE GetDIE(const DIERef _ref);
 
-  /// Returns the AT_Name of this DIE, if it exists, without parsing the entire
-  /// compile unit. An empty is string is returned upon error or if the
-  /// attribute is not present.
-  llvm::StringRef PeekDIEName(const DIERef _ref);
-
   enum {
 eDumpFlag_Verbose = (1 << 0),  // Verbose dumping
 eDumpFlag_ShowForm = (1 << 1), // Show the DW_form type
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 4da0d56fdcacb..79400e36e04f3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -9,7 +9,7 @@
 #include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
-#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
@@ -48,26 +48,30 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames 
_names) {
   return result;
 }
 
-std::optional
-DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry ) const {
+DWARFUnit *
+DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry ) const 
{
   // Look for a DWARF unit offset (CU offset or local TU offset) as they are
   // both offsets into the .debug_info section.
   std::optional unit_offset = entry.getCUOffset();
   if (!unit_offset) {
 unit_offset = entry.getLocalTUOffset();
 if (!unit_offset)
-  return std::nullopt;
+  return nullptr;
   }
 
   DWARFUnit *cu =
   m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset);
-  if (!cu)
-return std::nullopt;
+  return cu ? >GetNonSkeletonUnit() : nullptr;
+}
 
-  cu = >GetNonSkeletonUnit();
+std::optional
+DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry ) const {
+  DWARFUnit *unit = GetNonSkeletonUnit(entry);
+  if (!unit)
+return std::nullopt;
   if (std::optional die_offset = entry.getDIEUnitOffset())
-return

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-20 Thread via lldb-commits 

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff da6a0b7af29a222b2e16a10155b49d4fafe967f3 
1c36a0c63f2c7c69048de36ab524cc8df7056704 -- 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h 
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h 
lldb/test/API/lang/cpp/limit-debug-info/base.cpp 
lldb/test/API/lang/cpp/limit-debug-info/base.h 
lldb/test/API/lang/cpp/limit-debug-info/derived.cpp 
lldb/test/API/lang/cpp/limit-debug-info/derived.h 
lldb/test/API/lang/cpp/limit-debug-info/main.cpp 
lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 7fff5f52df..79400e36e0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -48,7 +48,8 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames _names) 
{
   return result;
 }
 
-DWARFUnit *DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry 
) const {
+DWARFUnit *
+DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry ) const 
{
   // Look for a DWARF unit offset (CU offset or local TU offset) as they are
   // both offsets into the .debug_info section.
   std::optional unit_offset = entry.getCUOffset();

``




https://github.com/llvm/llvm-project/pull/92745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-20 Thread via lldb-commits 

github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r 
da6a0b7af29a222b2e16a10155b49d4fafe967f3...1c36a0c63f2c7c69048de36ab524cc8df7056704
 lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
``





View the diff from darker here.


``diff
--- TestWithLimitDebugInfo.py   2024-05-20 12:56:24.00 +
+++ TestWithLimitDebugInfo.py   2024-05-20 13:07:51.177002 +
@@ -14,12 +14,16 @@
 # Load the executable
 target = self.dbg.CreateTarget(exe_path)
 self.assertTrue(target.IsValid(), VALID_TARGET)
 
 # Break on main function
-lldbutil.run_break_set_by_file_and_line(self, "derived.h", 
line_number("derived.h", "// break1"))
-lldbutil.run_break_set_by_file_and_line(self, "derived.h", 
line_number("derived.h", "// break2"))
+lldbutil.run_break_set_by_file_and_line(
+self, "derived.h", line_number("derived.h", "// break1")
+)
+lldbutil.run_break_set_by_file_and_line(
+self, "derived.h", line_number("derived.h", "// break2")
+)
 
 # Launch the process
 process = target.LaunchSimple(None, None, 
self.get_process_working_directory())
 self.assertTrue(process.IsValid(), PROCESS_IS_VALID)
 
@@ -40,6 +44,8 @@
 def test_default(self):
 self._run_test(dict(CFLAGS_EXTRAS="$(LIMIT_DEBUG_INFO_FLAGS)"))
 
 @add_test_categories(["dwarf", "dwo"])
 def test_debug_names(self):
-self._run_test(dict(CFLAGS_EXTRAS="$(LIMIT_DEBUG_INFO_FLAGS) -gdwarf-5 
-gpubnames"))
+self._run_test(
+dict(CFLAGS_EXTRAS="$(LIMIT_DEBUG_INFO_FLAGS) -gdwarf-5 
-gpubnames")
+)

``




https://github.com/llvm/llvm-project/pull/92745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-20 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

DWARFDebugInfo only knows how to resolve references in its own file, but in 
split dwarf, the index entries will refer to DIEs in the separate (DWO) file. 
To resolve the DIERef correctly we'd either need to go through the 
SymbolFileDWARF to get the full logic for resolving a DIERef, or use the fact 
that ToDIERef already looks up the correct unit while computing its result.

This patch does the latter.

This bug manifested itself in not being able to find type definitions for types 
in namespaces, so I've modified one of our type resolving test cases to run 
with debug_names, and added a namespaced class into it (it originally contained 
only a top-level class).

---
Full diff: https://github.com/llvm/llvm-project/pull/92745.diff


12 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp (-6) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h (-5) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
(+15-12) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h (+1-1) 
- (modified) lldb/test/API/lang/cpp/limit-debug-info/Makefile (-2) 
- (modified) lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py 
(+18-18) 
- (modified) lldb/test/API/lang/cpp/limit-debug-info/base.cpp (+4-5) 
- (modified) lldb/test/API/lang/cpp/limit-debug-info/base.h (+16-6) 
- (modified) lldb/test/API/lang/cpp/limit-debug-info/derived.cpp (+5-6) 
- (modified) lldb/test/API/lang/cpp/limit-debug-info/derived.h (+27-10) 
- (modified) lldb/test/API/lang/cpp/limit-debug-info/main.cpp (+2-4) 
- (modified) lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp (+12-12) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 44febcfac3b00..d28da728728e5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -259,9 +259,3 @@ DWARFDebugInfo::GetDIE(const DIERef _ref) {
 return cu->GetNonSkeletonUnit().GetDIE(die_ref.die_offset());
   return DWARFDIE(); // Not found
 }
-
-llvm::StringRef DWARFDebugInfo::PeekDIEName(const DIERef _ref) {
-  if (DWARFUnit *cu = GetUnit(die_ref))
-return cu->GetNonSkeletonUnit().PeekDIEName(die_ref.die_offset());
-  return llvm::StringRef();
-}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index c1f0cb0203fb7..456ebd908ccb2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -44,11 +44,6 @@ class DWARFDebugInfo {
   bool ContainsTypeUnits();
   DWARFDIE GetDIE(const DIERef _ref);
 
-  /// Returns the AT_Name of this DIE, if it exists, without parsing the entire
-  /// compile unit. An empty is string is returned upon error or if the
-  /// attribute is not present.
-  llvm::StringRef PeekDIEName(const DIERef _ref);
-
   enum {
 eDumpFlag_Verbose = (1 << 0),  // Verbose dumping
 eDumpFlag_ShowForm = (1 << 1), // Show the DW_form type
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 4da0d56fdcacb..7fff5f52dfd79 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -9,7 +9,7 @@
 #include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
-#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
@@ -48,26 +48,29 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames 
_names) {
   return result;
 }
 
-std::optional
-DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry ) const {
+DWARFUnit *DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry 
) const {
   // Look for a DWARF unit offset (CU offset or local TU offset) as they are
   // both offsets into the .debug_info section.
   std::optional unit_offset = entry.getCUOffset();
   if (!unit_offset) {
 unit_offset = entry.getLocalTUOffset();
 if (!unit_offset)
-  return std::nullopt;
+  return nullptr;
   }
 
   DWARFUnit *cu =
   m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset);
-  if (!cu)
-return std::nullopt;
+  return cu ? >GetNonSkeletonUnit() : nullptr;
+}
 
-  cu = >GetNonSkeletonUnit();
+std::optional
+DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry ) const {
+  DWARFUnit *unit = GetNonSkeletonUnit(entry);
+  if (!unit)
+return std::nullopt;
   if (std::optional die_offset = entry.getDIEUnitOffset())
-return

[Lldb-commits] [lldb] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (PR #92745)

2024-05-20 Thread Pavel Labath via lldb-commits 

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/92745

DWARFDebugInfo only knows how to resolve references in its own file, but in 
split dwarf, the index entries will refer to DIEs in the separate (DWO) file. 
To resolve the DIERef correctly we'd either need to go through the 
SymbolFileDWARF to get the full logic for resolving a DIERef, or use the fact 
that ToDIERef already looks up the correct unit while computing its result.

This patch does the latter.

This bug manifested itself in not being able to find type definitions for types 
in namespaces, so I've modified one of our type resolving test cases to run 
with debug_names, and added a namespaced class into it (it originally contained 
only a top-level class).

>From 1c36a0c63f2c7c69048de36ab524cc8df7056704 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Mon, 20 May 2024 12:30:46 +
Subject: [PATCH] [lldb/dwarf] Fix DW_IDX_parent processing for split dwarf

DWARFDebugInfo only knows how to resolve references in its own file, but
in split dwarf, the index entries will refer to DIEs in the separate
(DWO) file. To resolve the DIERef correctly we'd either need to go
through the SymbolFileDWARF to get the full logic for resolving a
DIERef, or use the fact that ToDIERef already looks up the correct unit
while computing its result.

This patch does the latter.

This bug manifested itself in not being able to find type definitions
for types in namespaces, so I've modified one of our type resolving test
cases to run with debug_names, and added a namespaced class into it (it
originally contained only a top-level class).
---
 .../SymbolFile/DWARF/DWARFDebugInfo.cpp   |  6 ---
 .../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h |  5 ---
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 27 --
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  2 +-
 .../API/lang/cpp/limit-debug-info/Makefile|  2 -
 .../TestWithLimitDebugInfo.py | 36 +-
 .../API/lang/cpp/limit-debug-info/base.cpp|  9 ++---
 .../test/API/lang/cpp/limit-debug-info/base.h | 22 ---
 .../API/lang/cpp/limit-debug-info/derived.cpp | 11 +++---
 .../API/lang/cpp/limit-debug-info/derived.h   | 37 ++-
 .../API/lang/cpp/limit-debug-info/main.cpp|  6 +--
 .../SymbolFile/DWARF/DWARFDIETest.cpp | 24 ++--
 12 files changed, 100 insertions(+), 87 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 44febcfac3b00..d28da728728e5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -259,9 +259,3 @@ DWARFDebugInfo::GetDIE(const DIERef _ref) {
 return cu->GetNonSkeletonUnit().GetDIE(die_ref.die_offset());
   return DWARFDIE(); // Not found
 }
-
-llvm::StringRef DWARFDebugInfo::PeekDIEName(const DIERef _ref) {
-  if (DWARFUnit *cu = GetUnit(die_ref))
-return cu->GetNonSkeletonUnit().PeekDIEName(die_ref.die_offset());
-  return llvm::StringRef();
-}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index c1f0cb0203fb7..456ebd908ccb2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -44,11 +44,6 @@ class DWARFDebugInfo {
   bool ContainsTypeUnits();
   DWARFDIE GetDIE(const DIERef _ref);
 
-  /// Returns the AT_Name of this DIE, if it exists, without parsing the entire
-  /// compile unit. An empty is string is returned upon error or if the
-  /// attribute is not present.
-  llvm::StringRef PeekDIEName(const DIERef _ref);
-
   enum {
 eDumpFlag_Verbose = (1 << 0),  // Verbose dumping
 eDumpFlag_ShowForm = (1 << 1), // Show the DW_form type
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 4da0d56fdcacb..7fff5f52dfd79 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -9,7 +9,7 @@
 #include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
-#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
@@ -48,26 +48,29 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames 
_names) {
   return result;
 }
 
-std::optional
-DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry ) const {
+DWARFUnit *DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry 
) const {
   // Look for a DWARF unit offset (CU offset or local TU offset) as they are
   // both offsets into the .debug_info section.