[LLDB] [Windows] Fix Windows-specific race condition in LLDB for sess…

…ion lifetime This can e.g. happen if the debugged executable exits before the initial stop, e.g. if it fails to load dependent DLLs. Add a virtual destructor to ProcessDebugger and let it clean up the session, and make ProcessWindows::OnExitProcess call ProcessDebugger::OnExitProcess for shared parts. Fix suggestion by Adrian McCarthy. Differential Revision: https://reviews.llvm.org/D69503
llvm · Oct 31, 2019 · 403cd57 · 403cd57
1 parent 3db1d13
commit 403cd57
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 10 deletions.
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -63,6 +63,8 @@ static bool IsPageExecutable(uint32_t protect) {
 
 namespace lldb_private {
 
+ProcessDebugger::~ProcessDebugger() {}
+
 lldb::pid_t ProcessDebugger::GetDebuggedProcessId() const {
   if (m_session_data)
     return m_session_data->m_debugger->GetProcess().GetProcessId();

diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.h b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.h
@@ -48,6 +48,8 @@ class ProcessWindowsData {
 class ProcessDebugger {
 
 public:
+  virtual ~ProcessDebugger();
+
   virtual void OnExitProcess(uint32_t exit_code);
   virtual void OnDebuggerConnected(lldb::addr_t image_base);
   virtual ExceptionResult OnDebugException(bool first_chance,

diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -625,16 +625,7 @@ void ProcessWindows::OnExitProcess(uint32_t exit_code) {
   SetProcessExitStatus(GetID(), true, 0, exit_code);
   SetPrivateState(eStateExited);
 
-  // If the process exits before any initial stop then notify the debugger
-  // of the error otherwise WaitForDebuggerConnection() will be blocked.
-  // An example of this issue is when a process fails to load a dependent DLL.
-  if (m_session_data && !m_session_data->m_initial_stop_received) {
-    Status error(exit_code, eErrorTypeWin32);
-    OnDebuggerError(error, 0);
-  }
-
-  // Reset the session.
-  m_session_data.reset();
+  ProcessDebugger::OnExitProcess(exit_code);
 }
 
 void ProcessWindows::OnDebuggerConnected(lldb::addr_t image_base) {

diff --git a/lldb/test/Shell/Process/Windows/launch_failure.yaml b/lldb/test/Shell/Process/Windows/launch_failure.yaml
@@ -0,0 +1,90 @@
+## Test that we don't crash when the process fails to launch before reaching
+## the initial stop. This test exe has a dependency on NonExistent.dll.
+
+# REQUIRES: system-windows
+# REQUIRES: native && target-x86_64
+
+# RUN: yaml2obj %s > %t.exe
+# RUN: %lldb %t.exe -o run 2>&1 | FileCheck %s
+
+# CHECK: error: process launch failed: unknown error
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4096
+  ImageBase:       1073741824
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:       IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ImportTable:
+    RelativeVirtualAddress: 8192
+    Size:            40
+  ResourceTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ExceptionTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  CertificateTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  BaseRelocationTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  Debug:
+    RelativeVirtualAddress: 0
+    Size:            0
+  Architecture:
+    RelativeVirtualAddress: 0
+    Size:            0
+  GlobalPtr:
+    RelativeVirtualAddress: 0
+    Size:            0
+  TlsTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  LoadConfigTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  BoundImport:
+    RelativeVirtualAddress: 0
+    Size:            0
+  IAT:
+    RelativeVirtualAddress: 8248
+    Size:            16
+  DelayImportDescriptor:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ClrRuntimeHeader:
+    RelativeVirtualAddress: 0
+    Size:            0
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  4096
+    VirtualSize:     7
+    SectionData:     48FF2531100000
+  - Name:            .rdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  8192
+    VirtualSize:     96
+    SectionData:     282000000000000000000000502000003820000000000000000000000000000000000000000000004820000000000000000000000000000048200000000000000000000000000000000066756E6300004E6F6E4578697374656E742E646C6C00
+symbols:         []
+...