[lldb] Enable support for DWARF64 format handling #145645
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Hemang Gadhavi (HemangGadhavi) ChangesThis PR introduces support for the DWARF64 format, enabling handling of 64-bit DWARF sections as defined by the DWARF specification. The update includes adjustments to header parsing and modification of form values to accommodate 64-bit offsets and values. Full diff: https://github.com/llvm/llvm-project/pull/145645.diff 3 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index fd3d45cef4c5e..1a712c3c769af 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -77,7 +77,10 @@ bool DWARFFormValue::ExtractValue(const DWARFDataExtractor &data,
case DW_FORM_strp:
case DW_FORM_line_strp:
case DW_FORM_sec_offset:
- m_value.uval = data.GetMaxU64(offset_ptr, 4);
+ if (m_unit->GetFormat() == DwarfFormat::DWARF32)
+ m_value.uval = data.GetMaxU64(offset_ptr, 4);
+ else if (m_unit->GetFormat() == DwarfFormat::DWARF64)
+ m_value.uval = data.GetMaxU64(offset_ptr, 8);
break;
case DW_FORM_addrx1:
case DW_FORM_strx1:
@@ -121,8 +124,12 @@ bool DWARFFormValue::ExtractValue(const DWARFDataExtractor &data,
assert(m_unit);
if (m_unit->GetVersion() <= 2)
ref_addr_size = m_unit->GetAddressByteSize();
- else
- ref_addr_size = 4;
+ else {
+ if (m_unit->GetFormat() == DwarfFormat::DWARF32)
+ ref_addr_size = 4;
+ else if (m_unit->GetFormat() == DwarfFormat::DWARF64)
+ ref_addr_size = 8;
+ }
m_value.uval = data.GetMaxU64(offset_ptr, ref_addr_size);
break;
case DW_FORM_indirect:
@@ -165,7 +172,7 @@ static FormSize g_form_sizes[] = {
{1, 1}, // 0x0b DW_FORM_data1
{1, 1}, // 0x0c DW_FORM_flag
{0, 0}, // 0x0d DW_FORM_sdata
- {1, 4}, // 0x0e DW_FORM_strp
+ {0, 0}, // 0x0e DW_FORM_strp (4 bytes for DWARF32, 8 bytes for DWARF64)
{0, 0}, // 0x0f DW_FORM_udata
{0, 0}, // 0x10 DW_FORM_ref_addr (addr size for DWARF2 and earlier, 4 bytes
// for DWARF32, 8 bytes for DWARF32 in DWARF 3 and later
@@ -175,7 +182,8 @@ static FormSize g_form_sizes[] = {
{1, 8}, // 0x14 DW_FORM_ref8
{0, 0}, // 0x15 DW_FORM_ref_udata
{0, 0}, // 0x16 DW_FORM_indirect
- {1, 4}, // 0x17 DW_FORM_sec_offset
+ {0,
+ 0}, // 0x17 DW_FORM_sec_offset (4 bytes for DWARF32, 8 bytes for DWARF64)
{0, 0}, // 0x18 DW_FORM_exprloc
{1, 0}, // 0x19 DW_FORM_flag_present
{0, 0}, // 0x1a DW_FORM_strx (ULEB128)
@@ -183,7 +191,7 @@ static FormSize g_form_sizes[] = {
{1, 4}, // 0x1c DW_FORM_ref_sup4
{0, 0}, // 0x1d DW_FORM_strp_sup (4 bytes for DWARF32, 8 bytes for DWARF64)
{1, 16}, // 0x1e DW_FORM_data16
- {1, 4}, // 0x1f DW_FORM_line_strp
+ {0, 0}, // 0x1f DW_FORM_line_strp (4 bytes for DWARF32, 8 bytes for DWARF64)
{1, 8}, // 0x20 DW_FORM_ref_sig8
};
@@ -251,8 +259,12 @@ bool DWARFFormValue::SkipValue(dw_form_t form,
// get this wrong
if (unit->GetVersion() <= 2)
ref_addr_size = unit->GetAddressByteSize();
- else
- ref_addr_size = 4;
+ else {
+ if (unit->GetFormat() == DwarfFormat::DWARF32)
+ ref_addr_size = 4;
+ else if (unit->GetFormat() == DwarfFormat::DWARF64)
+ ref_addr_size = 8;
+ }
*offset_ptr += ref_addr_size;
return true;
@@ -288,7 +300,10 @@ bool DWARFFormValue::SkipValue(dw_form_t form,
case DW_FORM_sec_offset:
case DW_FORM_strp:
case DW_FORM_line_strp:
- *offset_ptr += 4;
+ if (unit->GetFormat() == DwarfFormat::DWARF32)
+ *offset_ptr += 4;
+ else if (unit->GetFormat() == DwarfFormat::DWARF64)
+ *offset_ptr += 8;
return true;
// 4 byte values
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index ffd6f1dd52aff..f216ab13e8936 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -1073,20 +1073,7 @@ const lldb_private::DWARFDataExtractor &DWARFUnit::GetData() const {
: m_dwarf.GetDWARFContext().getOrLoadDebugInfoData();
}
-uint32_t DWARFUnit::GetHeaderByteSize() const {
- switch (m_header.getUnitType()) {
- case llvm::dwarf::DW_UT_compile:
- case llvm::dwarf::DW_UT_partial:
- return GetVersion() < 5 ? 11 : 12;
- case llvm::dwarf::DW_UT_skeleton:
- case llvm::dwarf::DW_UT_split_compile:
- return 20;
- case llvm::dwarf::DW_UT_type:
- case llvm::dwarf::DW_UT_split_type:
- return GetVersion() < 5 ? 23 : 24;
- }
- llvm_unreachable("invalid UnitType.");
-}
+uint32_t DWARFUnit::GetHeaderByteSize() const { return m_header.getSize(); }
std::optional<uint64_t>
DWARFUnit::GetStringOffsetSectionItem(uint32_t index) const {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index c05bba36ed74b..9f53d7dcdef53 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -119,6 +119,7 @@ class DWARFUnit : public DWARFExpression::Delegate, public UserID {
// Size of the CU data incl. header but without initial length.
dw_offset_t GetLength() const { return m_header.getLength(); }
uint16_t GetVersion() const override { return m_header.getVersion(); }
+ llvm::dwarf::DwarfFormat GetFormat() const { return m_header.getFormat(); }
const llvm::DWARFAbbreviationDeclarationSet *GetAbbreviations() const;
dw_offset_t GetAbbrevOffset() const;
uint8_t GetAddressByteSize() const override {
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- lldb/unittests/SymbolFile/DWARF/DWARF64UnitTest.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.hView the diff from clang-format here.diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index 2e98e3c33..8fc3bf8b5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -168,22 +168,22 @@ static FormSize g_form_sizes[] = {
{0, 0}, // 0x0f DW_FORM_udata
{0, 0}, // 0x10 DW_FORM_ref_addr (addr size for DWARF2 and earlier, 4 bytes
// for DWARF32, 8 bytes for DWARF32 in DWARF 3 and later
- {1, 1}, // 0x11 DW_FORM_ref1
- {1, 2}, // 0x12 DW_FORM_ref2
- {1, 4}, // 0x13 DW_FORM_ref4
- {1, 8}, // 0x14 DW_FORM_ref8
- {0, 0}, // 0x15 DW_FORM_ref_udata
- {0, 0}, // 0x16 DW_FORM_indirect
+ {1, 1}, // 0x11 DW_FORM_ref1
+ {1, 2}, // 0x12 DW_FORM_ref2
+ {1, 4}, // 0x13 DW_FORM_ref4
+ {1, 8}, // 0x14 DW_FORM_ref8
+ {0, 0}, // 0x15 DW_FORM_ref_udata
+ {0, 0}, // 0x16 DW_FORM_indirect
{0, 0}, // 0x17 DW_FORM_sec_offset (4 bytes for DWARF32,8 bytes for DWARF64)
- {0, 0}, // 0x18 DW_FORM_exprloc
- {1, 0}, // 0x19 DW_FORM_flag_present
- {0, 0}, // 0x1a DW_FORM_strx (ULEB128)
- {0, 0}, // 0x1b DW_FORM_addrx (ULEB128)
- {1, 4}, // 0x1c DW_FORM_ref_sup4
- {0, 0}, // 0x1d DW_FORM_strp_sup (4 bytes for DWARF32, 8 bytes for DWARF64)
+ {0, 0}, // 0x18 DW_FORM_exprloc
+ {1, 0}, // 0x19 DW_FORM_flag_present
+ {0, 0}, // 0x1a DW_FORM_strx (ULEB128)
+ {0, 0}, // 0x1b DW_FORM_addrx (ULEB128)
+ {1, 4}, // 0x1c DW_FORM_ref_sup4
+ {0, 0}, // 0x1d DW_FORM_strp_sup (4 bytes for DWARF32, 8 bytes for DWARF64)
{1, 16}, // 0x1e DW_FORM_data16
{0, 0}, // 0x1f DW_FORM_line_strp (4 bytes for DWARF32, 8 bytes for DWARF64)
- {1, 8}, // 0x20 DW_FORM_ref_sig8
+ {1, 8}, // 0x20 DW_FORM_ref_sig8
};
std::optional<uint8_t> DWARFFormValue::GetFixedSize(dw_form_t form,
|
|
@labath @DavidSpickett |
HemangGadhavi
force-pushed
the
dwarf64_support
branch
from
e0ea9bb to
547716f
Compare
HemangGadhavi
force-pushed
the
dwarf64_support
branch
from
547716f to
a85d648
Compare
Michael137
reviewed
Jun 25, 2025
Michael137
reviewed
Jun 25, 2025
There was a problem hiding this comment.
Also wondering about testing this. What tools can produce DWARF64?
Have you ran check-lldb with DWARF64 enabled and what results did you get?
I doubt anyone has spare capacity to add a whole new bot for this, but perhaps we would be ok with a few new tests that force DWARF64. Covering the main differences.
@Michael137 |
Start by looking at the main differences DWARF64 has. Write DWARF64 specific tests that cover those corner cases e.g. that some symbol can now be described as a greater size or have a greater offset, whatever it is. I'm assuming the answer isn't "everything changes", because the changes in this PR are very minimal. Run Anything that fails there, reduce the failure down to a targeted test case that can be run anywhere without having to run the whole test suite with DWARF64 enabled. So if you fail to set a breakpoint because the symbol lookup didn't work, reduce that to a test that constructs the DWARF64 data manually and does a symbol lookup. Then we end up with DWARF64 specific tests that can run on all the existing buildbots. |
@DavidSpickett answer to your question, we can produce
Thanks for the suggestion, I will start with the unittest by enabling the |
|
Clang only supports DWARF64 for ELF: So the existing Linux bots could run these tests. You can check the test compiler being used and see if it supports the option, that's more flexible than requiring it to be on Linux. |
I'm surprised that is the case, but I don't think it matters because I think you should be able to test all of this stuff without ever running a binary (which means you can build an elf target regardless of the host platform). That said, I think it would be best to avoid using clang for this sort of stuff, as very hard to tell what it actually tests. Like, you can't tell by looking at the c++ source code whether the resulting dwarf will use (e.g.) DW_FORM_line_strp or not. I'd recommend using assembly or dwarf yaml for that. E.g. you can take one of the simple tests from test/Shell/SymbolFile/DWARF/x86 and modify the assembly so it generates dwarf64. |
|
Also note that there are remnants of our previous attempt at dwarf64 support (which was deleted many years ago) lying around the DWARFDataExtractor class. However, since this isn't how llvm's DWARFDataExtractor works, I think we should not be using that (instead, we should delete it). LLVM's current approach is kind of similar to what you're doing actually, but instead of peppering the code with |
|
+1 on testing and Pavel's suggestion to use assembly/yaml for it so it can run everywhere. |
|
@labath @DavidSpickett @JDevlieghere @Michael137 I have addressed all the review comments please verify once again.
@labath I have added the On the Testing front, I have created the unit test case for Please review once and give your feedback, also let me know is there any additional testing required for the same ? Please Note that, clang-format is failling but not because of this PR changes but because of some existing format. |
labath
approved these changes
Jun 30, 2025
|
@DhruvSrivastavaX can you please merge it |
Michael137
reviewed
Jun 30, 2025
|
@HemangGadhavi , Please check the code format failure and let me have a check before merging. |
Michael137
reviewed
Jun 30, 2025
Michael137
reviewed
Jun 30, 2025
Michael137
reviewed
Jun 30, 2025
HemangGadhavi
force-pushed
the
dwarf64_support
branch
from
265e791 to
edeb253
Compare
JDevlieghere
approved these changes
Jun 30, 2025
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
|
Assuming there is not a lot more work to be done for DWARF64, this needs a release note - https://github.com/llvm/llvm-project/blob/main/llvm/docs/ReleaseNotes.md#changes-to-lldb. You can do that in this PR or a follow up. |
Updated with this PR only, Please review once @DavidSpickett Thanks! |
HemangGadhavi
force-pushed
the
dwarf64_support
branch
from
fc508f4 to
f622d2b
Compare
|
This has approvals already and looks good to me, do you need someone to merge it? |
DavidSpickett
approved these changes
Jul 1, 2025
|
Please go ahead and merge this, or I can do so for you if you're happy with it. |
DavidSpickett
changed the title
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/22423 Here is the relevant piece of the build log for the reference |
|
The above result is a flaky text, the next build was green so you can ignore it. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/10333 Here is the relevant piece of the build log for the reference |
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
This PR introduces support for the DWARF64 format, enabling handling of 64-bit DWARF sections as defined by the DWARF specification. The update includes adjustments to header parsing and modification of form values to accommodate 64-bit offsets and values. Also Added the testcase to verify the DWARF64 format.
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Jul 1, 2025
Follow-up to llvm#145645 (comment) There's no need for this variable to be declared at the function-level. We reset it in all the cases where it's used anyway.
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
This PR introduces support for the DWARF64 format, enabling handling of 64-bit DWARF sections as defined by the DWARF specification. The update includes adjustments to header parsing and modification of form values to accommodate 64-bit offsets and values. Also Added the testcase to verify the DWARF64 format.
DavidSpickett
added a commit
to DavidSpickett/llvm-project
that referenced
this pull request
Jul 2, 2025
Not that we ever do that, because this is unused code, but if someone was debugging lldb I guess they'd call this. Was missed in llvm#145645 Relates to llvm#135208
Michael137
added a commit
that referenced
this pull request
Jul 3, 2025
Follow-up to #145645 (comment) There's no need for this variable to be declared at the function-level. We reset it in all the cases where it's used anyway. This patch just inlines the usage of the variable entirely.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jul 3, 2025
…(#146557) Follow-up to llvm/llvm-project#145645 (comment) There's no need for this variable to be declared at the function-level. We reset it in all the cases where it's used anyway. This patch just inlines the usage of the variable entirely.
DavidSpickett
added a commit
that referenced
this pull request
Jul 3, 2025
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jul 3, 2025
…ddr (#146686) Not that we ever do that, because this is unused code, but if someone was debugging lldb I guess they'd call this. Was missed in llvm/llvm-project#145645 Relates to llvm/llvm-project#135208
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces support for the DWARF64 format, enabling handling of 64-bit DWARF sections as defined by the DWARF specification. The update includes adjustments to header parsing and modification of form values to accommodate 64-bit offsets and values.
Also Added the testcase to verify the DWARF64 format.