aykevl created this revision.
aykevl added reviewers: granata.enrico, labath, dylanmckay, Andrzej.
Herald added subscribers: llvm-commits, lldb-commits, hiraditya.
Herald added projects: LLDB, LLVM.
Addresses are usually two bytes in size on AVR, so make sure LLDB deals with
that.
---
I'm not sure I caught all cases. In particular,
lldb/include/lldb/DataFormatters/FormattersHelpers.h might be related but I've
left it alone as it didn't seem to be necessary.
Honestly I think LLDB would become a lot more forgiving to uncommon
architectures when these asserts are removed and avoids assumptions based on
the address size. When the address size is relevant (for example, to read a
value from memory), it should have an exhaustive test with a default case that
does an assert (for easy debugging). For example:
switch (addr_size) {
case 4:
// do one thing
case 8:
// do something else
default:
assert(false && "unknown addr_size");
}
This way, it's easy to find the places that make these assumptions just by
following the asserts.
I'm not sure how to add a test for this. I can test it locally by connecting to
a remote debugger (simavr <https://github.com/buserror/simavr>). However, I
don't know how to do something like that in the LLDB testing framework.
Additionally, for that to work I only needed the change in
DumpDataExtractor.cpp so I'm not sure how to exhaustively test this.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73961
Files:
lldb/source/Core/DumpDataExtractor.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Symbol/DWARFCallFrameInfo.cpp
lldb/source/Utility/DataExtractor.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp
Index: llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp
===================================================================
--- llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp
+++ llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp
@@ -131,7 +131,9 @@
Data.setAddressSize(HeaderData.AddrSize);
uint32_t AddrCount = DataSize / HeaderData.AddrSize;
for (uint32_t I = 0; I < AddrCount; ++I)
- if (HeaderData.AddrSize == 4)
+ if (HeaderData.AddrSize == 2)
+ Addrs.push_back(Data.getU16(OffsetPtr));
+ else if (HeaderData.AddrSize == 4)
Addrs.push_back(Data.getU32(OffsetPtr));
else
Addrs.push_back(Data.getU64(OffsetPtr));
Index: lldb/source/Utility/DataExtractor.cpp
===================================================================
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -133,7 +133,7 @@
m_end(const_cast<uint8_t *>(static_cast<const uint8_t *>(data)) + length),
m_byte_order(endian), m_addr_size(addr_size), m_data_sp(),
m_target_byte_size(target_byte_size) {
- assert(addr_size == 4 || addr_size == 8);
+ assert(addr_size == 2 || addr_size == 4 || addr_size == 8);
}
// Make a shared pointer reference to the shared data in "data_sp" and set the
@@ -146,7 +146,7 @@
: m_start(nullptr), m_end(nullptr), m_byte_order(endian),
m_addr_size(addr_size), m_data_sp(),
m_target_byte_size(target_byte_size) {
- assert(addr_size == 4 || addr_size == 8);
+ assert(addr_size == 2 || addr_size == 4 || addr_size == 8);
SetData(data_sp);
}
@@ -160,7 +160,7 @@
: m_start(nullptr), m_end(nullptr), m_byte_order(data.m_byte_order),
m_addr_size(data.m_addr_size), m_data_sp(),
m_target_byte_size(target_byte_size) {
- assert(m_addr_size == 4 || m_addr_size == 8);
+ assert(m_addr_size == 2 || m_addr_size == 4 || m_addr_size == 8);
if (data.ValidOffset(offset)) {
offset_t bytes_available = data.GetByteSize() - offset;
if (length > bytes_available)
@@ -173,7 +173,7 @@
: m_start(rhs.m_start), m_end(rhs.m_end), m_byte_order(rhs.m_byte_order),
m_addr_size(rhs.m_addr_size), m_data_sp(rhs.m_data_sp),
m_target_byte_size(rhs.m_target_byte_size) {
- assert(m_addr_size == 4 || m_addr_size == 8);
+ assert(m_addr_size == 2 || m_addr_size == 4 || m_addr_size == 8);
}
// Assignment operator
@@ -251,7 +251,7 @@
offset_t data_offset,
offset_t data_length) {
m_addr_size = data.m_addr_size;
- assert(m_addr_size == 4 || m_addr_size == 8);
+ assert(m_addr_size == 2 || m_addr_size == 4 || m_addr_size == 8);
// If "data" contains shared pointer to data, then we can use that
if (data.m_data_sp) {
m_byte_order = data.m_byte_order;
@@ -679,12 +679,12 @@
//
// RETURNS the address that was extracted, or zero on failure.
uint64_t DataExtractor::GetAddress(offset_t *offset_ptr) const {
- assert(m_addr_size == 4 || m_addr_size == 8);
+ assert(m_addr_size == 2 || m_addr_size == 4 || m_addr_size == 8);
return GetMaxU64(offset_ptr, m_addr_size);
}
uint64_t DataExtractor::GetAddress_unchecked(offset_t *offset_ptr) const {
- assert(m_addr_size == 4 || m_addr_size == 8);
+ assert(m_addr_size == 2 || m_addr_size == 4 || m_addr_size == 8);
return GetMaxU64_unchecked(offset_ptr, m_addr_size);
}
@@ -695,7 +695,7 @@
//
// RETURNS the pointer that was extracted, or zero on failure.
uint64_t DataExtractor::GetPointer(offset_t *offset_ptr) const {
- assert(m_addr_size == 4 || m_addr_size == 8);
+ assert(m_addr_size == 2 || m_addr_size == 4 || m_addr_size == 8);
return GetMaxU64(offset_ptr, m_addr_size);
}
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===================================================================
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -39,7 +39,7 @@
uint64_t baseAddress = 0;
uint64_t addressValue = 0;
const uint32_t addr_size = DE.GetAddressByteSize();
- assert(addr_size == 4 || addr_size == 8);
+ assert(addr_size == 2 || addr_size == 4 || addr_size == 8);
bool signExtendValue = false;
// Decode the base part or adjust our offset
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -809,7 +809,7 @@
bool length_OK = data.ValidOffset(header.GetNextUnitOffset() - 1);
bool version_OK = SymbolFileDWARF::SupportedVersion(header.m_version);
- bool addr_size_OK = (header.m_addr_size == 4) || (header.m_addr_size == 8);
+ bool addr_size_OK = (header.m_addr_size == 2) || (header.m_addr_size == 4) || (header.m_addr_size == 8);
bool type_offset_OK =
!header.IsTypeUnit() || (header.m_type_offset <= header.GetLength());
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
@@ -68,7 +68,7 @@
return llvm::make_error<llvm::object::GenericBinaryError>(
"Invalid arange header version");
- if (m_header.addr_size != 4 && m_header.addr_size != 8)
+ if (m_header.addr_size != 2 && m_header.addr_size != 4 && m_header.addr_size != 8)
return llvm::make_error<llvm::object::GenericBinaryError>(
"Invalid arange header address size");
Index: lldb/source/Core/DumpDataExtractor.cpp
===================================================================
--- lldb/source/Core/DumpDataExtractor.cpp
+++ lldb/source/Core/DumpDataExtractor.cpp
@@ -141,7 +141,7 @@
return start_offset;
if (item_format == eFormatPointer) {
- if (item_byte_size != 4 && item_byte_size != 8)
+ if (item_byte_size != 2 && item_byte_size != 4 && item_byte_size != 8)
item_byte_size = s->GetAddressByteSize();
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
