<div dir="ltr">I had to revert this because it breaks many tests on Windows (found by bisecting).  It was reverted in r347174.<div><br></div><div>You can reproduce one of the failures by building with this patch applied, then doing python bin/llvm-lit.py -sv ~/src/lldb/lit/SymbolFile/PDB.</div><div><br></div><div>2 of the tests should fail and you see an error about unable to read memory location.</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 15, 2018 at 1:38 PM Hui Huang via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hui added inline comments.<br>
<br>
<br>
================<br>
Comment at: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp:75<br>
+<br>
+void DynamicLoaderWindowsDYLD::DidLaunch() {<br>
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));<br>
----------------<br>
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.<br>
<br>
<br>
================<br>
Comment at: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp:102<br>
+<br>
+  UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false);<br>
<br>
----------------<br>
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.<br>
<br>
Moreover, if the load_addr is changed, make sure all the breakpoint address is recalculated. This feature could be a little bit complicated.<br>
<br>
<br>
Repository:<br>
  rLLDB LLDB<br>
<br>
<a href="https://reviews.llvm.org/D54544" rel="noreferrer" target="_blank">https://reviews.llvm.org/D54544</a><br>
<br>
<br>
<br>
</blockquote></div>