fdeazeve created this revision.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
A very old commit (9422dd64f870dd33) changed the signature of this function in a
number of ways. This patch aims to improve it:
1. Reword the documentation, which still mentions old parameters that no longer
exist, and to elaborate upon the behavior of this function.
2. Remove the unnecessary parameter `op_addr_idx`. This parameter is odd in a
couple of ways: we never use it with a value that is non-zero, and the matching
`Update_DW_OP_addr` function doesn't use a similar parameter. We also document
that this new behavior. If we ever decide to handle multiple "DW_OP_addr", we
can introduce the complexity again.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154265
Files:
lldb/include/lldb/Expression/DWARFExpression.h
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3457,8 +3457,8 @@
bool op_error = false;
const DWARFExpression* location = location_list.GetAlwaysValidExpr();
if (location)
- location_DW_OP_addr = location->GetLocation_DW_OP_addr(
- location_form.GetUnit(), 0, op_error);
+ location_DW_OP_addr =
+ location->GetLocation_DW_OP_addr(location_form.GetUnit(), op_error);
if (op_error) {
StreamString strm;
location->DumpLocation(&strm, eDescriptionLevelFull, nullptr);
Index: lldb/source/Expression/DWARFExpression.cpp
===================================================================
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -355,39 +355,28 @@
}
lldb::addr_t DWARFExpression::GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu,
- uint32_t op_addr_idx,
bool &error) const {
error = false;
lldb::offset_t offset = 0;
- uint32_t curr_op_addr_idx = 0;
while (m_data.ValidOffset(offset)) {
const uint8_t op = m_data.GetU8(&offset);
- if (op == DW_OP_addr) {
- const lldb::addr_t op_file_addr = m_data.GetAddress(&offset);
- if (curr_op_addr_idx == op_addr_idx)
- return op_file_addr;
- ++curr_op_addr_idx;
- } else if (op == DW_OP_GNU_addr_index || op == DW_OP_addrx) {
+ if (op == DW_OP_addr)
+ return m_data.GetAddress(&offset);
+ if (op == DW_OP_GNU_addr_index || op == DW_OP_addrx) {
uint64_t index = m_data.GetULEB128(&offset);
- if (curr_op_addr_idx == op_addr_idx) {
- if (!dwarf_cu) {
- error = true;
- break;
- }
-
+ if (dwarf_cu)
return dwarf_cu->ReadAddressFromDebugAddrSection(index);
- }
- ++curr_op_addr_idx;
- } else {
- const offset_t op_arg_size =
- GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
- if (op_arg_size == LLDB_INVALID_OFFSET) {
- error = true;
- break;
- }
- offset += op_arg_size;
+ error = true;
+ break;
+ }
+ const offset_t op_arg_size =
+ GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
+ if (op_arg_size == LLDB_INVALID_OFFSET) {
+ error = true;
+ break;
}
+ offset += op_arg_size;
}
return LLDB_INVALID_ADDRESS;
}
Index: lldb/include/lldb/Expression/DWARFExpression.h
===================================================================
--- lldb/include/lldb/Expression/DWARFExpression.h
+++ lldb/include/lldb/Expression/DWARFExpression.h
@@ -50,30 +50,22 @@
/// Return true if the location expression contains data
bool IsValid() const;
- /// If a location is not a location list, return true if the location
- /// contains a DW_OP_addr () opcode in the stream that matches \a file_addr.
- /// If file_addr is LLDB_INVALID_ADDRESS, the this function will return true
- /// if the variable there is any DW_OP_addr in a location that (yet still is
- /// NOT a location list). This helps us detect if a variable is a global or
- /// static variable since there is no other indication from DWARF debug
- /// info.
+ /// Return the address specified by the first
+ /// DW_OP_{addr, addrx, GNU_addr_index} in the operation stream.
///
/// \param[in] dwarf_cu
- /// The dwarf unit this expression belongs to.
- ///
- /// \param[in] op_addr_idx
- /// The DW_OP_addr index to retrieve in case there is more than
- /// one DW_OP_addr opcode in the location byte stream.
+ /// The dwarf unit this expression belongs to. Only required to resolve
+ /// DW_OP{addrx, GNU_addr_index}.
///
/// \param[out] error
/// If the location stream contains unknown DW_OP opcodes or the
/// data is missing, \a error will be set to \b true.
///
/// \return
- /// LLDB_INVALID_ADDRESS if the location doesn't contain a
- /// DW_OP_addr for \a op_addr_idx, otherwise a valid file address
+ /// The address specified by the operation, if the operation exists, or
+ /// LLDB_INVALID_ADDRESS otherwise.
lldb::addr_t GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu,
- uint32_t op_addr_idx, bool &error) const;
+ bool &error) const;
bool Update_DW_OP_addr(const DWARFUnit *dwarf_cu, lldb::addr_t file_addr);
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
