labath added a comment. In D72751#1848384 <https://reviews.llvm.org/D72751#1848384>, @paolosev wrote:
> Regarding: > > >> - make the base DynamicLoader class instantiatable, and use it whenever we > >> fail to find a specialized plugin > >> - same as above, but only do that for ProcessGDBRemote instances > >> - make ProcessGDBRemote call LoadModules() itself if no dynamic loader > >> instance is available WDYT? > > > > I am fine with 1 as long as we document the DynamicLoader class to say that > > it will call Process::LoadModules() and will be used if no specialized > > loader is needed for your platform. I would like to a see a solution that > > will work for any process plug-in and not just ProcessGDBRemote. If we > > change solution 3 above to say "Make lldb_private::Process call > > LoadModules() itself if no dynamic loader instance is available" then > > solution 3 is also fine. > > there is a problem: if I remove DynamicLoaderWasmDYLD what happens is that > `DynamicLoaderStatic` is found as a valid loader for a triple like > **"wasm32-unknown-unknown-wasm"** because the Triple::OS is > llvm::Triple::UnknownOS (I found out the hard way when I was registering > DynamicLoaderWasmDYLD after DynamicLoaderStatic :-)). > > ... > > > call stack: > DynamicLoaderStatic::CreateInstance(lldb_private::Process * process, bool > force) Line 29 > DynamicLoader::FindPlugin(lldb_private::Process * process, const char * > plugin_name) Line 52 > lldb_private::process_gdb_remote::ProcessGDBRemote::GetDynamicLoader() Line > 3993 > lldb_private::Process::CompleteAttach() Line 2931 > lldb_private::Process::ConnectRemote(lldb_private::Stream * strm, > llvm::StringRef remote_url) Line 3022 > > Could ProcessGDBRemote::GetDynamicLoader behave differently just when the > architecture is wasm32, maybe? But then it is probably cleaner to add this > plugin class, what do you think? Well.. I think DynamicLoaderStatic is being too grabby. :) However, when I though about that idea further, I realized that any default plugin we could implement this way could not be complete, as we would be missing the part which (un)loads modules when a new shared library (dis)appears. This is something that cannot be done in a generic way, as that usually requires setting breakpoint on some special symbol, or getting notifications about shared library events in some other way. I don't know whether this is something that you will also need/plan to implement for wasm, or if one can assume that all modules are loaded from the get-go, but this is what convinced me that putting this in a separate plugin is fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72751/new/ https://reviews.llvm.org/D72751 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
