From 619ddb5d49c8d34490a29b9a67ef8dc646661b48 Mon Sep 17 00:00:00 2001 From: "Pierson Lee (PIE)" Date: Fri, 30 Jun 2017 18:33:11 -0700 Subject: [PATCH 1/4] Fix termination on MinGW/Cygwin by calling Windows API to terminate. MinGW and Cygwin do not allow Async break to occur without user intervention. The problem is with -exec-abort (stop debugging) while in run mode requires us to do an async break so that we can abort. Since we can't do that without additional user intervention, we will terminate the child process instead. --- src/MICore/CommandFactories/gdb.cs | 1 + src/MICore/Debugger.cs | 46 +++++++++++++++++-- .../Engine.Impl/DebuggedProcess.cs | 1 + 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/MICore/CommandFactories/gdb.cs b/src/MICore/CommandFactories/gdb.cs index 3d769521a..d16ca3a59 100644 --- a/src/MICore/CommandFactories/gdb.cs +++ b/src/MICore/CommandFactories/gdb.cs @@ -207,6 +207,7 @@ public override async Task Terminate() // that isn't actually supported by gdb. await _debugger.CmdAsync("kill", ResultClass.None); } + private static string TypeBySize(uint size) { switch (size) diff --git a/src/MICore/Debugger.cs b/src/MICore/Debugger.cs index 25c18d3f2..20a85b445 100755 --- a/src/MICore/Debugger.cs +++ b/src/MICore/Debugger.cs @@ -11,6 +11,7 @@ using System.Linq; using Microsoft.Win32.SafeHandles; using Microsoft.DebugEngineHost; +using System.Runtime.InteropServices; namespace MICore { @@ -49,6 +50,8 @@ public class Debugger : ITransportCallback public bool IsCygwin { get; protected set; } + public bool IsMinGW { get; protected set; } + public bool SendNewLineAfterCmd { get; protected set; } public virtual void FlushBreakStateData() @@ -233,7 +236,7 @@ public Task AddInternalBreakAction(Func func) _retryCount = 0; _waitingToStop = true; - // When using signals to stop the proces, do not kick off another break attempt. The debug break injection and + // When using signals to stop the process, do not kick off another break attempt. The debug break injection and // signal based models are reliable so no retries are needed. Cygwin can't currently async-break reliably, so // use retries there. if (!IsLocalGdb() && !this.IsCygwin) @@ -604,14 +607,49 @@ protected bool IsCoreDump } } + [DllImport("kernel32.dll", SetLastError = true)] + public static extern IntPtr OpenProcess(int dwDesiredAccess, [MarshalAs(UnmanagedType.Bool)] bool bInheritHandle, int dwProcessId); + [DllImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + static extern bool TerminateProcess(IntPtr hProcess, uint uExitCode); + public async Task CmdTerminate() { if (!_terminating) { _terminating = true; - if (ProcessState == ProcessState.Running && this.MICommandFactory.Mode != MIMode.Clrdbg) - { - await AddInternalBreakAction(() => MICommandFactory.Terminate()); + if (ProcessState == ProcessState.Running && + this.MICommandFactory.Mode != MIMode.Clrdbg) + { + // MinGW and Cygwin on Windows don't support async break. Because of this, + // the normal path of sending an internal async break so we can exit doesn't work. + // Therefore, we will call TerminateProcess on the debuggee with the exit code of 0 + // to terminate debugging. + if ((this.IsCygwin || this.IsMinGW) && _debuggeePids.Count > 0) + { + int debuggeePid = _debuggeePids.First().Value; + IntPtr handle = IntPtr.Zero; + try + { + // 0x1 = Terminate + handle = OpenProcess(0x1, false, debuggeePid); + if (handle != IntPtr.Zero && TerminateProcess(handle, 0)) + { + // OperationThread's _runningOpCompleteEvent is doing WaitOne(). Calling MICommandFactory.Terminate() will Set() it, unblocking the UI. + await MICommandFactory.Terminate(); + return new Results(ResultClass.done); + } + } + finally + { + if (handle != IntPtr.Zero) + Marshal.FreeCoTaskMem(handle); + } + } + else + { + await AddInternalBreakAction(() => MICommandFactory.Terminate()); + } } else { diff --git a/src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs b/src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs index a6bdbda9c..7aa740ea1 100755 --- a/src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs +++ b/src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs @@ -790,6 +790,7 @@ private void CheckCygwin(List commands, LocalLaunchOptions localL } else { + this.IsMinGW = true; // Gdb on windows and not cygwin implies mingw _engineTelemetry.SendWindowsRuntimeEnvironment(EngineTelemetry.WindowsRuntimeEnvironment.MinGW); } From c824810ee6cc1e490cc23d7f880497b9abc9b508 Mon Sep 17 00:00:00 2001 From: "Pierson Lee (PIE)" Date: Wed, 5 Jul 2017 16:19:32 -0700 Subject: [PATCH 2/4] PR Comments --- src/MICore/Debugger.cs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/MICore/Debugger.cs b/src/MICore/Debugger.cs index 20a85b445..7f0a2daed 100755 --- a/src/MICore/Debugger.cs +++ b/src/MICore/Debugger.cs @@ -576,23 +576,18 @@ public Task CmdBreak(BreakRequest request) internal bool IsLocalGdb() { - if (this.MICommandFactory.Mode == MIMode.Gdb && + return (this.MICommandFactory.Mode == MIMode.Gdb && this._launchOptions is LocalLaunchOptions && - String.IsNullOrEmpty(((LocalLaunchOptions)this._launchOptions).MIDebuggerServerAddress) - ) - { - return true; - } - else - { - return false; - } + String.IsNullOrEmpty(((LocalLaunchOptions)this._launchOptions).MIDebuggerServerAddress)); + } private bool IsRemoteGdb() { return this.MICommandFactory.Mode == MIMode.Gdb && - this._launchOptions is PipeLaunchOptions; + (this._launchOptions is PipeLaunchOptions || + (this._launchOptions is LocalLaunchOptions + && !String.IsNullOrEmpty(((LocalLaunchOptions)this._launchOptions).MIDebuggerServerAddress))); } protected bool IsCoreDump @@ -608,10 +603,12 @@ protected bool IsCoreDump } [DllImport("kernel32.dll", SetLastError = true)] - public static extern IntPtr OpenProcess(int dwDesiredAccess, [MarshalAs(UnmanagedType.Bool)] bool bInheritHandle, int dwProcessId); + private static extern IntPtr OpenProcess(int dwDesiredAccess, [MarshalAs(UnmanagedType.Bool)] bool bInheritHandle, int dwProcessId); [DllImport("kernel32.dll", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] - static extern bool TerminateProcess(IntPtr hProcess, uint uExitCode); + private static extern bool TerminateProcess(IntPtr hProcess, uint uExitCode); + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool CloseHandle(IntPtr hHandle); public async Task CmdTerminate() { @@ -625,7 +622,9 @@ public async Task CmdTerminate() // the normal path of sending an internal async break so we can exit doesn't work. // Therefore, we will call TerminateProcess on the debuggee with the exit code of 0 // to terminate debugging. - if ((this.IsCygwin || this.IsMinGW) && _debuggeePids.Count > 0) + if (this.IsLocalGdb() && + (this.IsCygwin || this.IsMinGW) && + _debuggeePids.Count > 0) { int debuggeePid = _debuggeePids.First().Value; IntPtr handle = IntPtr.Zero; @@ -643,7 +642,10 @@ public async Task CmdTerminate() finally { if (handle != IntPtr.Zero) - Marshal.FreeCoTaskMem(handle); + { + bool close = CloseHandle(handle); + Debug.Assert(close, "Why did CloseHandle fail?"); + } } } else From e6ae29f273190d5455db1b6b11f163dddc8cca99 Mon Sep 17 00:00:00 2001 From: "Pierson Lee (PIE)" Date: Wed, 5 Jul 2017 16:28:51 -0700 Subject: [PATCH 3/4] Fix remote scenario --- src/MIDebugEngine/Engine.Impl/DebuggedThread.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/MIDebugEngine/Engine.Impl/DebuggedThread.cs b/src/MIDebugEngine/Engine.Impl/DebuggedThread.cs index 74a9510fd..cea81d92a 100644 --- a/src/MIDebugEngine/Engine.Impl/DebuggedThread.cs +++ b/src/MIDebugEngine/Engine.Impl/DebuggedThread.cs @@ -217,7 +217,11 @@ internal async Task ThreadCreatedEvent(int id, string groupId) Results results = await _debugger.MICommandFactory.ThreadInfo(tid); if (results.ResultClass != ResultClass.done) { - Debug.Fail("Thread info not successful"); + // This can happen on some versions of gdb where thread-info is not supported while running, so only assert if we're also not running. + if (this._debugger.ProcessState != ProcessState.Running) + { + Debug.Fail("Thread info not successful"); + } } else { From 0b5cb5eb3029845705d08f07447e10c4c9bca1b7 Mon Sep 17 00:00:00 2001 From: "Pierson Lee (PIE)" Date: Fri, 7 Jul 2017 13:11:55 -0700 Subject: [PATCH 4/4] Refactor to terminate all debuggeePids --- src/MICore/Debugger.cs | 77 +++++++++++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 28 deletions(-) diff --git a/src/MICore/Debugger.cs b/src/MICore/Debugger.cs index 7f0a2daed..8e3dbe9d9 100755 --- a/src/MICore/Debugger.cs +++ b/src/MICore/Debugger.cs @@ -602,14 +602,6 @@ protected bool IsCoreDump } } - [DllImport("kernel32.dll", SetLastError = true)] - private static extern IntPtr OpenProcess(int dwDesiredAccess, [MarshalAs(UnmanagedType.Bool)] bool bInheritHandle, int dwProcessId); - [DllImport("kernel32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - private static extern bool TerminateProcess(IntPtr hProcess, uint uExitCode); - [DllImport("kernel32.dll", SetLastError = true)] - private static extern bool CloseHandle(IntPtr hHandle); - public async Task CmdTerminate() { if (!_terminating) @@ -623,29 +615,14 @@ public async Task CmdTerminate() // Therefore, we will call TerminateProcess on the debuggee with the exit code of 0 // to terminate debugging. if (this.IsLocalGdb() && - (this.IsCygwin || this.IsMinGW) && + (this.IsCygwin || this.IsMinGW) && _debuggeePids.Count > 0) { - int debuggeePid = _debuggeePids.First().Value; - IntPtr handle = IntPtr.Zero; - try - { - // 0x1 = Terminate - handle = OpenProcess(0x1, false, debuggeePid); - if (handle != IntPtr.Zero && TerminateProcess(handle, 0)) - { - // OperationThread's _runningOpCompleteEvent is doing WaitOne(). Calling MICommandFactory.Terminate() will Set() it, unblocking the UI. - await MICommandFactory.Terminate(); - return new Results(ResultClass.done); - } - } - finally + if (TerminateAllPids()) { - if (handle != IntPtr.Zero) - { - bool close = CloseHandle(handle); - Debug.Assert(close, "Why did CloseHandle fail?"); - } + // OperationThread's _runningOpCompleteEvent is doing WaitOne(). Calling MICommandFactory.Terminate() will Set() it, unblocking the UI. + await MICommandFactory.Terminate(); + return new Results(ResultClass.done); } } else @@ -662,6 +639,50 @@ public async Task CmdTerminate() return new Results(ResultClass.done); } + [DllImport("kernel32.dll", SetLastError = true)] + private static extern IntPtr OpenProcess(int dwDesiredAccess, [MarshalAs(UnmanagedType.Bool)] bool bInheritHandle, int dwProcessId); + + [DllImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static extern bool TerminateProcess(IntPtr hProcess, uint uExitCode); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool CloseHandle(IntPtr hHandle); + + /// + /// Call PInvoke to terminate all debuggee PIDs. This is to solve MinGW/Cygwin issues in Windows and SHOULD NOT be used in other cases. + /// + /// True if any pids were terminated successfully + private bool TerminateAllPids() + { + var terminated = false; + foreach (var pid in _debuggeePids) + { + int debuggeePid = pid.Value; + IntPtr handle = IntPtr.Zero; + try + { + // 0x1 = Terminate + handle = OpenProcess(0x1, false, debuggeePid); + if (handle != IntPtr.Zero && TerminateProcess(handle, 0)) + { + terminated = true; + } + } + finally + { + if (handle != IntPtr.Zero) + { + bool close = CloseHandle(handle); + Debug.Assert(close, "Why did CloseHandle fail?"); + } + } + } + + return terminated; + } + + public async Task CmdDetach() { _detaching = true;