clayborg added a comment.
Sorry for the late comments. Looks like it got checked in already, but will
submit inline comments anyway.
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:963
+static llvm::Optional<lldb::addr_t>
+ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp,
+ Status *error_ptr, const char *dw_op_type,
----------------
Pass the module_sp in as a reference to avoid the increment/decrement on the
reference count?
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:963
+static llvm::Optional<lldb::addr_t>
+ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp,
+ Status *error_ptr, const char *dw_op_type,
----------------
clayborg wrote:
> Pass the module_sp in as a reference to avoid the increment/decrement on the
> reference count?
Maybe "ResolveLoadAddress" would be a better name?
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:971
+ "need module to resolve file address for %s", dw_op_type);
+ return {};
+ }
----------------
Do we prefer "return {}" over "return llvm::None"?
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:983
+ if (load_addr == LLDB_INVALID_ADDRESS &&
+ (check_sectionoffset && !so_addr.IsSectionOffset())) {
+ if (error_ptr)
----------------
Not sure if you need to do this as the call to
"module_sp->ResolveFileAddress(...) will return false if the address wasn't
resolved to a address that is section offset.
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:1006-1017
+ switch (size) {
+ case 1:
+ return addr_data.GetU8(&addr_data_offset);
+ case 2:
+ return addr_data.GetU16(&addr_data_offset);
+ case 4:
+ return addr_data.GetU32(&addr_data_offset);
----------------
DataExtractor already has a GetMaxU64() call you can use. It handles anything
<= 8 bytes in size. Do we really want to handle the "default:" case above or
the "else" clause in my suggested fix though?
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:1102
frame->CalculateTarget().get());
+
break;
----------------
revert
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121408/new/
https://reviews.llvm.org/D121408
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
