jankratochvil added a comment. In D63868#1563802 <https://reviews.llvm.org/D63868#1563802>, @labath wrote:
> Does this mean that there is a bug in lldb-server, where some memory read > requests fail if they span an unreadable page. Can this also be triggered by > a $m packet? Would you be interested in distilling that into a test? OK, I will try to reproduce it - you are right GDB documentation <https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#Packets> says for the `m` packet: "//The reply may contain fewer addressable memory units than requested if the server was able to read only part of the region of memory. //" And LLDB is using `ReadMemory` in `NativeProcessProtocol::ReadMemoryWithoutTrap` from `GDBRemoteCommunicationServerLLGS::Handle_memory_read`. > I think it would be better to do the loading inside the DynamicLoader > plugin, and have `ProcessGDBRemote::LoadModules` be responsible only for > sending the appropriate qXfer packet and parsing the response. The fact that > loading the libraries from a `LoadedModuleInfoList` is largely similar for > posix&windows can be handled (at a later date) by factoring the common code > into a utility function, a common base class or something else... IIUC you believe this patch should be more refactored? I do not have much overview of this code of LLDB (+Windows libraries in general). IIUC you propose: - The current libraries loading code from `LoadedModuleInfoList` of `ProcessGDBRemote::LoadModules` moved to `lldb/source/Plugins/Platform/Windows/` - later: `DynamicLoaderPOSIXDYLD::RefreshModules` and `DynamicLoaderPOSIXDYLD::LoadAllCurrentModules` somehow reusing that code (after it is moved back to some common library)? But that looks to me as too little code to make it worth it. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63868/new/ https://reviews.llvm.org/D63868 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
