Jlalond wrote:

> > [This 
> > codepath](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L118)
> >  results in none of the modules being loaded, or rebased, and I don't why 
> > this should apply without checking all modules.
> 
> Yeah, I don't like that code either, which is why I'm leaning more and more 
> towards deleting it. Judging by the patch which introduced it, I believe this 
> check is basically a proxy for "have the modules been loaded by the process 
> class", and the intention more-or-less was to make the dynamic loader plugin 
> a no-op in this case. What the other of the patch probably did not realize 
> (just like I did not realize it when I was implementing module loading in 
> ProcessMinidump) is that this makes thread local data access impossible (in 
> fairness, it's quite possible that, at that point, thread-local access was 
> not implemented yet).

Great context.

In my mind, I think every core file format will have to handle loading it's own 
modules that are from the core file. We will still want the dynamic loader 
plugin to do it's own book keeping. I haven't tested it, but I think we should 
allow the DYLD to do it's own book keeping but just not handle 
loading/unloading modules owned by the CoreProcess. This will likely require 
some extension of Process, but I think this a good evolution of the patch you 
linked

https://github.com/llvm/llvm-project/pull/109477
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to