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 <pa...@labath.sk>
Date: Mon, 20 May 2024 12:30:46 +0000
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 &die_ref) {
return cu->GetNonSkeletonUnit().GetDIE(die_ref.die_offset());
return DWARFDIE(); // Not found
}
-
-llvm::StringRef DWARFDebugInfo::PeekDIEName(const DIERef &die_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 &die_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 &die_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
&debug_names) {
return result;
}
-std::optional<DIERef>
-DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const {
+DWARFUnit *DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry
&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<uint64_t> 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 ? &cu->GetNonSkeletonUnit() : nullptr;
+}
- cu = &cu->GetNonSkeletonUnit();
+std::optional<DIERef>
+DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const {
+ DWARFUnit *unit = GetNonSkeletonUnit(entry);
+ if (!unit)
+ return std::nullopt;
if (std::optional<uint64_t> die_offset = entry.getDIEUnitOffset())
- return DIERef(cu->GetSymbolFileDWARF().GetFileIndex(),
- DIERef::Section::DebugInfo, cu->GetOffset() + *die_offset);
+ return DIERef(unit->GetSymbolFileDWARF().GetFileIndex(),
+ DIERef::Section::DebugInfo, unit->GetOffset() + *die_offset);
return std::nullopt;
}
@@ -306,10 +309,10 @@ bool DebugNamesDWARFIndex::SameParentChain(
auto maybe_dieoffset = entry.getDIEUnitOffset();
if (!maybe_dieoffset)
return false;
- auto die_ref = ToDIERef(entry);
- if (!die_ref)
+ DWARFUnit *unit = GetNonSkeletonUnit(entry);
+ if (!unit)
return false;
- return name == m_debug_info.PeekDIEName(*die_ref);
+ return name == unit->PeekDIEName(unit->GetOffset() + *maybe_dieoffset);
};
// If the AT_name of any parent fails to match the expected name, we don't
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index b54dd1162d20a..81fb8f88b805a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -10,7 +10,6 @@
#define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DEBUGNAMESDWARFINDEX_H
#include "Plugins/SymbolFile/DWARF/DWARFIndex.h"
-#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
#include "Plugins/SymbolFile/DWARF/ManualDWARFIndex.h"
#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
#include "lldb/Utility/ConstString.h"
@@ -84,6 +83,7 @@ class DebugNamesDWARFIndex : public DWARFIndex {
std::unique_ptr<DebugNames> m_debug_names_up;
ManualDWARFIndex m_fallback;
+ DWARFUnit *GetNonSkeletonUnit(const DebugNames::Entry &entry) const;
std::optional<DIERef> ToDIERef(const DebugNames::Entry &entry) const;
bool ProcessEntry(const DebugNames::Entry &entry,
llvm::function_ref<bool(DWARFDIE die)> callback);
diff --git a/lldb/test/API/lang/cpp/limit-debug-info/Makefile
b/lldb/test/API/lang/cpp/limit-debug-info/Makefile
index 30230b3469ac3..cbb9690bcecba 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/Makefile
+++ b/lldb/test/API/lang/cpp/limit-debug-info/Makefile
@@ -1,5 +1,3 @@
CXX_SOURCES = main.cpp derived.cpp base.cpp
-CFLAGS_EXTRAS = $(LIMIT_DEBUG_INFO_FLAGS)
-
include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
b/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
index a4422cee91f03..0f6bb67263071 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
+++ b/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
@@ -5,13 +5,8 @@
class TestWithLimitDebugInfo(TestBase):
- @add_test_categories(["dwarf", "dwo"])
- def test_limit_debug_info(self):
- self.build()
-
- src_file = os.path.join(self.getSourceDir(), "main.cpp")
- src_file_spec = lldb.SBFileSpec(src_file)
- self.assertTrue(src_file_spec.IsValid(), "breakpoint file")
+ def _run_test(self, build_dict):
+ self.build(dictionary=build_dict)
# Get the path of the executable
exe_path = self.getBuildArtifact("a.out")
@@ -21,10 +16,8 @@ def test_limit_debug_info(self):
self.assertTrue(target.IsValid(), VALID_TARGET)
# Break on main function
- breakpoint = target.BreakpointCreateBySourceRegex("break here",
src_file_spec)
- self.assertTrue(
- breakpoint.IsValid() and breakpoint.GetNumLocations() >= 1,
VALID_BREAKPOINT
- )
+ 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())
@@ -32,14 +25,21 @@ def test_limit_debug_info(self):
# Get the thread of the process
self.assertEqual(process.GetState(), lldb.eStateStopped,
PROCESS_STOPPED)
- thread = lldbutil.get_stopped_thread(process,
lldb.eStopReasonBreakpoint)
- thread.StepInto()
- # Get frame for current thread
- frame = thread.GetSelectedFrame()
+ self.expect_expr("1", result_type="int", result_value="1")
+ self.expect_expr("this", result_type="Foo *")
+ self.expect_expr("this->x", result_type="int", result_value="12345")
+
+ self.runCmd("continue")
self.expect_expr("1", result_type="int", result_value="1")
+ self.expect_expr("this", result_type="ns::Foo2 *")
+ self.expect_expr("this->x", result_type="int", result_value="23456")
- v2 = frame.EvaluateExpression("this")
- self.assertTrue(v2.IsValid(), "'expr this' results in a valid SBValue
object")
- self.assertSuccess(v2.GetError(), "'expr this' succeeds without an
error.")
+ @add_test_categories(["dwarf", "dwo"])
+ 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"))
diff --git a/lldb/test/API/lang/cpp/limit-debug-info/base.cpp
b/lldb/test/API/lang/cpp/limit-debug-info/base.cpp
index 2968644888208..062eaa3c6f268 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/base.cpp
+++ b/lldb/test/API/lang/cpp/limit-debug-info/base.cpp
@@ -1,8 +1,7 @@
#include "base.h"
-FooNS::FooNS() : x(12345) {}
-
-void FooNS::bar() {
- x = 54321;
-}
+FooBase::FooBase() : x(12345) {}
+ns::Foo2Base::Foo2Base() : x(23456) {}
+void FooBase::bar() {}
+void ns::Foo2Base::bar() {}
diff --git a/lldb/test/API/lang/cpp/limit-debug-info/base.h
b/lldb/test/API/lang/cpp/limit-debug-info/base.h
index f4da76701c78a..8e4dd17e7007f 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/base.h
+++ b/lldb/test/API/lang/cpp/limit-debug-info/base.h
@@ -1,12 +1,22 @@
-class FooNS
-{
+class FooBase {
public:
- virtual void bar();
- virtual char baz() = 0;
+ virtual void bar();
protected:
- FooNS();
+ FooBase();
- int x;
+ int x;
};
+namespace ns {
+class Foo2Base {
+public:
+ virtual void bar();
+
+protected:
+ Foo2Base();
+
+ int x;
+};
+
+} // namespace ns
diff --git a/lldb/test/API/lang/cpp/limit-debug-info/derived.cpp
b/lldb/test/API/lang/cpp/limit-debug-info/derived.cpp
index 911fe3d9bc173..cbda8e706b520 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/derived.cpp
+++ b/lldb/test/API/lang/cpp/limit-debug-info/derived.cpp
@@ -1,11 +1,10 @@
#include "derived.h"
-Foo foo1;
-Foo foo2;
-
Foo::Foo() { a = 12345; }
+ns::Foo2::Foo2() { a = 23456; }
-char Foo::baz() {
- return (char)(x&0xff);
-}
+Foo foo1;
+Foo foo2;
+ns::Foo2 foo2_1;
+ns::Foo2 foo2_2;
diff --git a/lldb/test/API/lang/cpp/limit-debug-info/derived.h
b/lldb/test/API/lang/cpp/limit-debug-info/derived.h
index 8f95c52a595f0..a4aab37d9dee8 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/derived.h
+++ b/lldb/test/API/lang/cpp/limit-debug-info/derived.h
@@ -1,19 +1,36 @@
#include "base.h"
-class Foo : public FooNS
-{
+class Foo : public FooBase {
public:
- Foo();
+ Foo();
- // Deliberately defined by hand.
- Foo &operator=(const Foo &rhs) {
- a = rhs.a;
- return *this;
- }
+ // Deliberately defined by hand.
+ Foo &operator=(const Foo &rhs) {
+ x = rhs.x; // break1
+ a = rhs.a;
+ return *this;
+ }
+ int a;
+};
+
+namespace ns {
+class Foo2 : public Foo2Base {
+public:
+ Foo2();
- char baz() override;
- int a;
+ // Deliberately defined by hand.
+ Foo2 &operator=(const Foo2 &rhs) {
+ x = rhs.x; // break2
+ a = rhs.a;
+ return *this;
+ }
+
+ int a;
};
+} // namespace ns
extern Foo foo1;
extern Foo foo2;
+
+extern ns::Foo2 foo2_1;
+extern ns::Foo2 foo2_2;
diff --git a/lldb/test/API/lang/cpp/limit-debug-info/main.cpp
b/lldb/test/API/lang/cpp/limit-debug-info/main.cpp
index 35cb0373ae396..405fc2cdd4c86 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/main.cpp
+++ b/lldb/test/API/lang/cpp/limit-debug-info/main.cpp
@@ -1,8 +1,6 @@
#include "derived.h"
int main() {
- foo1 = foo2; // break here
-
- foo1.bar();
- return foo1.baz();
+ foo1 = foo2;
+ foo2_1 = foo2_2;
}
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
index bcb211815f9f3..20742ea512309 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
@@ -9,6 +9,7 @@
#include "Plugins/SymbolFile/DWARF/DWARFDIE.h"
#include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
#include "TestingSupport/Symbol/YAMLModuleTester.h"
+#include "lldb/Core/dwarf.h"
#include "llvm/ADT/STLExtras.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -169,21 +170,20 @@ TEST(DWARFDIETest, PeekName) {
YAMLModuleTester t(yamldata);
auto *symbol_file =
llvm::cast<SymbolFileDWARF>(t.GetModule()->GetSymbolFile());
- auto &debug_info = symbol_file->DebugInfo();
+ DWARFUnit *unit = symbol_file->DebugInfo().GetUnitAtIndex(0);
- DIERef first_die(std::nullopt, DIERef::Section::DebugInfo,
- 11 /*FirstDIEOffset*/);
- EXPECT_EQ(debug_info.PeekDIEName(first_die), "");
+ dw_offset_t first_die_offset = 11;
+ EXPECT_EQ(unit->PeekDIEName(first_die_offset), "");
- DIERef second_die(std::nullopt, DIERef::Section::DebugInfo, 14);
- EXPECT_EQ(debug_info.PeekDIEName(second_die), "NameType1");
+ dw_offset_t second_die_offset = 14;
+ EXPECT_EQ(unit->PeekDIEName(second_die_offset), "NameType1");
- DIERef third_die(std::nullopt, DIERef::Section::DebugInfo, 19);
- EXPECT_EQ(debug_info.PeekDIEName(third_die), "NameType2");
+ dw_offset_t third_die_offset = 19;
+ EXPECT_EQ(unit->PeekDIEName(third_die_offset), "NameType2");
- DIERef fourth_die(std::nullopt, DIERef::Section::DebugInfo, 24);
- EXPECT_EQ(debug_info.PeekDIEName(fourth_die), "NameType1");
+ dw_offset_t fourth_die_offset = 24;
+ EXPECT_EQ(unit->PeekDIEName(fourth_die_offset), "NameType1");
- DIERef fifth_die(std::nullopt, DIERef::Section::DebugInfo, 26);
- EXPECT_EQ(debug_info.PeekDIEName(fifth_die), "NameType2");
+ dw_offset_t fifth_die_offset = 26;
+ EXPECT_EQ(unit->PeekDIEName(fifth_die_offset), "NameType2");
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
