tfiala accepted this revision.
tfiala added a comment.
This revision is now accepted and ready to land.
LGTM.
================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4907
@@ -4883,2 +4906,3 @@
+ m_process->GetTarget().ModulesDidUnload (removed_modules, false);
new_modules.ForEach ([&target](const lldb::ModuleSP module_sp) -> bool
----------------
fjricci wrote:
> tfiala wrote:
> > It looks like this code will unload any modules currently listed as loaded
> > via m_process->GetTarget().GetImages(), if they do not appear in the
> > module_list argument to this function. Is that correct behavior? (It
> > might be, but that's not what I would have guessed without digging deeper).
> >
> > I might be not following the flow here correctly, though. Can you clarify?
> > Did the full test suite run with this change? Thanks!
> So yes, this will remove any existing modules that are not in the svr4
> packet, provided that there were modules listed in the svr4 packet
> (indicating that the remote is using the packet) - if `new_modules.GetSize()
> == 0`, we won't remove anything.
>
> As far as I can tell from the gdb protocol specs, the svr4 packet should
> contain a list of all loaded libraries, so as long as the svr4 packet
> contains libraries, I believe that removing modules which are no longer
> listed is the correct behavior.
>
> I did run the full suite on linux (with lldb-server), and it passes with this
> change.
>
> As a side note, the module_list argument is actually an output parameter,
> filled by GetLoadedModuleList().
Oh of course I see the flow.
http://reviews.llvm.org/D19230
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
