[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.



> 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 :-)).
>  There is an explicit check for UnknownOS:
> 
>   DynamicLoader *DynamicLoaderStatic::CreateInstance(Process *process,
>                                                      bool force) {
>     bool create = force;
>     if (!create) {
>       const llvm::Triple &triple_ref =
>           process->GetTarget().GetArchitecture().GetTriple();
>       const llvm::Triple::OSType os_type = triple_ref.getOS();
>       if ((os_type == llvm::Triple::UnknownOS))
>         create = true;
>     }
>     ...
>   
>   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?

So if you really have no OS, and no vendor, then this plug-in does get used. Why? Because it assumes that things will be loaded where they are (file_addr == load_addr). Could you create your WASM stuff in a way such that all addresses that are in a memory loaded object file have the file address the same as the load address? Do you ever actually have files on disk that we load for WASM? Or is it always loaded from memory?

It **is** ok to add your plug-in that recognizes WASM with no OS and no vendor, you will just need to make sure that the plug-in is registered before DynamicLoaderStatic. Since the plug-in manager will run through each plug-in linearly when looking for a match. This is also why any "auto registration of plug-ins" will cause problems as if we don't control the plug-in registration order, we will have issues that will arise. That is a different topic though, but I thought I remembered seeing a patch that tried to do this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72751/new/

https://reviews.llvm.org/D72751