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 lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits