paolosev added a comment.
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 :-)).
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?
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
