[Lldb-commits] [lldb] [lldb] rm DWARFDebugRanges (PR #116379)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Nov 15 22:31:40 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
<details>
<summary>Changes</summary>
The class is only used from one place, which is trivial to implement using the llvm class.
The main difference is that in the new implementation, the ranges are parsed each time anew (instead of being parsed at startup and cached). I believe this is fine because:
- this is already how things work with DWARF v5 debug_rnglists
- parsing debug_ranges is fairly fast (definitely faster than rnglists)
- generally, this result will be cached at a higher level anyway. Browsing the code I did find one instance where that is not the case -- SymbolFileDWARF::ResolveFunctionAndBlock -- which is called each time we resolve an address (to the block level). However, this function is already pretty suboptimal: it first traverses the DIE tree (which involves parsing all the DIE attributes) to find the correct block, then it parses them again to construct the `lldb_private::Block` representation, and *then* it uses the ID of the block DIE it found in the first step to look up the `Block` object. If this turns out to be a bottleneck, I think there are better ways to optimize it than caching the debug_ranges parse.
The motiviation for this is that DWARFDebugRanges sorts the block ranges, even though the order of the ranges is load-bearing (in the absence of DW_AT_low_pc, the "base address" of a scope is determined by the first range entry). Delaying the parsing (and sorting) step makes it easier to access the first entry.
---
Full diff: https://github.com/llvm/llvm-project/pull/116379.diff
9 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt (-1)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp (-1)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h (-1)
- (removed) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp (-56)
- (removed) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h (-34)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp (+41-33)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (-14)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (-4)
- (modified) lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s (+1-1)
``````````diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt b/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
index ea644a61b1bdaa..e87194dfe74cb3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
+++ b/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
@@ -21,7 +21,6 @@ add_lldb_library(lldbPluginSymbolFileDWARF PLUGIN
DWARFDebugInfo.cpp
DWARFDebugInfoEntry.cpp
DWARFDebugMacro.cpp
- DWARFDebugRanges.cpp
DWARFDeclContext.cpp
DWARFDefines.cpp
DWARFDIE.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index c0a2066f8239e5..4ecb2ed616a128 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -25,7 +25,6 @@
#include "DWARFCompileUnit.h"
#include "DWARFDebugAranges.h"
#include "DWARFDebugInfo.h"
-#include "DWARFDebugRanges.h"
#include "DWARFDeclContext.h"
#include "DWARFFormValue.h"
#include "DWARFUnit.h"
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
index 20c07c95ec47b5..0e50aab3292ae6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -14,7 +14,6 @@
#include "DWARFAttribute.h"
#include "DWARFBaseDIE.h"
-#include "DWARFDebugRanges.h"
#include <map>
#include <optional>
#include <set>
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
deleted file mode 100644
index fd8f4e12ff770c..00000000000000
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
+++ /dev/null
@@ -1,56 +0,0 @@
-//===-- DWARFDebugRanges.cpp ----------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "DWARFDebugRanges.h"
-#include "DWARFUnit.h"
-#include "llvm/DebugInfo/DWARF/DWARFDebugRangeList.h"
-
-using namespace lldb_private;
-using namespace lldb_private::plugin::dwarf;
-
-DWARFDebugRanges::DWARFDebugRanges() : m_range_map() {}
-
-void DWARFDebugRanges::Extract(DWARFContext &context) {
- llvm::DWARFDataExtractor extractor =
- context.getOrLoadRangesData().GetAsLLVMDWARF();
- llvm::DWARFDebugRangeList extracted_list;
- uint64_t current_offset = 0;
- auto extract_next_list = [&] {
- if (auto error = extracted_list.extract(extractor, ¤t_offset)) {
- consumeError(std::move(error));
- return false;
- }
- return true;
- };
-
- uint64_t previous_offset = current_offset;
- while (extractor.isValidOffset(current_offset) && extract_next_list()) {
- DWARFRangeList &lldb_range_list = m_range_map[previous_offset];
- lldb_range_list.Reserve(extracted_list.getEntries().size());
- for (auto &range : extracted_list.getEntries())
- lldb_range_list.Append(range.StartAddress,
- range.EndAddress - range.StartAddress);
- lldb_range_list.Sort();
- previous_offset = current_offset;
- }
-}
-
-DWARFRangeList
-DWARFDebugRanges::FindRanges(const DWARFUnit *cu,
- dw_offset_t debug_ranges_offset) const {
- dw_addr_t debug_ranges_address = cu->GetRangesBase() + debug_ranges_offset;
- auto pos = m_range_map.find(debug_ranges_address);
- DWARFRangeList ans =
- pos == m_range_map.end() ? DWARFRangeList() : pos->second;
-
- // All DW_AT_ranges are relative to the base address of the compile
- // unit. We add the compile unit base address to make sure all the
- // addresses are properly fixed up.
- ans.Slide(cu->GetBaseAddress());
- return ans;
-}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
deleted file mode 100644
index a04fcf59d5bfd6..00000000000000
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
+++ /dev/null
@@ -1,34 +0,0 @@
-//===-- DWARFDebugRanges.h --------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGRANGES_H
-#define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGRANGES_H
-
-#include "lldb/Core/dwarf.h"
-#include <map>
-
-namespace lldb_private::plugin {
-namespace dwarf {
-class DWARFUnit;
-class DWARFContext;
-
-class DWARFDebugRanges {
-public:
- DWARFDebugRanges();
-
- void Extract(DWARFContext &context);
- DWARFRangeList FindRanges(const DWARFUnit *cu,
- dw_offset_t debug_ranges_offset) const;
-
-protected:
- std::map<dw_offset_t, DWARFRangeList> m_range_map;
-};
-} // namespace dwarf
-} // namespace lldb_private::plugin
-
-#endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGRANGES_H
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 0eb882b0e7d4f5..0c9fd31cbfaefe 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -13,8 +13,10 @@
#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/Timer.h"
+#include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
#include "llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h"
#include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
+#include "llvm/DebugInfo/DWARF/DWARFDebugRangeList.h"
#include "llvm/Object/Error.h"
#include "DWARFCompileUnit.h"
@@ -1029,43 +1031,49 @@ DWARFUnit::GetStringOffsetSectionItem(uint32_t index) const {
llvm::Expected<DWARFRangeList>
DWARFUnit::FindRnglistFromOffset(dw_offset_t offset) {
+ llvm::DWARFAddressRangesVector llvm_ranges;
if (GetVersion() <= 4) {
- const DWARFDebugRanges *debug_ranges = m_dwarf.GetDebugRanges();
- if (!debug_ranges)
- return llvm::make_error<llvm::object::GenericBinaryError>(
- "No debug_ranges section");
- return debug_ranges->FindRanges(this, offset);
+ llvm::DWARFDataExtractor data =
+ m_dwarf.GetDWARFContext().getOrLoadRangesData().GetAsLLVMDWARF();
+ data.setAddressSize(m_header.getAddressByteSize());
+
+ llvm::DWARFDebugRangeList list;
+ if (llvm::Error e = list.extract(data, &offset))
+ return e;
+ llvm_ranges = list.getAbsoluteRanges(
+ llvm::object::SectionedAddress{GetBaseAddress()});
+ } else {
+ if (!GetRnglistTable())
+ return llvm::createStringError(std::errc::invalid_argument,
+ "missing or invalid range list table");
+
+ llvm::DWARFDataExtractor data = GetRnglistData().GetAsLLVMDWARF();
+
+ // As DW_AT_rnglists_base may be missing we need to call setAddressSize.
+ data.setAddressSize(m_header.getAddressByteSize());
+ auto range_list_or_error = GetRnglistTable()->findList(data, offset);
+ if (!range_list_or_error)
+ return range_list_or_error.takeError();
+
+ llvm::Expected<llvm::DWARFAddressRangesVector> expected_llvm_ranges =
+ range_list_or_error->getAbsoluteRanges(
+ llvm::object::SectionedAddress{GetBaseAddress()},
+ GetAddressByteSize(), [&](uint32_t index) {
+ uint32_t index_size = GetAddressByteSize();
+ dw_offset_t addr_base = GetAddrBase();
+ lldb::offset_t offset =
+ addr_base + static_cast<lldb::offset_t>(index) * index_size;
+ return llvm::object::SectionedAddress{
+ m_dwarf.GetDWARFContext().getOrLoadAddrData().GetMaxU64(
+ &offset, index_size)};
+ });
+ if (!expected_llvm_ranges)
+ return expected_llvm_ranges.takeError();
+ llvm_ranges = std::move(*expected_llvm_ranges);
}
- if (!GetRnglistTable())
- return llvm::createStringError(std::errc::invalid_argument,
- "missing or invalid range list table");
-
- llvm::DWARFDataExtractor data = GetRnglistData().GetAsLLVMDWARF();
-
- // As DW_AT_rnglists_base may be missing we need to call setAddressSize.
- data.setAddressSize(m_header.getAddressByteSize());
- auto range_list_or_error = GetRnglistTable()->findList(data, offset);
- if (!range_list_or_error)
- return range_list_or_error.takeError();
-
- llvm::Expected<llvm::DWARFAddressRangesVector> llvm_ranges =
- range_list_or_error->getAbsoluteRanges(
- llvm::object::SectionedAddress{GetBaseAddress()},
- GetAddressByteSize(), [&](uint32_t index) {
- uint32_t index_size = GetAddressByteSize();
- dw_offset_t addr_base = GetAddrBase();
- lldb::offset_t offset =
- addr_base + static_cast<lldb::offset_t>(index) * index_size;
- return llvm::object::SectionedAddress{
- m_dwarf.GetDWARFContext().getOrLoadAddrData().GetMaxU64(
- &offset, index_size)};
- });
- if (!llvm_ranges)
- return llvm_ranges.takeError();
-
DWARFRangeList ranges;
- for (const llvm::DWARFAddressRange &llvm_range : *llvm_ranges) {
+ for (const llvm::DWARFAddressRange &llvm_range : llvm_ranges) {
ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
llvm_range.HighPC - llvm_range.LowPC));
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 666595a6d0635f..4fd113fcbb6f61 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -63,7 +63,6 @@
#include "DWARFDebugAranges.h"
#include "DWARFDebugInfo.h"
#include "DWARFDebugMacro.h"
-#include "DWARFDebugRanges.h"
#include "DWARFDeclContext.h"
#include "DWARFFormValue.h"
#include "DWARFTypeUnit.h"
@@ -737,19 +736,6 @@ DWARFCompileUnit *SymbolFileDWARF::GetDWARFCompileUnit(CompileUnit *comp_unit) {
return llvm::cast_or_null<DWARFCompileUnit>(dwarf_cu);
}
-DWARFDebugRanges *SymbolFileDWARF::GetDebugRanges() {
- if (!m_ranges) {
- LLDB_SCOPED_TIMER();
-
- if (m_context.getOrLoadRangesData().GetByteSize() > 0)
- m_ranges = std::make_unique<DWARFDebugRanges>();
-
- if (m_ranges)
- m_ranges->Extract(m_context);
- }
- return m_ranges.get();
-}
-
/// Make an absolute path out of \p file_spec and remap it using the
/// module's source remapping dictionary.
static void MakeAbsoluteAndRemap(FileSpec &file_spec, DWARFUnit &dwarf_cu,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 4967b37d753a09..3a0b0e8087891c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -54,7 +54,6 @@ class DWARFDebugAranges;
class DWARFDebugInfo;
class DWARFDebugInfoEntry;
class DWARFDebugLine;
-class DWARFDebugRanges;
class DWARFDeclContext;
class DWARFFormValue;
class DWARFTypeUnit;
@@ -212,8 +211,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
DWARFDebugInfo &DebugInfo();
- DWARFDebugRanges *GetDebugRanges();
-
static bool SupportedVersion(uint16_t version);
DWARFDIE
@@ -531,7 +528,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
typedef std::set<DIERef> DIERefSet;
typedef llvm::StringMap<DIERefSet> NameToOffsetMap;
NameToOffsetMap m_function_scope_qualified_name_map;
- std::unique_ptr<DWARFDebugRanges> m_ranges;
UniqueDWARFASTTypeMap m_unique_ast_type_map;
// A map from DIE to lldb_private::Type. For record type, the key might be
// either declaration DIE or definition DIE.
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
index 7ee7320bc71e5d..504ba8a5e62bef 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
@@ -2,7 +2,7 @@
# RUN: %lldb %t -o "image lookup -v -s lookup_ranges" -o exit 2>%t.error | FileCheck %s
# RUN: cat %t.error | FileCheck %s --check-prefix ERROR
-# ERROR: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x0000000000000047) attribute, but range extraction failed (No debug_ranges section),
+# ERROR: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x0000000000000047) attribute, but range extraction failed (invalid range list offset 0x47),
# CHECK: Function: id = {0x0000001c}, name = "ranges", range = [0x0000000000000000-0x0000000000000004)
# CHECK: Blocks: id = {0x0000001c}, range = [0x00000000-0x00000004)
``````````
</details>
https://github.com/llvm/llvm-project/pull/116379
More information about the lldb-commits
mailing list