[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 194000.
jasonmolenda added a comment.

Added a testcase, TestModuleLoadedNotifys.

Renamed the pure virtual notifier methods in ModuleList to add 'Notify' at the start of their names, updated the names in subclass Target as well.  Added a NotifyModulesRemoved and changed ModuleList::Remove(ModuleList&) so that it removes all of the modules one-by-one from the collection and then calls NotifyModulesRemoved() so they are broadcast as a group.  I renamed the methods to make it clearer in Target that these are subclass implementations.

Changed Target::SetExecutableModule() so that it will load all dependent libraries without notifying, and send a group notification about all of them once they've been added.

Moved the call to LoadScriptingResourceForModule from Target::NotifyModuleAdded to Target::ModulesDidLoad so DynamicLoader plugins can call ModulesDidLoad with the full list of libraries and still get scripting files loaded.

I tried changing Target::GetOrCreateModule to return Expected<ModuleSP> but I wasn't thrilled with the double-dereference of the returned value when I started updating callers.  It wasn't horrible, but it looked messy.  I should go back and try again but my first shot at a few of them didn't look great.

I want to hand-test the scripting resource loading a bit on Monday but this is probably about what I'm doing for now.  At this point, DynamicLoaderDarwin sends batched notifications on the CreateTarget() binary dependent binaries.  It sends a batched notification when we start a real process and throw away all those binaries.  It sends out batched notifications when we load the binaries back in via dyld notifications.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60172

Files:
  include/lldb/Core/ModuleList.h
  include/lldb/Target/Target.h
  packages/Python/lldbsuite/test/functionalities/target-new-solib-notifications/Makefile
  packages/Python/lldbsuite/test/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
  packages/Python/lldbsuite/test/functionalities/target-new-solib-notifications/main.cpp
  source/API/SBTarget.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Core/DynamicLoader.cpp
  source/Core/ModuleList.cpp
  source/Expression/FunctionCaller.cpp
  source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  source/Plugins/Process/elf-core/ProcessElfCore.cpp
  source/Plugins/Process/minidump/ProcessMinidump.cpp
  source/Target/Target.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60172.194000.patch
Type: text/x-patch
Size: 33663 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190406/6f078864/attachment-0001.bin>