[Lldb-commits] [PATCH] D61587: [lldb] Added support for dwarf expressions DW_OP_call2/DW_OP_call4

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.


================
Comment at: include/lldb/Expression/DWARFExpression.h:307
                        const lldb::RegisterKind reg_set,
                        const Value *initial_value_ptr,
+                       const Value *object_address_ptr,
----------------
Do we need "initial_value_ptr" anymore? We should be able to remove it and just use the "stack" argument you added below. Probably best to just turn this value into:

```
std::vector<Value> &stack
```

And remove the "std::vector<Value> &stack" below.


================
Comment at: source/Expression/DWARFExpression.cpp:2695
+    case DW_OP_call2: {
+      dw_offset_t die_ref_offset = opcodes.GetU16(&offset);
+      const DWARFDIE ref_die = dwarf_cu->GetDIE(die_ref_offset);
----------------
die_ref_offset us a CU relative offset. You must add "dwarf_cu->GetOffset()" to this to get the correct absolute DIE offset:

```
dw_offset_t die_ref_offset = opcodes.GetU16(&offset) + dwarf_cu->GetOffset();
```
Your example will work for the first compile unit and fail for any subsequent ones since the first CU is at offset zero.


================
Comment at: source/Expression/DWARFExpression.cpp:2703-2723
+      DWARFAttributes attributes;
+      const size_t num_attrib = ref_die.GetAttributes(attributes);
+      if (num_attrib == 0)
+        break;
+
+      const uint32_t attr_index = attributes.FindAttributeIndex(DW_AT_location);
+      if (attr_index == UINT32_MAX)
----------------
Make these lines into a function of DWARFExpression and call it for call2 and call4:

```
... DWARFExpression::EvaluateCall(DWARFDie Die, ....)
```


================
Comment at: source/Expression/DWARFExpression.cpp:2746
+    case DW_OP_call4: {
+      dw_offset_t die_ref_offset = opcodes.GetU32(&offset);
+      const DWARFDIE ref_die = dwarf_cu->GetDIE(die_ref_offset);
----------------
add CU offset like in call2 case.


================
Comment at: source/Expression/DWARFExpression.cpp:2754-2774
+      DWARFAttributes attributes;
+      const size_t num_attrib = ref_die.GetAttributes(attributes);
+      if (num_attrib == 0)
+        break;
+
+      const uint32_t attr_index = attributes.FindAttributeIndex(DW_AT_location);
+      if (attr_index == UINT32_MAX)
----------------
call new DWARFExpression::EvaluateCall() function you created as noted in above inlined comment.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61587/new/

https://reviews.llvm.org/D61587