[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/include/lldb/Expression/DWARFExpression.h:142
 
+  std::pair<lldb::addr_t, lldb::addr_t> GetLocationListAddresses() const;
+
----------------
Might be better as two functions? One to get the CU address and func address:

```
lldb_private::Address GetCUAddress();
lldb_private::Address GetFuncAddress();
```

The DWARFExpression has a Module when the location comes from DWARF and this can be used to get the arch and sanitize the address by calling GetOpcodeLoadAddress on the address before returning it. 


================
Comment at: lldb/include/lldb/Utility/RangeMap.h:247-250
+  Entry *GetMutableEntryAtIndex(size_t i) {
+    return ((i < m_entries.size()) ? &m_entries[i] : nullptr);
+  }
+
----------------
Remove if we make DWARFRangeList handle this as mentioned in inline comment


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:102-106
+std::pair<addr_t, addr_t> DWARFExpression::GetLocationListAddresses() const {
+  return std::make_pair(m_loclist_addresses->cu_file_addr,
+                        m_loclist_addresses->func_file_addr);
+}
+
----------------
Convert to:

```
lldb_private::Address DWARFExpression::GetCUAddress();
lldb_private::Address DWARFExpression::GetFuncAddress();
```



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:397-400
+  for (size_t i = 0; i < ranges.GetSize(); i++) {
+    DWARFRangeList::Entry *entry = ranges.GetMutableEntryAtIndex(i);
+    entry->base = arch.GetOpcodeLoadAddress(entry->base);
+  }
----------------
What about asking the DWARFRangeList to fix all code addresses? This code would become:

```
ranges.GetOpcodeAddresses(arch);
```




================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:401-405
+  if (frame_base && frame_base->IsLocationList()) {
+    std::pair<lldb::addr_t, lldb::addr_t> loclist = frame_base->GetLocationListAddresses();
+    loclist.second = arch.GetOpcodeLoadAddress(loclist.second);
+    frame_base->SetLocationListAddresses(loclist.first, loclist.second);
+  }
----------------
Remove if we add:
```
lldb_private::Address DWARFExpression::GetCUAddress();
lldb_private::Address DWARFExpression::GetFuncAddress();
```



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:776-779
+    for (size_t i = 0; i < ranges.GetSize(); i++) {
+      DWARFRangeList::Entry *entry = ranges.GetMutableEntryAtIndex(i);
+      entry->base = arch.GetOpcodeLoadAddress(entry->base);
+    }
----------------
Call new DWARFRangeList::GetOpcodeAddresses?

```
ranges.GetOpcodeAddresses(arch);
```


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

https://reviews.llvm.org/D70840