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

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo marked 2 inline comments as done.
mstorsjo added inline comments.


================
Comment at: lldb/include/lldb/Expression/DWARFExpression.h:142
 
+  std::pair<lldb::addr_t, lldb::addr_t> GetLocationListAddresses() const;
+
----------------
clayborg wrote:
> 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. 
Oh, if we'd make DWARFExpression::SetLocationListAddresses do the sanitization, then we don't need to add the getters at all - that'd save a lot of extra changes as well.


================
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);
+  }
----------------
clayborg wrote:
> What about asking the DWARFRangeList to fix all code addresses? This code would become:
> 
> ```
> ranges.GetOpcodeAddresses(arch);
> ```
> 
> 
Yeah, I was thinking of some refactoring like that. I would have gone for a method that mutates the current range, e.g. `ranges.FixOpcodeAddresses(arch)`, but would you prefer it to be something that returns a new sanitized range instead?


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

https://reviews.llvm.org/D70840