labath added inline comments.
================
Comment at: lldb/include/lldb/Expression/DWARFExpression.h:260
- bool GetLocation(lldb::addr_t func_load_addr, lldb::addr_t pc,
- lldb::offset_t &offset, lldb::offset_t &len);
+ void RelocateLowHighPC(lldb::addr_t load_function_start, lldb::addr_t
&low_pc,
+ lldb::addr_t &high_pc) const;
----------------
JDevlieghere wrote:
> Should this be part of the DWARFExpression API? It feels like a static
> function might be sufficient. Maybe it could take a range directly?
It's a private member function, so it's not really a part of any API. However,
given that now it's called from only a single place, it's not actually useful.
It served a purpose in the preceding patch (I'm going to need a stamp on that
too BTW), since there it had like five callers, but now that this patch
centralizes them, I can just inline it.
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:53
+ if (data.ValidOffsetForDataOfSize(offset, index_size))
+ return data.GetMaxU64_unchecked(&offset, index_size);
+ return LLDB_INVALID_ADDRESS;
----------------
JDevlieghere wrote:
> Why the unchecked?
Because I've already checked that the offset is valid (that's the difference
between the checked and unchecked versions, though I'm not really sure if we
need them).
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:57
+
+static std::unique_ptr<llvm::DWARFLocationTable>
+GetLocationTable(DWARFExpression::LocationListFormat format, const
DataExtractor &data) {
----------------
aprantl wrote:
> Doxygen comment?
I've added /something/, though I think the purpose is mostly obvious here.
Btw, this part is going to get refactored further, since the current design
doesn't really permit mixing DWARF 4&5 location lists within a single object
file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71003/new/
https://reviews.llvm.org/D71003
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
