[Lldb-commits] [PATCH] D54544: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

[Hui Huang via Phabricator via lldb-commits]

Hui added inline comments.


================
Comment at: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp:75
+
+void DynamicLoaderWindowsDYLD::DidLaunch() {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
----------------
I think DynamicLoaderWindowsDYLD::DidAttach & DidLaunch are intended for remote debugging. As the similar functionalities have been done in  ProcessWindows::DidLaunch & DidAttach, the test in here is actually testing those native ones and they happen to be correct.


================
Comment at: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp:102
+
+  UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false);
 
----------------
A remote debugging shows if the 'qFileLoadAddress' remote packet is not implemented or incorrectly handled, GetFileLoadAddress by the remote process will return error resulting base_addr stay '0. Since you are calling with the last argument 'false' to indicate it is not an offset, that will be an issue. Better to move it to line 99 and check if the load_addr is LLDB_INVALID_ADDRESS.

Moreover, if the load_addr is changed, make sure all the breakpoint address is recalculated. This feature could be a little bit complicated.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54544