From fe17e026959cc263c12fafa3a8e9e83503f9a18b Mon Sep 17 00:00:00 2001
From: Alvin Wong <alvin@alvinhc.com>
Date: Wed, 28 Sep 2022 12:58:51 +0300
Subject: [PATCH] [lldb][Windows] Always call SetExecutableModule on debugger
 connected

In `ProcessWindows::OnDebuggerConnected` (triggered from
`CREATE_PROCESS_DEBUG_EVENT`), we should always call
`Target::SetExecutableModule` regardless of whether LLDB has already
preloaded the executable modules. `SetExecutableModule` has the side
effect of clearing the module list of the Target, which help make sure
that module #0 is the executable module and the rest of the modules are
listed according to the DLL load order in the process (technically this
has no real consequences but it seems to make more sense anyway.) It
also fixes an issue where the modules preloaded by LLDB will be
duplicated when the debuggee process actually loads the DLL.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D134636
---
 .../Process/Windows/Common/ProcessWindows.cpp      | 36 ++++++++++------------
 lldb/test/Shell/Target/Inputs/main.c               |  2 ++
 lldb/test/Shell/Target/Inputs/shlib.c              |  1 +
 .../Target/dependent-modules-nodupe-windows.test   | 24 +++++++++++++++
 4 files changed, 44 insertions(+), 19 deletions(-)
 create mode 100644 lldb/test/Shell/Target/Inputs/main.c
 create mode 100644 lldb/test/Shell/Target/Inputs/shlib.c
 create mode 100644 lldb/test/Shell/Target/dependent-modules-nodupe-windows.test

diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
index e3d04c9..a84dd62 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -653,28 +653,26 @@ void ProcessWindows::OnDebuggerConnected(lldb::addr_t image_base) {
   LLDB_LOG(log, "Debugger connected to process {0}.  Image base = {1:x}",
            debugger->GetProcess().GetProcessId(), image_base);
 
-  ModuleSP module = GetTarget().GetExecutableModule();
-  if (!module) {
-    // During attach, we won't have the executable module, so find it now.
-    const DWORD pid = debugger->GetProcess().GetProcessId();
-    const std::string file_name = GetProcessExecutableName(pid);
-    if (file_name.empty()) {
-      return;
-    }
-
-    FileSpec executable_file(file_name);
-    FileSystem::Instance().Resolve(executable_file);
-    ModuleSpec module_spec(executable_file);
-    Status error;
-    module =
-        GetTarget().GetOrCreateModule(module_spec, true /* notify */, &error);
-    if (!module) {
-      return;
-    }
+  ModuleSP module;
+  // During attach, we won't have the executable module, so find it now.
+  const DWORD pid = debugger->GetProcess().GetProcessId();
+  const std::string file_name = GetProcessExecutableName(pid);
+  if (file_name.empty()) {
+    return;
+  }
 
-    GetTarget().SetExecutableModule(module, eLoadDependentsNo);
+  FileSpec executable_file(file_name);
+  FileSystem::Instance().Resolve(executable_file);
+  ModuleSpec module_spec(executable_file);
+  Status error;
+  module =
+      GetTarget().GetOrCreateModule(module_spec, true /* notify */, &error);
+  if (!module) {
+    return;
   }
 
+  GetTarget().SetExecutableModule(module, eLoadDependentsNo);
+
   if (auto dyld = GetDynamicLoader())
     dyld->OnLoadModule(module, ModuleSpec(), image_base);
 
diff --git a/lldb/test/Shell/Target/Inputs/main.c b/lldb/test/Shell/Target/Inputs/main.c
new file mode 100644
index 0000000..761410b
--- /dev/null
+++ b/lldb/test/Shell/Target/Inputs/main.c
@@ -0,0 +1,2 @@
+__declspec(dllimport) void exportFunc(void);
+int main() { exportFunc(); }
diff --git a/lldb/test/Shell/Target/Inputs/shlib.c b/lldb/test/Shell/Target/Inputs/shlib.c
new file mode 100644
index 0000000..b0a8d46
--- /dev/null
+++ b/lldb/test/Shell/Target/Inputs/shlib.c
@@ -0,0 +1 @@
+__declspec(dllexport) void exportFunc(void) {}
diff --git a/lldb/test/Shell/Target/dependent-modules-nodupe-windows.test b/lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
new file mode 100644
index 0000000..52e46cd
--- /dev/null
+++ b/lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
@@ -0,0 +1,24 @@
+# REQUIRES: system-windows
+
+# Checks that dependent modules preloaded by LLDB are not duplicated when the
+# process actually loads the DLL.
+
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll \
+# RUN:             %if windows-msvc %{-Wl,-implib:%t.shlib.lib%} \
+# RUN:             %else %{-Wl,--out-implib=%t.shlib.lib%}
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.lib -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \
+# RUN:       -o "#after" -o "target modules list" %t.main.exe | FileCheck %s
+
+# CHECK-LABEL: #before
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: .shlib.dll
+
+# CHECK-LABEL: #after
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: ntdll.dll
+# CHECK-NEXT: kernel32.dll
+# CHECK:      .shlib.dll
+# CHECK-NOT:  .shlib.dll
-- 
2.7.4