[LLDB][NFC] Added the interface DWARFExpression::Delegate to break dependencies and reduce lldb-server size #131645
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
…ies and reduce lldb-server size This patch addresses the issue llvm#129543. After this patch DWARFExpression does not call DWARFUnit directly and does not depend on lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp and a lot of clang code. After this patch the size of lldb-server binary (Linux Aarch64) is reduced from 42MB to 13MB with LLVM 20.0.0 and from 47MB to 17MB with LLVM 21.0.0.
|
@llvm/pr-subscribers-lldb Author: Dmitry Vasilyev (slydiman) ChangesThis patch addresses the issue #129543. After this patch DWARFExpression does not call DWARFUnit directly and does not depend on lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp and a lot of clang code. After this patch the size of lldb-server binary (Linux Aarch64) is reduced from 42MB to 13MB with LLVM 20.0.0 Full diff: https://github.com/llvm/llvm-project/pull/131645.diff 5 Files Affected:
diff --git a/lldb/include/lldb/Expression/DWARFExpression.h b/lldb/include/lldb/Expression/DWARFExpression.h
index 2c1e717ee32eb..cf4098f2acc51 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -23,7 +23,7 @@ namespace lldb_private {
namespace plugin {
namespace dwarf {
-class DWARFUnit;
+class DWARFUnitInterface;
} // namespace dwarf
} // namespace plugin
@@ -65,20 +65,20 @@ class DWARFExpression {
/// \return
/// The address specified by the operation, if the operation exists, or
/// an llvm::Error otherwise.
- llvm::Expected<lldb::addr_t>
- GetLocation_DW_OP_addr(const plugin::dwarf::DWARFUnit *dwarf_cu) const;
+ llvm::Expected<lldb::addr_t> GetLocation_DW_OP_addr(
+ const plugin::dwarf::DWARFUnitInterface *dwarf_cu) const;
- bool Update_DW_OP_addr(const plugin::dwarf::DWARFUnit *dwarf_cu,
+ bool Update_DW_OP_addr(const plugin::dwarf::DWARFUnitInterface *dwarf_cu,
lldb::addr_t file_addr);
void UpdateValue(uint64_t const_value, lldb::offset_t const_value_byte_size,
uint8_t addr_byte_size);
- bool
- ContainsThreadLocalStorage(const plugin::dwarf::DWARFUnit *dwarf_cu) const;
+ bool ContainsThreadLocalStorage(
+ const plugin::dwarf::DWARFUnitInterface *dwarf_cu) const;
bool LinkThreadLocalStorage(
- const plugin::dwarf::DWARFUnit *dwarf_cu,
+ const plugin::dwarf::DWARFUnitInterface *dwarf_cu,
std::function<lldb::addr_t(lldb::addr_t file_addr)> const
&link_address_callback);
@@ -132,13 +132,14 @@ class DWARFExpression {
static llvm::Expected<Value>
Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
lldb::ModuleSP module_sp, const DataExtractor &opcodes,
- const plugin::dwarf::DWARFUnit *dwarf_cu,
+ const plugin::dwarf::DWARFUnitInterface *dwarf_cu,
const lldb::RegisterKind reg_set, const Value *initial_value_ptr,
const Value *object_address_ptr);
- static bool ParseDWARFLocationList(const plugin::dwarf::DWARFUnit *dwarf_cu,
- const DataExtractor &data,
- DWARFExpressionList *loc_list);
+ static bool
+ ParseDWARFLocationList(const plugin::dwarf::DWARFUnitInterface *dwarf_cu,
+ const DataExtractor &data,
+ DWARFExpressionList *loc_list);
bool GetExpressionData(DataExtractor &data) const {
data = m_data;
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index f48f3ab9307dd..41fbca59db60f 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -133,7 +133,7 @@ static llvm::Error ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
const lldb::offset_t data_offset,
const LocationAtom op,
- const DWARFUnit *dwarf_cu) {
+ const DWARFUnitInterface *dwarf_cu) {
lldb::offset_t offset = data_offset;
switch (op) {
// Only used in LLVM metadata.
@@ -362,7 +362,8 @@ static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
// + LEB128
{
data.Skip_LEB128(&offset);
- return DWARFUnit::GetAddressByteSize(dwarf_cu) + offset - data_offset;
+ return DWARFUnitInterface::GetAddressByteSize(dwarf_cu) + offset -
+ data_offset;
}
case DW_OP_GNU_entry_value:
@@ -393,8 +394,8 @@ static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
return LLDB_INVALID_OFFSET;
}
-llvm::Expected<lldb::addr_t>
-DWARFExpression::GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu) const {
+llvm::Expected<lldb::addr_t> DWARFExpression::GetLocation_DW_OP_addr(
+ const DWARFUnitInterface *dwarf_cu) const {
lldb::offset_t offset = 0;
while (m_data.ValidOffset(offset)) {
const LocationAtom op = static_cast<LocationAtom>(m_data.GetU8(&offset));
@@ -422,7 +423,7 @@ DWARFExpression::GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu) const {
return LLDB_INVALID_ADDRESS;
}
-bool DWARFExpression::Update_DW_OP_addr(const DWARFUnit *dwarf_cu,
+bool DWARFExpression::Update_DW_OP_addr(const DWARFUnitInterface *dwarf_cu,
lldb::addr_t file_addr) {
lldb::offset_t offset = 0;
while (m_data.ValidOffset(offset)) {
@@ -481,7 +482,7 @@ bool DWARFExpression::Update_DW_OP_addr(const DWARFUnit *dwarf_cu,
}
bool DWARFExpression::ContainsThreadLocalStorage(
- const DWARFUnit *dwarf_cu) const {
+ const DWARFUnitInterface *dwarf_cu) const {
lldb::offset_t offset = 0;
while (m_data.ValidOffset(offset)) {
const LocationAtom op = static_cast<LocationAtom>(m_data.GetU8(&offset));
@@ -497,7 +498,7 @@ bool DWARFExpression::ContainsThreadLocalStorage(
return false;
}
bool DWARFExpression::LinkThreadLocalStorage(
- const DWARFUnit *dwarf_cu,
+ const DWARFUnitInterface *dwarf_cu,
std::function<lldb::addr_t(lldb::addr_t file_addr)> const
&link_address_callback) {
const uint32_t addr_byte_size = m_data.GetAddressByteSize();
@@ -783,7 +784,8 @@ enum LocationDescriptionKind {
/* Composite*/
};
/// Adjust value's ValueType according to the kind of location description.
-void UpdateValueTypeFromLocationDescription(Log *log, const DWARFUnit *dwarf_cu,
+void UpdateValueTypeFromLocationDescription(Log *log,
+ const DWARFUnitInterface *dwarf_cu,
LocationDescriptionKind kind,
Value *value = nullptr) {
// Note that this function is conflating DWARF expressions with
@@ -875,7 +877,7 @@ static Scalar DerefSizeExtractDataHelper(uint8_t *addr_bytes,
llvm::Expected<Value> DWARFExpression::Evaluate(
ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
lldb::ModuleSP module_sp, const DataExtractor &opcodes,
- const DWARFUnit *dwarf_cu, const lldb::RegisterKind reg_kind,
+ const DWARFUnitInterface *dwarf_cu, const lldb::RegisterKind reg_kind,
const Value *initial_value_ptr, const Value *object_address_ptr) {
if (opcodes.GetByteSize() == 0)
@@ -2164,35 +2166,10 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
if (!bit_size)
return llvm::createStringError("unspecified architecture");
} else {
- // Retrieve the type DIE that the value is being converted to. This
- // offset is compile unit relative so we need to fix it up.
- const uint64_t abs_die_offset = die_offset + dwarf_cu->GetOffset();
- // FIXME: the constness has annoying ripple effects.
- DWARFDIE die = const_cast<DWARFUnit *>(dwarf_cu)->GetDIE(abs_die_offset);
- if (!die)
- return llvm::createStringError(
- "cannot resolve DW_OP_convert type DIE");
- uint64_t encoding =
- die.GetAttributeValueAsUnsigned(DW_AT_encoding, DW_ATE_hi_user);
- bit_size = die.GetAttributeValueAsUnsigned(DW_AT_byte_size, 0) * 8;
- if (!bit_size)
- bit_size = die.GetAttributeValueAsUnsigned(DW_AT_bit_size, 0);
- if (!bit_size)
- return llvm::createStringError(
- "unsupported type size in DW_OP_convert");
- switch (encoding) {
- case DW_ATE_signed:
- case DW_ATE_signed_char:
- sign = true;
- break;
- case DW_ATE_unsigned:
- case DW_ATE_unsigned_char:
- sign = false;
- break;
- default:
- return llvm::createStringError(
- "unsupported encoding in DW_OP_convert");
- }
+ if (llvm::Error err =
+ const_cast<DWARFUnitInterface *>(dwarf_cu)->GetBitSizeAndSign(
+ die_offset, bit_size, sign))
+ return err;
}
Scalar &top = stack.back().ResolveValue(exe_ctx);
top.TruncOrExtendTo(bit_size, sign);
@@ -2353,7 +2330,7 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
}
bool DWARFExpression::ParseDWARFLocationList(
- const DWARFUnit *dwarf_cu, const DataExtractor &data,
+ const DWARFUnitInterface *dwarf_cu, const DataExtractor &data,
DWARFExpressionList *location_list) {
location_list->Clear();
std::unique_ptr<llvm::DWARFLocationTable> loctable_up =
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index fd3d45cef4c5e..c2f4cc6a24af7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -44,8 +44,8 @@ bool DWARFFormValue::ExtractValue(const DWARFDataExtractor &data,
switch (m_form) {
case DW_FORM_addr:
assert(m_unit);
- m_value.uval =
- data.GetMaxU64(offset_ptr, DWARFUnit::GetAddressByteSize(m_unit));
+ m_value.uval = data.GetMaxU64(
+ offset_ptr, DWARFUnitInterface::GetAddressByteSize(m_unit));
break;
case DW_FORM_block1:
m_value.uval = data.GetU8(offset_ptr);
@@ -242,7 +242,7 @@ bool DWARFFormValue::SkipValue(dw_form_t form,
// Compile unit address sized values
case DW_FORM_addr:
- *offset_ptr += DWARFUnit::GetAddressByteSize(unit);
+ *offset_ptr += DWARFUnitInterface::GetAddressByteSize(unit);
return true;
case DW_FORM_ref_addr:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 1ceeef76f7cc3..168b134b84e2f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -672,6 +672,37 @@ DWARFUnit::GetDIE(dw_offset_t die_offset) {
return DWARFDIE(); // Not found
}
+llvm::Error DWARFUnit::GetBitSizeAndSign(uint64_t die_offset,
+ uint64_t &bit_size, bool &sign) {
+ // Retrieve the type DIE that the value is being converted to. This
+ // offset is compile unit relative so we need to fix it up.
+ const uint64_t abs_die_offset = die_offset + GetOffset();
+ // FIXME: the constness has annoying ripple effects.
+ DWARFDIE die = GetDIE(abs_die_offset);
+ if (!die)
+ return llvm::createStringError("cannot resolve DW_OP_convert type DIE");
+ uint64_t encoding =
+ die.GetAttributeValueAsUnsigned(DW_AT_encoding, DW_ATE_hi_user);
+ bit_size = die.GetAttributeValueAsUnsigned(DW_AT_byte_size, 0) * 8;
+ if (!bit_size)
+ bit_size = die.GetAttributeValueAsUnsigned(DW_AT_bit_size, 0);
+ if (!bit_size)
+ return llvm::createStringError("unsupported type size in DW_OP_convert");
+ switch (encoding) {
+ case DW_ATE_signed:
+ case DW_ATE_signed_char:
+ sign = true;
+ break;
+ case DW_ATE_unsigned:
+ case DW_ATE_unsigned_char:
+ sign = false;
+ break;
+ default:
+ return llvm::createStringError("unsupported encoding in DW_OP_convert");
+ }
+ return llvm::Error::success();
+}
+
llvm::StringRef DWARFUnit::PeekDIEName(dw_offset_t die_offset) {
DWARFDebugInfoEntry die;
if (!die.Extract(GetData(), *this, &die_offset))
@@ -703,14 +734,6 @@ DWARFUnit &DWARFUnit::GetNonSkeletonUnit() {
return *this;
}
-uint8_t DWARFUnit::GetAddressByteSize(const DWARFUnit *cu) {
- if (cu)
- return cu->GetAddressByteSize();
- return DWARFUnit::GetDefaultAddressSize();
-}
-
-uint8_t DWARFUnit::GetDefaultAddressSize() { return 4; }
-
DWARFCompileUnit *DWARFUnit::GetSkeletonUnit() {
if (m_skeleton_unit.load() == nullptr && IsDWOUnit()) {
SymbolFileDWARFDwo *dwo =
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index ba142ae86fe0e..8d0bf51208108 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -38,7 +38,34 @@ enum DWARFProducer {
eProducerOther
};
-class DWARFUnit : public UserID {
+class DWARFUnitInterface {
+public:
+ DWARFUnitInterface() = default;
+ virtual ~DWARFUnitInterface() = default;
+
+ virtual SymbolFileDWARF &GetSymbolFileDWARF() const = 0;
+ virtual dw_addr_t ReadAddressFromDebugAddrSection(uint32_t index) const = 0;
+ virtual uint16_t GetVersion() const = 0;
+ virtual std::unique_ptr<llvm::DWARFLocationTable>
+ GetLocationTable(const DataExtractor &data) const = 0;
+ virtual dw_addr_t GetBaseAddress() const = 0;
+ virtual uint8_t GetAddressByteSize() const = 0;
+ virtual llvm::Error GetBitSizeAndSign(uint64_t die_offset, uint64_t &bit_size,
+ bool &sign) = 0;
+
+ static uint8_t GetAddressByteSize(const DWARFUnitInterface *cu) {
+ if (cu)
+ return cu->GetAddressByteSize();
+ return GetDefaultAddressSize();
+ }
+
+ static uint8_t GetDefaultAddressSize() { return 4; }
+
+ DWARFUnitInterface(const DWARFUnitInterface &) = delete;
+ DWARFUnitInterface &operator=(const DWARFUnitInterface &) = delete;
+};
+
+class DWARFUnit : public UserID, public DWARFUnitInterface {
using die_iterator_range =
llvm::iterator_range<DWARFDebugInfoEntry::collection::iterator>;
@@ -116,12 +143,14 @@ class DWARFUnit : public UserID {
size_t GetDebugInfoSize() const;
// Size of the CU data incl. header but without initial length.
dw_offset_t GetLength() const { return m_header.getLength(); }
- uint16_t GetVersion() const { return m_header.getVersion(); }
+ uint16_t GetVersion() const override { return m_header.getVersion(); }
const llvm::DWARFAbbreviationDeclarationSet *GetAbbreviations() const;
dw_offset_t GetAbbrevOffset() const;
- uint8_t GetAddressByteSize() const { return m_header.getAddressByteSize(); }
+ uint8_t GetAddressByteSize() const override {
+ return m_header.getAddressByteSize();
+ }
dw_addr_t GetAddrBase() const { return m_addr_base.value_or(0); }
- dw_addr_t GetBaseAddress() const { return m_base_addr; }
+ dw_addr_t GetBaseAddress() const override { return m_base_addr; }
dw_offset_t GetLineTableOffset();
dw_addr_t GetRangesBase() const { return m_ranges_base; }
dw_addr_t GetStrOffsetsBase() const { return m_str_offsets_base; }
@@ -131,7 +160,7 @@ class DWARFUnit : public UserID {
void SetStrOffsetsBase(dw_offset_t str_offsets_base);
virtual void BuildAddressRangeTable(DWARFDebugAranges *debug_aranges) = 0;
- dw_addr_t ReadAddressFromDebugAddrSection(uint32_t index) const;
+ dw_addr_t ReadAddressFromDebugAddrSection(uint32_t index) const override;
lldb::ByteOrder GetByteOrder() const;
@@ -145,6 +174,9 @@ class DWARFUnit : public UserID {
DWARFDIE GetDIE(dw_offset_t die_offset);
+ llvm::Error GetBitSizeAndSign(uint64_t die_offset, uint64_t &bit_size,
+ bool &sign) override;
+
/// Returns the AT_Name of the DIE at `die_offset`, if it exists, without
/// parsing the entire compile unit. An empty is string is returned upon
/// error or if the attribute is not present.
@@ -152,10 +184,6 @@ class DWARFUnit : public UserID {
DWARFUnit &GetNonSkeletonUnit();
- static uint8_t GetAddressByteSize(const DWARFUnit *cu);
-
- static uint8_t GetDefaultAddressSize();
-
lldb_private::CompileUnit *GetLLDBCompUnit() const { return m_lldb_cu; }
void SetLLDBCompUnit(lldb_private::CompileUnit *cu) { m_lldb_cu = cu; }
@@ -174,7 +202,7 @@ class DWARFUnit : public UserID {
bool Supports_unnamed_objc_bitfields();
- SymbolFileDWARF &GetSymbolFileDWARF() const { return m_dwarf; }
+ SymbolFileDWARF &GetSymbolFileDWARF() const override { return m_dwarf; }
DWARFProducer GetProducer();
@@ -237,7 +265,7 @@ class DWARFUnit : public UserID {
/// Return the location table for parsing the given location list data. The
/// format is chosen according to the unit type. Never returns null.
std::unique_ptr<llvm::DWARFLocationTable>
- GetLocationTable(const DataExtractor &data) const;
+ GetLocationTable(const DataExtractor &data) const override;
DWARFDataExtractor GetLocationData() const;
|
labath
reviewed
Mar 18, 2025
There was a problem hiding this comment.
I think something like this makes sense, but if we're doing this, I think we should do it properly and actually remove the DWARFExpression->SymbolFileDWARF dependency. We can't do that if DWARFUnitInterface is defined in the symbol file plugin, nor while it contains methods like GetSymbolFileDWARF(). Moving the interface definition elsewhere is easy. The second part requires some thought. It looks like the main use for that method is to provide vendor extensibility for dwarf parsing, so I think we should expose that as an interface (i.e., have methods like ParseVendorDWARFOpcodeSize and GetVendorDWARFOpcodeSize) instead of the raw DWARF symbol file.
For this reason, I also think the interface shouldn't be called DWARFUnitInterface, but rather something centred on DWARFExpression and the things it needs from its users. For lack of imagination, I'm going to suggest DWARFExpression::Delegate (because we already use that term in some places), but maybe we'll come up with something more specific in the process. (This means it may not make sense to DWARFUnit to inherit from DWARFExpression::Delegate, but that's okay -- we can always use composition instead)
I also want to look at each function in that interface and see if it's really necessary. Two examples:
- GetLocationTable is only used from ParseDWARFLocationList and that is only used from DWARF plugin code. Instead of adding this to the interface, I think we should move ParseDWARFLocationList into the DWARF plugin. (This can/should be a separate patch)
- I'm not sure that
GetAddressByteSizeis necessary since the DataExtractor member (m_data) already has a method with the same name (and it's even used in some places). Before adding that to the interface, I'd like to see if we can make everything use the value in m_data instead.
Moved ParseDWARFLocationList to DWARF plugin.
slydiman
changed the title
|
After this patch and #132274 the size of lldb-server is reduced from 47MB to 8MB. |
labath
reviewed
Mar 21, 2025
Comment on lines
46
to
48
| virtual llvm::Error GetDIEBitSizeAndSign(uint64_t die_offset, | ||
| uint64_t &bit_size, | ||
| bool &sign) = 0; |
There was a problem hiding this comment.
Suggested change
| virtual llvm::Error GetDIEBitSizeAndSign(uint64_t die_offset, | |
| uint64_t &bit_size, | |
| bool &sign) = 0; | |
| virtual llvm::Expected<std::pair<uint64_t, bool>> GetDIEBitSizeAndSign(uint64_t relative_die_offset) const = 0; |
(I'm prioritizing the interface over implementation, so I'm making the method const. You can put the const_cast into the implementation (I think it's only needed for GetDIE. I am also emphasizing the relativeness of the die offset as lldb default is to use absolute section offsets)
| if (!bit_size) | ||
| bit_size = die.GetAttributeValueAsUnsigned(DW_AT_bit_size, 0); | ||
| if (!bit_size) | ||
| return llvm::createStringError("unsupported type size in DW_OP_convert"); |
There was a problem hiding this comment.
Suggested change
| return llvm::createStringError("unsupported type size in DW_OP_convert"); | |
| return llvm::createStringError("unsupported type size"); |
Let's not mention DW_OP_convert here. (same below)
Comment on lines
744
to
746
| bool DWARFUnit::ParseDWARFLocationList( | ||
| const DataExtractor &data, DWARFExpressionList *location_list) const { | ||
| location_list->Clear(); |
There was a problem hiding this comment.
Let's change this to a reference while we're here.
Suggested change
| bool DWARFUnit::ParseDWARFLocationList( | |
| const DataExtractor &data, DWARFExpressionList *location_list) const { | |
| location_list->Clear(); | |
| bool DWARFUnit::ParseDWARFLocationList( | |
| const DataExtractor &data, DWARFExpressionList &location_list) const { | |
| location_list.Clear(); |
| @@ -38,7 +39,7 @@ enum DWARFProducer { | |||
| eProducerOther | |||
| }; | |||
|
|
|||
| class DWARFUnit : public UserID { | |||
| class DWARFUnit : public UserID, public DWARFExpression::Delegate { | |||
There was a problem hiding this comment.
Suggested change
| class DWARFUnit : public UserID, public DWARFExpression::Delegate { | |
| class DWARFUnit : public DWARFExpression::Delegate, public UserID { |
(because Delegate is a "true" base class (with virtual methods and whatnot) while UserID is a mixin)
labath
reviewed
Mar 21, 2025
| @@ -37,7 +36,6 @@ | |||
| #include "lldb/Target/StackID.h" | |||
| #include "lldb/Target/Target.h" | |||
| #include "lldb/Target/Thread.h" | |||
| #include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h" | |||
| #include "llvm/DebugInfo/DWARF/DWARFExpression.h" | |||
|
|
|||
| #include "Plugins/SymbolFile/DWARF/DWARFUnit.h" | |||
There was a problem hiding this comment.
Does all this mean this header can be removed now?
There was a problem hiding this comment.
Yes, because the code which is required this header has been moved to DWARFUnit.cpp
There was a problem hiding this comment.
Could you remove it then? :)
labath
approved these changes
Mar 24, 2025
| @@ -37,7 +36,6 @@ | |||
| #include "lldb/Target/StackID.h" | |||
| #include "lldb/Target/Target.h" | |||
| #include "lldb/Target/Thread.h" | |||
| #include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h" | |||
| #include "llvm/DebugInfo/DWARF/DWARFExpression.h" | |||
|
|
|||
| #include "Plugins/SymbolFile/DWARF/DWARFUnit.h" | |||
There was a problem hiding this comment.
Could you remove it then? :)
…ression.cpp Moved DW_OP_value_to_name() from DWARFDefines.cpp to DWARFExpression.cpp
|
@labath |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/14835 Here is the relevant piece of the build log for the reference |
basioli-k
added a commit
that referenced
this pull request
Mar 24, 2025
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 25, 2025
…tion In rdar://147772462 a merge conflict occured due to this upstream commit: ``` commit 2edf534 Author: Dmitry Vasilyev <[email protected]> Date: Mon Mar 24 18:40:49 2025 +0400 [LLDB][NFC] Added the interface DWARFExpression::Delegate to break dependencies and reduce lldb-server size (llvm#131645) ``` I resolved the conflict but it failed to build. This patch unbreaks the build but only when `LLDB_ENABLE_SWIFT` is OFF. This will hopefully be enough to unblock the automergers. The build will almost certainly be broken when `LLDB_ENABLE_SWIFT` is ON but I don't understand the code well enough to fix it correctly so the LLDB folks will need to properly resolve this in a follow up patch. rdar://147795421
|
Should this also change the interface of DWAFExpressionList? |
|
This type of patch also slightly worries me in that it could make it harder to unify the LLDB dwarf data structures with the LLVM ones |
Yes, I will prepare the patch. Thanks |
slydiman
added a commit
to slydiman/llvm-project
that referenced
this pull request
Mar 26, 2025
…ExpressionList too This is an update for llvm#131645.
That is something I considered, but I think this might actually help with that. At this point the LLDB and llvm dwarf classes are so different, that the only way I see to unify them is to do it piecemeal, and this kind of decoupling helps with that. So, if you wanted to replace LLDB DWARFUnit with llvm's, you could do that without touching DWARFExpression. It's true lldb's DWARFUnit now inherits from some DWARFExpression thingy, but that's easy to replace with composition. I alluded to that in the earlier comments, but in the end I didn't insist on it. Maybe I should have. If you think it's better to do that now, I'd be certainly fine with that. This doesn't help with replacing LLDB's DWARFExpression with the llvm version, but I don't think it makes it worse either. I see a couple of possible ways forwards there:
|
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 patch addresses the issue #129543.
After this patch DWARFExpression does not call DWARFUnit directly and does not depend on lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp and a lot of clang code.
After this patch the size of lldb-server binary (Linux Aarch64) is reduced from 42MB to 13MB with LLVM 20.0.0
and from 47MB to 17MB with LLVM 21.0.0.