zequanwu added a comment.
> The main thing I am wondering is if it wouldn't be better to (instead of
> essentially duplicating the DWARFExpression interface in the
> DWARFExpressionList class) somehow split the process of finding the
> appropriate expression (for the given PC value or whatever), and the actual
> evaluation. This would be particularly nice if the arguments can be split in
> such a way that those that are used for locating the expression are not
> repeated in the "evaluate" call. Have you considered this?
Then we need to duplicate the code that gets expression data and register kind
before every call to `DWARFExpression::Evaluate`, making it less clean.
> Also, if I understand correctly, one of the side effects of this patch that
> the DWARF location list is now parsed more eagerly (when the containing
> object (e.g. variable) is constructed rather than during evaluation time). I
> think that's probably fine, but it's definitely worth calling out.
Yes, the dwarf location list now is parsed at the time when parsing variable
DIE. It used to be that a dwarf location list will be parsed every time when we
try to access a dwarf expression inside.
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:2581
-static DataExtractor ToDataExtractor(const llvm::DWARFLocationExpression &loc,
- ByteOrder byte_order, uint32_t addr_size)
{
- auto buffer_sp =
- std::make_shared<DataBufferHeap>(loc.Expr.data(), loc.Expr.size());
- return DataExtractor(buffer_sp, byte_order, addr_size);
-}
-
-bool DWARFExpression::DumpLocations(Stream *s, lldb::DescriptionLevel level,
- addr_t load_function_start, addr_t addr,
- ABI *abi) {
- if (!IsLocationList()) {
- DumpLocation(s, m_data, level, abi);
- return true;
- }
- bool dump_all = addr == LLDB_INVALID_ADDRESS;
- llvm::ListSeparator separator;
- auto callback = [&](llvm::DWARFLocationExpression loc) -> bool {
- if (loc.Range &&
- (dump_all || (loc.Range->LowPC <= addr && addr < loc.Range->HighPC))) {
- uint32_t addr_size = m_data.GetAddressByteSize();
- DataExtractor data = ToDataExtractor(loc, m_data.GetByteOrder(),
- m_data.GetAddressByteSize());
- s->AsRawOstream() << separator;
- s->PutCString("[");
- s->AsRawOstream() << llvm::format_hex(loc.Range->LowPC,
- 2 + 2 * addr_size);
- s->PutCString(", ");
- s->AsRawOstream() << llvm::format_hex(loc.Range->HighPC,
- 2 + 2 * addr_size);
- s->PutCString(") -> ");
- DumpLocation(s, data, level, abi);
- return dump_all;
- }
- return true;
- };
- if (!GetLocationExpressions(load_function_start, callback))
- return false;
- return true;
-}
-
-bool DWARFExpression::GetLocationExpressions(
- addr_t load_function_start,
- llvm::function_ref<bool(llvm::DWARFLocationExpression)> callback) const {
- if (load_function_start == LLDB_INVALID_ADDRESS)
- return false;
-
- Log *log = GetLog(LLDBLog::Expressions);
-
+bool DWARFExpression::ParseDWARFLocationList(
+ const DWARFUnit *dwarf_cu, const DataExtractor &data,
----------------
labath wrote:
> Could this go into the list class?
This function calls `ReadAddressFromDebugAddrSection` that is called multiple
places in `DWARFExpression`. If we move it to `DWARFExpressionList`, we need to
duplicate the `ReadAddressFromDebugAddrSection`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125509/new/
https://reviews.llvm.org/D125509
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
