Implement WindowsDYLD::DidAttach for querying the load address with a gdb-remote

[lanza]

Summary

Windows lldb debugging currently uses a process plugin to handle
launching and attaching to a process. Launching a process via a debug
server (e.g. ds2) and attaching to it with `gdb-remote port` currently
doesn't communicate address information of the executable properly.

Implement DynamicLoaderWindowsDYLD::DidAttach to obtain the proper
address information.

[lanza]

This fixes the broken patch from https://reviews.llvm.org/D54544

DynamicLoaderWindowsDYLD::DidLaunch is called right after ProcessWindows::DidLaunch in Process::Launch, so the previous patch was breaking the standard ProcessWindows pipeline for launching an inferior. DynamicLoaderWindowsDYLD::DidAttach is not called.

This patch's purpose is to fix debugging via

C:\> ds2 g 9021 C:\path\to\test.exe

// in another terminal
C:\> lldb C:\path\to\test.exe
(lldb) gdb-remote 9021

The code path used for attaching to a gdb-remote on Windows is the same as used on Linux & Darwin and thus DynamicLoaderWindowsDYLD::DidAttach gets called via Process::CompleteAttach but DynamicLoaderWindowsDYLD::DidLaunch does not get called.

So to fix the previous bug the logic was moved from DidLaunch to DidAttach.

However, a test for this change can't be added without modifying the python test suite to teach it how to check for availability of ds2 and then to test with it. I toyed around with that a bit but it would require quite a few changes to lldb/packages/Python/lldbsuite/test/tools/lldb-server to be Windows compatible or just building a bunch of new infrastructure. Though I do plan on integrating ds2 into the test-suite and possibly as a tool in lldb/tools to ease the test process at some point.

[lanza]

Rewrite commit message

[lanza]

@zturner I copied the win-py3-buildbot as best I could and found no regressions. Cool to land this?

[zturner]

Looks good if the tests pass. If something comes up (fingers crossed) we can always revert again.

[lanza]

@davide The review was here, not sure why this didn't get attached to the commit:

Landed in revision: https://llvm.org/svn/llvm-project/lldb/trunk@348526

[davide]

In D55302#1322131, @lanza wrote:

@davide The review was here, not sure why this didn't get attached to the commit:

Landed in revision: https://llvm.org/svn/llvm-project/lldb/trunk@348526

Sure, thanks for the explanation, Nathan!

source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp

 //===-- DynamicLoaderWindowsDYLD.cpp --------------------------------*- C++
 //-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "DynamicLoaderWindowsDYLD.h"
 
+#include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadPlanStepInstruction.h"
+#include "lldb/Utility/Log.h"
 
 #include "llvm/ADT/Triple.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 DynamicLoaderWindowsDYLD::DynamicLoaderWindowsDYLD(Process *process)
     : DynamicLoader(process) {}
DynamicLoader *DynamicLoaderWindowsDYLD::CreateInstance(Process *process,
   }
 
   if (should_create)
     return new DynamicLoaderWindowsDYLD(process);
 
   return nullptr;
 }
 
-void DynamicLoaderWindowsDYLD::DidAttach() {}
+void DynamicLoaderWindowsDYLD::DidAttach() {
+    Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (log)
+    log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__);
+
+  ModuleSP executable = GetTargetExecutable();
+
+  if (!executable.get())
+    return;
+
+  // Try to fetch the load address of the file from the process, since there
+  // could be randomization of the load address.
+
+  // It might happen that the remote has a different dir for the file, so we
+  // only send the basename of the executable in the query. I think this is safe
+  // because I doubt that two executables with the same basenames are loaded in
+  // memory...
+  FileSpec file_spec(
+      executable->GetPlatformFileSpec().GetFilename().GetCString());
+  bool is_loaded;
+  addr_t base_addr = 0;
+  lldb::addr_t load_addr;
+  Status error = m_process->GetFileLoadAddress(file_spec, is_loaded, load_addr);
+  if (error.Success() && is_loaded) {
+    base_addr = load_addr;
+    UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false);
+  }
+
+  ModuleList module_list;
+  module_list.Append(executable);
+  m_process->GetTarget().ModulesDidLoad(module_list);
+  m_process->LoadModules();
+}
 
 void DynamicLoaderWindowsDYLD::DidLaunch() {}
 
 Status DynamicLoaderWindowsDYLD::CanLoadImage() { return Status(); }
 
 ConstString DynamicLoaderWindowsDYLD::GetPluginName() {
   return GetPluginNameStatic();
 }