[lldb] Extract DW_OP_deref evaluation code (NFC)

[JDevlieghere]

The DWARF expression evaluator is essentially a large state machine implemented as a switch statement. For anything but trivial operations, having the evaluation logic implemented inline is difficult to follow, especially when there are nested switches. This commit moves evaluation of DW_OP_deref out-of-line, similar to Evaluate_DW_OP_entry_value.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

The DWARF expression evaluator is essentially a large state machine implemented as a switch statement. For anything but trivial operations, having the evaluation logic implemented inline is difficult to follow, especially when there are nested switches. This commit moves evaluation of DW_OP_deref out-of-line, similar to Evaluate_DW_OP_entry_value.

---

Full diff: https://github.com/llvm/llvm-project/pull/146801.diff

2 Files Affected:

• (modified) lldb/include/lldb/Expression/DWARFExpression.h (+3-1)
• (modified) lldb/source/Expression/DWARFExpression.cpp (+67-70)

diff --git a/lldb/include/lldb/Expression/DWARFExpression.h b/lldb/include/lldb/Expression/DWARFExpression.h
index 0adbe3e8df2ee..37853c0b5a8fc 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -35,6 +35,8 @@ namespace lldb_private {
 /// location expression or a location list and interprets it.
 class DWARFExpression {
 public:
+  using Stack = std::vector<Value>;
+
   class Delegate {
   public:
     Delegate() = default;
@@ -53,7 +55,7 @@ class DWARFExpression {
     virtual bool ParseVendorDWARFOpcode(uint8_t op,
                                         const DataExtractor &opcodes,
                                         lldb::offset_t &offset,
-                                        std::vector<Value> &stack) const = 0;
+                                        Stack &stack) const = 0;
 
     Delegate(const Delegate &) = delete;
     Delegate &operator=(const Delegate &) = delete;
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 2df27513a0b3f..0e7cd8ede6dfa 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -589,7 +589,7 @@ bool DWARFExpression::LinkThreadLocalStorage(
   return true;
 }
 
-static llvm::Error Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
+static llvm::Error Evaluate_DW_OP_entry_value(DWARFExpression::Stack &stack,
                                               ExecutionContext *exe_ctx,
                                               RegisterContext *reg_ctx,
                                               const DataExtractor &opcodes,
@@ -654,7 +654,7 @@ static llvm::Error Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
   addr_t return_pc = LLDB_INVALID_ADDRESS;
   uint32_t current_frame_idx = current_frame->GetFrameIndex();
 
-  for (uint32_t parent_frame_idx = current_frame_idx + 1;;parent_frame_idx++) {
+  for (uint32_t parent_frame_idx = current_frame_idx + 1;; parent_frame_idx++) {
     parent_frame = thread->GetStackFrameAtIndex(parent_frame_idx);
     // If this is null, we're at the end of the stack.
     if (!parent_frame)
@@ -860,6 +860,64 @@ ResolveLoadAddress(ExecutionContext *exe_ctx, lldb::ModuleSP &module_sp,
   return load_addr;
 }
 
+static llvm::Error Evaluate_DW_OP_deref(DWARFExpression::Stack &stack,
+                                        ExecutionContext *exe_ctx,
+                                        lldb::ModuleSP module_sp,
+                                        Process *process) {
+  if (stack.empty())
+    return llvm::createStringError("expression stack empty for DW_OP_deref");
+
+  const Value::ValueType value_type = stack.back().GetValueType();
+  switch (value_type) {
+  case Value::ValueType::HostAddress: {
+    void *src = (void *)stack.back().GetScalar().ULongLong();
+    intptr_t ptr;
+    ::memcpy(&ptr, src, sizeof(void *));
+    stack.back().GetScalar() = ptr;
+    stack.back().ClearContext();
+  } break;
+  case Value::ValueType::FileAddress: {
+    auto file_addr = stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
+    Address so_addr;
+    auto maybe_load_addr = ResolveLoadAddress(exe_ctx, module_sp, "DW_OP_deref",
+                                              file_addr, so_addr);
+    if (!maybe_load_addr)
+      return maybe_load_addr.takeError();
+    stack.back().GetScalar() = *maybe_load_addr;
+    // Fall through to load address promotion code below.
+  }
+    [[fallthrough]];
+  case Value::ValueType::Scalar:
+    // Promote Scalar to LoadAddress and fall through.
+    stack.back().SetValueType(Value::ValueType::LoadAddress);
+    [[fallthrough]];
+  case Value::ValueType::LoadAddress: {
+    if (!exe_ctx)
+      return llvm::createStringError("NULL execution context for DW_OP_deref");
+    if (!process)
+      return llvm::createStringError("NULL process for DW_OP_deref");
+    lldb::addr_t pointer_addr =
+        stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
+    Status error;
+    lldb::addr_t pointer_value =
+        process->ReadPointerFromMemory(pointer_addr, error);
+    if (pointer_value == LLDB_INVALID_ADDRESS)
+      return llvm::createStringError(
+          "Failed to dereference pointer from 0x%" PRIx64
+          " for DW_OP_deref: %s\n",
+          pointer_addr, error.AsCString());
+    if (ABISP abi_sp = process->GetABI())
+      pointer_value = abi_sp->FixCodeAddress(pointer_value);
+    stack.back().GetScalar() = pointer_value;
+    stack.back().ClearContext();
+  } break;
+  case Value::ValueType::Invalid:
+    return llvm::createStringError("invalid value type for DW_OP_deref");
+  }
+
+  return llvm::Error::success();
+}
+
 /// Helper function to move common code used to load sized data from a uint8_t
 /// buffer.
 ///
@@ -890,7 +948,8 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
   if (opcodes.GetByteSize() == 0)
     return llvm::createStringError(
         "no location, value may have been optimized out");
-  std::vector<Value> stack;
+
+  Stack stack;
 
   Process *process = nullptr;
   StackFrame *frame = nullptr;
@@ -1019,69 +1078,9 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
     // retrieved from the dereferenced address is the size of an address on the
     // target machine.
     case DW_OP_deref: {
-      if (stack.empty())
-        return llvm::createStringError(
-            "expression stack empty for DW_OP_deref");
-      Value::ValueType value_type = stack.back().GetValueType();
-      switch (value_type) {
-      case Value::ValueType::HostAddress: {
-        void *src = (void *)stack.back().GetScalar().ULongLong();
-        intptr_t ptr;
-        ::memcpy(&ptr, src, sizeof(void *));
-        stack.back().GetScalar() = ptr;
-        stack.back().ClearContext();
-      } break;
-      case Value::ValueType::FileAddress: {
-        auto file_addr = stack.back().GetScalar().ULongLong(
-            LLDB_INVALID_ADDRESS);
-
-        Address so_addr;
-        auto maybe_load_addr = ResolveLoadAddress(
-            exe_ctx, module_sp, "DW_OP_deref", file_addr, so_addr);
-
-        if (!maybe_load_addr)
-          return maybe_load_addr.takeError();
-
-        stack.back().GetScalar() = *maybe_load_addr;
-        // Fall through to load address promotion code below.
-      }
-        [[fallthrough]];
-      case Value::ValueType::Scalar:
-        // Promote Scalar to LoadAddress and fall through.
-        stack.back().SetValueType(Value::ValueType::LoadAddress);
-        [[fallthrough]];
-      case Value::ValueType::LoadAddress:
-        if (exe_ctx) {
-          if (process) {
-            lldb::addr_t pointer_addr =
-                stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
-            Status error;
-            lldb::addr_t pointer_value =
-                process->ReadPointerFromMemory(pointer_addr, error);
-            if (pointer_value != LLDB_INVALID_ADDRESS) {
-              if (ABISP abi_sp = process->GetABI())
-                pointer_value = abi_sp->FixCodeAddress(pointer_value);
-              stack.back().GetScalar() = pointer_value;
-              stack.back().ClearContext();
-            } else {
-              return llvm::createStringError(
-                  "Failed to dereference pointer from 0x%" PRIx64
-                  " for DW_OP_deref: %s\n",
-                  pointer_addr, error.AsCString());
-            }
-          } else {
-            return llvm::createStringError("NULL process for DW_OP_deref");
-          }
-        } else {
-          return llvm::createStringError(
-              "NULL execution context for DW_OP_deref");
-        }
-        break;
-
-      case Value::ValueType::Invalid:
-        return llvm::createStringError("invalid value type for DW_OP_deref");
-      }
-
+      if (llvm::Error err =
+              Evaluate_DW_OP_deref(stack, exe_ctx, module_sp, process))
+        return err;
     } break;
 
     // OPCODE: DW_OP_deref_size
@@ -1966,8 +1965,7 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
           case Value::ValueType::Scalar: {
             uint32_t bit_size = piece_byte_size * 8;
             uint32_t bit_offset = 0;
-            if (!scalar.ExtractBitfield(
-                    bit_size, bit_offset)) {
+            if (!scalar.ExtractBitfield(bit_size, bit_offset)) {
               return llvm::createStringError(
                   "unable to extract %" PRIu64 " bytes from a %" PRIu64
                   " byte scalar value.",
@@ -2409,8 +2407,7 @@ bool DWARFExpression::MatchesOperand(
     return MatchUnaryOp(
         MatchOpType(Instruction::Operand::Type::Dereference),
         MatchBinaryOp(MatchOpType(Instruction::Operand::Type::Sum),
-                      MatchRegOp(*reg),
-                      MatchImmOp(offset)))(operand);
+                      MatchRegOp(*reg), MatchImmOp(offset)))(operand);
   } else {
     return MatchRegOp(*reg)(operand);
   }