labath added a comment. I like the direction this is going. I have one small comment about the code organization inline, but the bigger question I have in mind is: After these changes, is there any use for `Host::GetProcessBaseAddress` left? If so, what is it? I'd like to understand it and remove it.
================ Comment at: source/Plugins/Process/Windows/Common/ProcessWindows.cpp:868 +DynamicLoader *ProcessWindows::GetDynamicLoader() { + if (m_dyld_ap.get() == NULL) ---------------- You could have this function return `DynamicLoaderWindowsDYLD *` (thanks to the covariant return type thingy), and avoid having to cast the result in a bunch of places. ================ Comment at: source/Plugins/Process/Windows/Common/ProcessWindows.cpp:1039-1044 ModuleSP module = GetTarget().GetSharedModule(module_spec, &error); - bool load_addr_changed = false; - module->SetLoadAddress(GetTarget(), module_addr, false, load_addr_changed); - - ModuleList loaded_modules; - loaded_modules.Append(module); - GetTarget().ModulesDidLoad(loaded_modules); + if (error.Success()) { + auto dyld = static_cast<DynamicLoaderWindowsDYLD *>(GetDynamicLoader()); + assert(dyld); + dyld->OnLoadModule(module, module_addr); + } ---------------- On other platforms, it is the responsibility of the DYLD plugin to add the module to the target list (i.e. call Target::GetSharedModule or something equivalent). I think it would be more consistent to move this code there too. At that point you may just delete `ProcessWindows::OnLoadDll` and have the debugger thread call `process_windows->GetDynamicLoader()->OnLoadDLL()` directly. The same goes for module UnloadDLL. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56237/new/ https://reviews.llvm.org/D56237 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits