Aj0SK added inline comments.
================
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:549
+ bool is_interesting = false;
+ for (size_t interesting_address : interesting_addresses)
+ if (interesting_address >= addr && interesting_address < addr + size) {
----------------
clayborg wrote:
> Shouldn't this be the main loop that would replace the "while
> (range_info.GetRange().GetRangeBase() != LLDB_INVALID_ADDRESS)" loop? I would
> think that the main loop wiould be something like:
>
> ```
> std::set<addr_t> visited_region_base_addresses;
> for (size_t interesting_address : interesting_addresses) {
> error = process_sp->GetMemoryRegionInfo(interesting_address, range_info);
> // Skip failed memory region requests or any regions with no permissions.
> if (error.Fail() || range_info.GetPermissions() == 0)
> continue;
> const addr_t addr = range_info.GetRange().GetRangeBase();
> // Skip any regions we have already saved out.
> if (visited_region_base_addresses.insert(addr).second == false)
> continue;
> const addr_t size = range_info.GetRange().GetByteSize();
> if (size == 0)
> continue;
> auto data_up = std::make_unique<DataBufferHeap>(size, 0);
> const size_t bytes_read = process_sp->ReadMemory(addr, data_up->GetBytes(),
> size, error);
> if (bytes_read == 0)
> continue;
> // We have a good memory region with valid bytes to store.
> LocationDescriptor memory_dump;
> memory_dump.DataSize = static_cast<llvm::support::ulittle32_t>(size);
> memory_dump.RVA
> =static_cast<llvm::support::ulittle32_t>(GetCurrentDataEndOffset());
> MemoryDescriptor memory_desc;
> memory_desc.StartOfMemoryRange =
> static_cast<llvm::support::ulittle64_t>(addr);
> memory_desc.Memory = memory_dump;
> mem_descriptors.push_back(memory_desc);
> m_data.AppendData(data_up->GetBytes(), bytes_read);
> }
> ```
>
>
This kind of an approach was something I also thought about and in fact, I was
mainly inspired by this [[
https://github.com/llvm-mirror/lldb/blob/d01083a850f577b85501a0902b52fd0930de72c7/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp#L6051
| approach ]]. Also, the previous comment about the need of saving the loaded
modules addresses in a set doesn't apply to my solution, am I correct? Just to
clear it a little bit, I like your solution though and it's better when I think
about it. Thanks for the suggestion!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
