[DRAFT][lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

[fdeazeve]

Summary

This uses a DenseMap to update addresses that are located in the debug_addr
section.
This is posted here for analysis in the context of https://discourse.llvm.org/t/dwarfexpression-and-dw-op-addrx/71627

[fdeazeve]

This comment was removed by fdeazeve.

[fdeazeve]

We're going with the other solution. See discourse thread linked

lldb/include/lldb/Expression/DWARFExpression.h

 #define LLDB_EXPRESSION_DWARFEXPRESSION_H
 
 #include "lldb/Core/Address.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Scalar.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/DebugInfo/DWARF/DWARFLocationExpression.h"
 #include <functional>
 
 class DWARFUnit;
 
 namespace lldb_private {
 
 /// \class DWARFExpression DWARFExpression.h
public:
   ///     If non-NULL, used to report errors in expression evaluation.
   ///
   /// \return
   ///     True on success; false otherwise.  If error_ptr is non-NULL,
   ///     details of the failure are provided through it.
   static bool Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
                        lldb::ModuleSP module_sp, const DataExtractor &opcodes,
                        const DWARFUnit *dwarf_cu,
+                       const llvm::DenseMap<uint32_t, lldb::addr_t> *addrx_map,
                        const lldb::RegisterKind reg_set,
                        const Value *initial_value_ptr,
                        const Value *object_address_ptr, Value &result,
                        Status *error_ptr);
 
   static bool ParseDWARFLocationList(const DWARFUnit *dwarf_cu,
                                      const DataExtractor &data,
                                      DWARFExpressionList *loc_list);
 
   bool GetExpressionData(DataExtractor &data) const {
     data = m_data;
     return data.GetByteSize() > 0;
   }
 
+  const llvm::DenseMap<uint32_t, lldb::addr_t> &GetAddrxMap() const {
+    return m_addrx_map;
+  }
+
   void DumpLocation(Stream *s, lldb::DescriptionLevel level, ABI *abi) const;
 
   bool MatchesOperand(StackFrame &frame, const Instruction::Operand &op) const;
 
 private:
   /// A data extractor capable of reading opcode bytes
   DataExtractor m_data;
 
+  llvm::DenseMap<uint32_t, lldb::addr_t> m_addrx_map;
+
   /// One of the defines that starts with LLDB_REGKIND_
   lldb::RegisterKind m_reg_kind = lldb::eRegisterKindDWARF;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_EXPRESSION_DWARFEXPRESSION_H

lldb/source/Expression/DWARFExpression.cpp

if (op == DW_OP_addr) {
       ++curr_op_addr_idx;
     } else 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 (auto it = m_addrx_map.find(index); it != m_addrx_map.end())
+          return it->getSecond();
         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;
if (op == DW_OP_addr) {
       // Replace the address in the new buffer
       if (encoder.PutAddress(offset, file_addr) == UINT32_MAX)
         return false;
 
       // All went well, so now we can reset the data using a shared pointer to
       // the heap data so "m_data" will now correctly manage the heap data.
       m_data.SetData(encoder.GetDataBuffer());
       return true;
-    } else {
-      const offset_t op_arg_size =
-          GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
-      if (op_arg_size == LLDB_INVALID_OFFSET)
-        break;
-      offset += op_arg_size;
-    }
-  }
+    }
+    if (op == DW_OP_addrx) {
+      lldb::offset_t old_offset = offset;
+      uint64_t index = m_data.GetULEB128(&offset);
+      if (old_offset == offset)
+        return false;
+      m_addrx_map[index] = file_addr;
+      return true;
+    }
   return false;
 }
 
 bool DWARFExpression::ContainsThreadLocalStorage(
     const DWARFUnit *dwarf_cu) const {
   lldb::offset_t offset = 0;
   while (m_data.ValidOffset(offset)) {
     const uint8_t op = m_data.GetU8(&offset);
if (size <= 8)
     return addr_data.GetMaxU64(&addr_data_offset, size);
   else
     return addr_data.GetAddress(&addr_data_offset);
 }
 
 bool 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 Value *initial_value_ptr, const Value *object_address_ptr,
-    Value &result, Status *error_ptr) {
+    const DWARFUnit *dwarf_cu,
+    const llvm::DenseMap<uint32_t, lldb::addr_t> *addrx_map,
+    const lldb::RegisterKind reg_kind, const Value *initial_value_ptr,
+    const Value *object_address_ptr, Value &result, Status *error_ptr) {
 
   if (opcodes.GetByteSize() == 0) {
     if (error_ptr)
       error_ptr->SetErrorString(
           "no location, value may have been optimized out");
     return false;
   }
   std::vector<Value> stack;

     case DW_OP_GNU_addr_index: {
       if (!dwarf_cu) {
         if (error_ptr)
           error_ptr->SetErrorString("DW_OP_GNU_addr_index found without a "
                                     "compile unit being specified");
         return false;
       }
       uint64_t index = opcodes.GetULEB128(&offset);
-      lldb::addr_t value = dwarf_cu->ReadAddressFromDebugAddrSection(index);
+      lldb::addr_t value = [&] {
+        if (addrx_map)
+          if (auto it = addrx_map->find(index); it != addrx_map->end())
+            return it->second;
+        return dwarf_cu->ReadAddressFromDebugAddrSection(index);
+      }();
       stack.push_back(Scalar(value));
       if (target &&
           target->GetArchitecture().GetCore() == ArchSpec::eCore_wasm32) {
         // wasm file sections aren't mapped into memory, therefore addresses can
         // never point into a file section and are always LoadAddresses.
         stack.back().SetValueType(Value::ValueType::LoadAddress);
       } else {
         stack.back().SetValueType(Value::ValueType::FileAddress);

lldb/source/Expression/DWARFExpressionList.cpp

if (!entry) {
         error_ptr->SetErrorString("variable not available");
       }
       return false;
     }
     expr = *entry;
   }
   expr.GetExpressionData(data);
   reg_kind = expr.GetRegisterKind();
-  return DWARFExpression::Evaluate(exe_ctx, reg_ctx, module_sp, data,
-                                   m_dwarf_cu, reg_kind, initial_value_ptr,
-                                   object_address_ptr, result, error_ptr);
+  return DWARFExpression::Evaluate(
+      exe_ctx, reg_ctx, module_sp, data, m_dwarf_cu, &expr.GetAddrxMap(),
+      reg_kind, initial_value_ptr, object_address_ptr, result, error_ptr);
 }

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

