[Lldb-commits] [lldb] fe17e02 - [lldb][Windows] Always call SetExecutableModule on debugger connected

[Martin Storsjö via lldb-commits]

Author: Alvin Wong
Date: 2022-09-30T13:51:56+03:00
New Revision: fe17e026959cc263c12fafa3a8e9e83503f9a18b

URL: https://github.com/llvm/llvm-project/commit/fe17e026959cc263c12fafa3a8e9e83503f9a18b
DIFF: https://github.com/llvm/llvm-project/commit/fe17e026959cc263c12fafa3a8e9e83503f9a18b.diff

LOG: [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

Added: 
    lldb/test/Shell/Target/Inputs/main.c
    lldb/test/Shell/Target/Inputs/shlib.c
    lldb/test/Shell/Target/dependent-modules-nodupe-windows.test

Modified: 
    lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
index e3d04c9f34491..a84dd62357de7 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 0000000000000..761410b1b52fe
--- /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 0000000000000..b0a8d463fb663
--- /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 0000000000000..52e46cd0c9b21
--- /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