labath added a comment.

I think there are some architectural issues here in how ProcessWindows works. 
Normally (i.e., on all non-windows process classes), it is the job of the 
dynamic loader plugin to create modules and set their load addresses. However, 
ProcessWindows does things differently, and creates the modules by itself 
(ProcessWindows::OnLoadDLL).
I think the behavior of ProcessWindows is fine (it flows naturally from how 
modules are handled on windows), but if it's going to handle module loading by 
itself, then I think it should do the job completely, and not only half-way. 
So, if we want ProcessWindows to do the module loading, then it should return a 
null value for `GetDynamicLoader()` (or a reasonably stubbed out 
implementation) instead of relying on a generic dynamic loader plugin to finish 
the job.

Or, if we want to have the DynamicLoader plugin, then ProcessWindows should 
stop meddling with the module list in the OnLoadDLL function. One way to 
achieve that would be:

- ProcessWindows::OnLoadDLL calls DynamicLoaderWindowsDYLD::OnLoadDll
- dynamic loader loads the module and sets load address
- dynamic loader does not have to call `ProcessWindows::GetFileLoadAddress` as 
it already got the load address from in the OnLoadDLL function

This would make the flow very similar to how other platforms handle it (the 
only difference would be that the "OnLoadDLL" equivalent is called from a 
breakpoint callback function, instead of from a special debug event callback, 
but that's just due to a difference in how modules are reported).


Repository:
  rLLDB LLDB

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

Reply via email to