if (attributes.ExtractFormValueAtIndex(i, form_value)) {
           Value memberOffset(0);
           const DWARFDataExtractor &debug_info_data = die.GetData();
           uint32_t block_length = form_value.Unsigned();
           uint32_t block_offset =
               form_value.BlockData() - debug_info_data.GetDataStart();
           if (DWARFExpression::Evaluate(
                   nullptr, nullptr, module_sp,
                   DataExtractor(debug_info_data, block_offset, block_length),
-                  die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr,
+                  die.GetCU(), nullptr, eRegisterKindDWARF, &initialValue, nullptr,
                   memberOffset, nullptr)) {
             member_byte_offset = memberOffset.ResolveValue(nullptr).UInt();
           }
         } else {
           // With DWARF 3 and later, if the value is an integer constant,
           // this form value is the offset in bytes from the beginning of
           // the containing entity.
           member_byte_offset = form_value.Unsigned();
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
           uint32_t block_length = form_value.Unsigned();
           uint32_t block_offset =
               form_value.BlockData() - debug_info_data.GetDataStart();
           if (DWARFExpression::Evaluate(
                   nullptr, // ExecutionContext *
                   nullptr, // RegisterContext *
                   module_sp,
                   DataExtractor(debug_info_data, block_offset, block_length),
-                  die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr,
-                  memberOffset, nullptr)) {
+                  die.GetCU(), nullptr, eRegisterKindDWARF, &initialValue,
+                  nullptr, memberOffset, nullptr)) {
             member_byte_offset = memberOffset.ResolveValue(nullptr).UInt();
           }
         } else {
           // With DWARF 3 and later, if the value is an integer constant,
           // this form value is the offset in bytes from the beginning of
           // the containing entity.
           member_byte_offset = form_value.Unsigned();
         }

lldb/unittests/Expression/DWARFExpressionTest.cpp

 static llvm::Expected<Scalar> Evaluate(llvm::ArrayRef<uint8_t> expr,
                                        lldb::ModuleSP module_sp = {},
                                        DWARFUnit *unit = nullptr,
                                        ExecutionContext *exe_ctx = nullptr) {
   DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle,
                           /*addr_size*/ 4);
   Value result;
   Status status;
-  if (!DWARFExpression::Evaluate(exe_ctx, /*reg_ctx*/ nullptr, module_sp,
-                                 extractor, unit, lldb::eRegisterKindLLDB,
-                                 /*initial_value_ptr*/ nullptr,
-                                 /*object_address_ptr*/ nullptr, result,
-                                 &status))
+  if (!DWARFExpression::Evaluate(
+          exe_ctx, /*reg_ctx*/ nullptr, module_sp, extractor, unit, nullptr,
+          lldb::eRegisterKindLLDB,
+          /*object_address_ptr*/ nullptr, result, &status))
     return status.ToError();
 
   switch (result.GetValueType()) {
   case Value::ValueType::Scalar:
     return result.GetScalar();
   case Value::ValueType::LoadAddress:
     return LLDB_INVALID_ADDRESS;
   case Value::ValueType::HostAddress: {
TEST_F(DWARFExpressionMockProcessTest, WASM_DW_OP_addr) {
   // DW_OP_addr takes a single operand of address size width:
   uint8_t expr[] = {DW_OP_addr, 0x40, 0x0, 0x0, 0x0};
   DataExtractor extractor(expr, sizeof(expr), lldb::eByteOrderLittle,
                           /*addr_size*/ 4);
   Value result;
   Status status;
   ASSERT_TRUE(DWARFExpression::Evaluate(
       &exe_ctx, /*reg_ctx*/ nullptr, /*module_sp*/ {}, extractor,
-      /*unit*/ nullptr, lldb::eRegisterKindLLDB,
+      /*unit*/ nullptr, nullptr, lldb::eRegisterKindLLDB,
       /*initial_value_ptr*/ nullptr,
       /*object_address_ptr*/ nullptr, result, &status))
       << status.ToError();
 
   ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress);
 }
 
 TEST_F(DWARFExpressionMockProcessTest, WASM_DW_OP_addr_index) {
)";
   // DW_OP_addrx takes a single leb128 operand, the index in the addr table:
   uint8_t expr[] = {DW_OP_addrx, 0x01};
   DataExtractor extractor(expr, sizeof(expr), lldb::eByteOrderLittle,
                           /*addr_size*/ 4);
   Value result;
   Status status;
   ASSERT_TRUE(DWARFExpression::Evaluate(
       &exe_ctx, /*reg_ctx*/ nullptr, /*module_sp*/ {}, extractor, dwarf_cu,
-      lldb::eRegisterKindLLDB,
+      nullptr, lldb::eRegisterKindLLDB,
       /*initial_value_ptr*/ nullptr,
       /*object_address_ptr*/ nullptr, result, &status))
       << status.ToError();
 
   ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress);
   ASSERT_EQ(result.GetScalar().UInt(), 0x5678u);
 }