Skip to content

Allow lldb to load .dwp files with large .debug_info or .debug_types. #73736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

clayborg
Copy link
Collaborator

A previous patch to llvm allowed the DWARFUnitIndex class to handle .debug_info.dwo and .debug_types.dwo sections to go over 4GB by checking for this case and fixing up the DWARFUnitIndex. LLDB's DWARF parser tries to use the llvm's DWARF parser when it can, and LLDB's DWARF parser uses the llvm::DWARFUnitIndex which should allow us to load large .dwp files, but there were a few things missing on the LLDB front:

  • support for parsing DWARFUnit objects when the offset exceeds 4GB due to a 32 bit truncation issue
  • not populating the required DWARF sections when we call DWARFContext::GetAsLLVM() which didn't allow the fixups to happen as the data was missing.

This patch fixes these issues and now allows LLDB to parse large .dwp files without issues. The issue was discovered when running the "target modules dump separate-debug-info" command on one of these binaries that used a large .dwp file.

This is unfortunately hard to test without creating a huge .dwp file, so there are currently no tests for this that I can think of adding that wouldn't cause disk space constraints or making testing times longer by producing a huge .dwp file.

A previous patch to llvm allowed the DWARFUnitIndex class to handle .debug_info.dwo and .debug_types.dwo sections to go over 4GB by checking for this case and fixing up the DWARFUnitIndex. LLDB's DWARF parser tries to use the llvm's DWARF parser when it can, and LLDB's DWARF parser uses the llvm::DWARFUnitIndex which should allow us to load large .dwp files, but there were a few things missing on the LLDB front:
- support for parsing DWARFUnit objects when the offset exceeds 4GB due to a 32 bit truncation issue
- not populating the required DWARF sections when we call DWARFContext::GetAsLLVM() which didn't allow the fixups to happen as the data was missing.

This patch fixes these issues and now allows LLDB to parse large .dwp files without issues. The issue was discovered when running the "target modules dump separate-debug-info" command on one of these binaries that used a large .dwp file.

This is unfortunately hard to test without creating a huge .dwp file, so there are currently no tests for this that I can think of adding that wouldn't cause disk space constraints or making testing times longer by producing a huge .dwp file.
@llvmbot
Copy link
Member

llvmbot commented Nov 29, 2023

@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)

Changes

A previous patch to llvm allowed the DWARFUnitIndex class to handle .debug_info.dwo and .debug_types.dwo sections to go over 4GB by checking for this case and fixing up the DWARFUnitIndex. LLDB's DWARF parser tries to use the llvm's DWARF parser when it can, and LLDB's DWARF parser uses the llvm::DWARFUnitIndex which should allow us to load large .dwp files, but there were a few things missing on the LLDB front:

  • support for parsing DWARFUnit objects when the offset exceeds 4GB due to a 32 bit truncation issue
  • not populating the required DWARF sections when we call DWARFContext::GetAsLLVM() which didn't allow the fixups to happen as the data was missing.

This patch fixes these issues and now allows LLDB to parse large .dwp files without issues. The issue was discovered when running the "target modules dump separate-debug-info" command on one of these binaries that used a large .dwp file.

This is unfortunately hard to test without creating a huge .dwp file, so there are currently no tests for this that I can think of adding that wouldn't cause disk space constraints or making testing times longer by producing a huge .dwp file.


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

2 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp (+4-1)
  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h (+2-2)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
index ee347036dbbc034..e3872dc626be038 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
@@ -142,7 +142,10 @@ llvm::DWARFContext &DWARFContext::GetAsLLVM() {
     AddSection("debug_line_str", getOrLoadLineStrData());
     AddSection("debug_cu_index", getOrLoadCuIndexData());
     AddSection("debug_tu_index", getOrLoadTuIndexData());
-
+    if (isDwo()) {
+      AddSection("debug_info.dwo", getOrLoadDebugInfoData());
+      AddSection("debug_types.dwo", getOrLoadDebugTypesData());
+    }
     m_llvm_context = llvm::DWARFContext::create(section_map, addr_size);
   }
   return *m_llvm_context;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index 3aef03712d00dc8..3f528e913d8cfab 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -76,7 +76,7 @@ class DWARFUnitHeader {
     return m_unit_type == llvm::dwarf::DW_UT_type ||
            m_unit_type == llvm::dwarf::DW_UT_split_type;
   }
-  uint32_t GetNextUnitOffset() const { return m_offset + m_length + 4; }
+  dw_offset_t GetNextUnitOffset() const { return m_offset + m_length + 4; }
 
   llvm::Error ApplyIndexEntry(const llvm::DWARFUnitIndex::Entry *index_entry);
 
@@ -157,7 +157,7 @@ class DWARFUnit : public UserID {
   // 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 { return m_header.GetLength(); }
+  dw_offset_t GetLength() const { return m_header.GetLength(); }
   uint16_t GetVersion() const { return m_header.GetVersion(); }
   const llvm::DWARFAbbreviationDeclarationSet *GetAbbreviations() const;
   dw_offset_t GetAbbrevOffset() const;

@clayborg clayborg requested a review from zhyty November 29, 2023 02:15
Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me!

Copy link
Contributor

@ayermolo ayermolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing places I missed.

@clayborg clayborg merged commit ce00133 into llvm:main Nov 29, 2023
@clayborg
Copy link
Collaborator Author

The original patch that added the ability for the CU and TU indexes being able to exceed 4GB was this:
https://reviews.llvm.org/D137882

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants