[Lldb-commits] [lldb] f292ca1 - [lldb][NFC] Simplify GetLocation_DW_OP_addr function

Felipe de Azevedo Piovezan via lldb-commits lldb-commits at lists.llvm.org
Mon Jul 3 05:39:21 PDT 2023


Author: Felipe de Azevedo Piovezan
Date: 2023-07-03T08:36:57-04:00
New Revision: f292ca136240a9c694aa280ad7fd0b961930804a

URL: https://github.com/llvm/llvm-project/commit/f292ca136240a9c694aa280ad7fd0b961930804a
DIFF: https://github.com/llvm/llvm-project/commit/f292ca136240a9c694aa280ad7fd0b961930804a.diff

LOG: [lldb][NFC] Simplify GetLocation_DW_OP_addr function

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.

Differential Revision: https://reviews.llvm.org/D154265

Added: 
    

Modified: 
    lldb/include/lldb/Expression/DWARFExpression.h
    lldb/source/Expression/DWARFExpression.cpp
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Expression/DWARFExpression.h b/lldb/include/lldb/Expression/DWARFExpression.h
index aea41926d848d2..380910ba0ea3d6 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -50,30 +50,22 @@ class DWARFExpression {
   /// 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);
 

diff  --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 5f71a12456972e..c9524870f316f4 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -355,39 +355,28 @@ static offset_t GetOpcodeDataSize(const DataExtractor &data,
 }
 
 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;
 }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index dc98e5cf4f5783..e0c0d2f48ec17c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3457,8 +3457,8 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
       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);


        


More information about the lldb-commits mailing list