[ion] 52813: [KERNEL32]: User-mode Debugging fixes. Bug: RemoveHandles was checking for process AND thread handle match, instead of OR. Bug: CloseAllProcessHandles and RemoveHandles's processing lo...
23 Jul
2011
23 Jul
'11
6:28 p.m.
Author: ion
Date: Sat Jul 23 18:28:55 2011
New Revision: 52813
URL: http://svn.reactos.org/svn/reactos?rev=52813&view=rev
Log:
[KERNEL32]: User-mode Debugging fixes.
Bug: RemoveHandles was checking for process AND thread handle match, instead of OR.
Bug: CloseAllProcessHandles and RemoveHandles's processing loop was buggy with its
unlinking and re-use of nested ThreadData.
Bug: ProcessIdToHandle was requesting more process rights than needed.
Bug: Some ULONG<->BOOL conversions were dirty (as in, could result in values of 2 or
higher).
Bug: In WaitForDebugEvent, During CREATE_PROCESS_DEBUG_EVENT, the wrong handle was saved
as the thread handle.
Bug: In WaitForDebugEvent, all events returned TRUE. Instead, only valid events should
return TRUE, and non-existing events should return FALSE.
Add some asserts.
Simplify MarkThreadHandle and MarkProcessHandle.
Simplify OutputDebugStringW.
Modified:
trunk/reactos/dll/win32/kernel32/client/debugger.c
Modified: trunk/reactos/dll/win32/kernel32/client/debugger.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/…
==============================================================================
--- trunk/reactos/dll/win32/kernel32/client/debugger.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/client/debugger.c [iso-8859-1] Sat Jul 23 18:28:55
2011
@@ -264,8 +264,7 @@
PDBGSS_THREAD_DATA ThreadData;
/* Loop all thread data events */
- ThreadData = DbgSsGetThreadData();
- while (ThreadData)
+ for (ThreadData = DbgSsGetThreadData(); ThreadData; ThreadData =
ThreadData->Next)
{
/* Check if this one matches */
if (ThreadData->ThreadId == dwThreadId)
@@ -274,9 +273,6 @@
ThreadData->HandleMarked = TRUE;
break;
}
-
- /* Move to the next one */
- ThreadData = ThreadData->Next;
}
}
@@ -285,116 +281,81 @@
MarkProcessHandle(IN DWORD dwProcessId)
{
PDBGSS_THREAD_DATA ThreadData;
+
+ /* Loop all thread data events */
+ for (ThreadData = DbgSsGetThreadData(); ThreadData; ThreadData =
ThreadData->Next)
+ {
+ /* Check if this one matches */
+ if ((ThreadData->ProcessId == dwProcessId) &&
!(ThreadData->ThreadId))
+ {
+ /* Mark the structure and break out */
+ ThreadData->HandleMarked = TRUE;
+ break;
+ }
+ }
+}
+
+VOID
+WINAPI
+RemoveHandles(IN DWORD dwProcessId,
+ IN DWORD dwThreadId)
+{
+ PDBGSS_THREAD_DATA ThreadData, ThisData;
/* Loop all thread data events */
ThreadData = DbgSsGetThreadData();
- while (ThreadData)
+ for (ThisData = ThreadData; ThisData; ThisData = ThisData->Next)
{
/* Check if this one matches */
- if (ThreadData->ProcessId == dwProcessId)
+ if ((ThisData->HandleMarked) &&
+ ((ThisData->ProcessId == dwProcessId) || (ThisData->ThreadId ==
dwThreadId)))
{
- /* Make sure the thread ID is empty */
- if (!ThreadData->ThreadId)
- {
- /* Mark the structure and break out */
- ThreadData->HandleMarked = TRUE;
- break;
- }
+ /* Close open handles */
+ if (ThisData->ThreadHandle) CloseHandle(ThisData->ThreadHandle);
+ if (ThisData->ProcessHandle) CloseHandle(ThisData->ProcessHandle);
+
+ /* Unlink the thread data */
+ ThreadData->Next = ThisData->Next;
+
+ /* Free it*/
+ RtlFreeHeap(RtlGetProcessHeap(), 0, ThisData);
}
-
- /* Move to the next one */
- ThreadData = ThreadData->Next;
+ else
+ {
+ /* Move to the next one */
+ ThreadData = ThisData;
+ }
}
}
VOID
WINAPI
-RemoveHandles(IN DWORD dwProcessId,
- IN DWORD dwThreadId)
-{
- PDBGSS_THREAD_DATA ThreadData;
+CloseAllProcessHandles(IN DWORD dwProcessId)
+{
+ PDBGSS_THREAD_DATA ThreadData, ThisData;
/* Loop all thread data events */
ThreadData = DbgSsGetThreadData();
- while (ThreadData)
+ for (ThisData = ThreadData; ThisData; ThisData = ThisData->Next)
{
/* Check if this one matches */
- if (ThreadData->ProcessId == dwProcessId)
+ if (ThisData->ProcessId == dwProcessId)
{
- /* Make sure the thread ID matches too */
- if (ThreadData->ThreadId == dwThreadId)
- {
- /* Check if we have a thread handle */
- if (ThreadData->ThreadHandle)
- {
- /* Close it */
- CloseHandle(ThreadData->ThreadHandle);
- }
-
- /* Check if we have a process handle */
- if (ThreadData->ProcessHandle)
- {
- /* Close it */
- CloseHandle(ThreadData->ProcessHandle);
- }
-
- /* Unlink the thread data */
- DbgSsSetThreadData(ThreadData->Next);
-
- /* Free it*/
- RtlFreeHeap(RtlGetProcessHeap(), 0, ThreadData);
-
- /* Move to the next structure */
- ThreadData = DbgSsGetThreadData();
- continue;
- }
+ /* Close open handles */
+ if (ThisData->ThreadHandle) CloseHandle(ThisData->ThreadHandle);
+ if (ThisData->ProcessHandle) CloseHandle(ThisData->ProcessHandle);
+
+ /* Unlink the thread data */
+ ThreadData->Next = ThisData->Next;
+
+ /* Free it*/
+ RtlFreeHeap(RtlGetProcessHeap(), 0, ThisData);
}
-
- /* Move to the next one */
- ThreadData = ThreadData->Next;
- }
-}
-
-VOID
-WINAPI
-CloseAllProcessHandles(IN DWORD dwProcessId)
-{
- PDBGSS_THREAD_DATA ThreadData;
-
- /* Loop all thread data events */
- ThreadData = DbgSsGetThreadData();
- while (ThreadData)
- {
- /* Check if this one matches */
- if (ThreadData->ProcessId == dwProcessId)
+ else
{
- /* Check if we have a thread handle */
- if (ThreadData->ThreadHandle)
- {
- /* Close it */
- CloseHandle(ThreadData->ThreadHandle);
- }
-
- /* Check if we have a process handle */
- if (ThreadData->ProcessHandle)
- {
- /* Close it */
- CloseHandle(ThreadData->ProcessHandle);
- }
-
- /* Unlink the thread data */
- DbgSsSetThreadData(ThreadData->Next);
-
- /* Free it*/
- RtlFreeHeap(RtlGetProcessHeap(), 0, ThreadData);
-
- /* Move to the next structure */
- ThreadData = DbgSsGetThreadData();
- continue;
+ /* Move to the next one */
+ ThreadData = ThisData;
}
-
- /* Move to the next one */
- ThreadData = ThreadData->Next;
}
}
@@ -415,7 +376,9 @@
ClientId.UniqueProcess = UlongToHandle(dwProcessId);
InitializeObjectAttributes(&ObjectAttributes, NULL, 0, NULL, NULL);
Status = NtOpenProcess(&Handle,
- PROCESS_ALL_ACCESS,
+ PROCESS_CREATE_THREAD | PROCESS_VM_OPERATION |
+ PROCESS_VM_WRITE | PROCESS_VM_READ |
+ PROCESS_SUSPEND_RESUME | PROCESS_QUERY_INFORMATION,
&ObjectAttributes,
&ClientId);
if (!NT_SUCCESS(Status))
@@ -459,7 +422,7 @@
if (NT_SUCCESS(Status))
{
/* Return the current state */
- *pbDebuggerPresent = (DebugPort) ? TRUE : FALSE;
+ *pbDebuggerPresent = DebugPort != NULL;
return TRUE;
}
@@ -507,7 +470,7 @@
WINAPI
DebugActiveProcess(IN DWORD dwProcessId)
{
- NTSTATUS Status;
+ NTSTATUS Status, Status1;
HANDLE Handle;
/* Connect to the debugger */
@@ -524,7 +487,10 @@
/* Now debug the process */
Status = DbgUiDebugActiveProcess(Handle);
- NtClose(Handle);
+
+ /* Close the handle since we're done */
+ Status1 = NtClose(Handle);
+ ASSERT(NT_SUCCESS(Status1));
/* Check if debugging worked */
if (!NT_SUCCESS(Status))
@@ -545,7 +511,7 @@
WINAPI
DebugActiveProcessStop(IN DWORD dwProcessId)
{
- NTSTATUS Status;
+ NTSTATUS Status, Status1;
HANDLE Handle;
/* Get the process handle */
@@ -557,7 +523,8 @@
/* Now stop debgging the process */
Status = DbgUiStopDebugging(Handle);
- NtClose(Handle);
+ Status1 = NtClose(Handle);
+ ASSERT(NT_SUCCESS(Status1));
/* Check for failure */
if (!NT_SUCCESS(Status))
@@ -614,7 +581,7 @@
}
/* Now set the kill-on-exit state */
- State = KillOnExit;
+ State = KillOnExit != 0;
Status = NtSetInformationDebugObject(Handle,
DebugObjectKillProcessOnExitInformation,
&State,
@@ -711,15 +678,14 @@
/* Setup the thread data */
SaveThreadHandle(lpDebugEvent->dwProcessId,
lpDebugEvent->dwThreadId,
- lpDebugEvent->u.CreateThread.hThread);
+ lpDebugEvent->u.CreateProcessInfo.hThread);
break;
/* Process was exited */
case EXIT_PROCESS_DEBUG_EVENT:
- /* Mark the thread data as such */
+ /* Mark the thread data as such and fall through */
MarkProcessHandle(lpDebugEvent->dwProcessId);
- break;
/* Thread was exited */
case EXIT_THREAD_DEBUG_EVENT:
@@ -727,10 +693,18 @@
/* Mark the thread data */
MarkThreadHandle(lpDebugEvent->dwThreadId);
break;
-
- /* Nothing to do for anything else */
+
+ /* Nothing to do */
+ case EXCEPTION_DEBUG_EVENT:
+ case LOAD_DLL_DEBUG_EVENT:
+ case UNLOAD_DLL_DEBUG_EVENT:
+ case OUTPUT_DEBUG_STRING_EVENT:
+ case RIP_EVENT:
+ break;
+
+ /* Fail anything else */
default:
- break;
+ return FALSE;
}
/* Return success */
@@ -957,29 +931,24 @@
*/
VOID
WINAPI
-OutputDebugStringW(IN LPCWSTR _OutputString)
-{
- UNICODE_STRING wstrOut;
- ANSI_STRING strOut;
- NTSTATUS nErrCode;
+OutputDebugStringW(IN LPCWSTR OutputString)
+{
+ UNICODE_STRING UnicodeString;
+ ANSI_STRING AnsiString;
+ NTSTATUS Status;
/* convert the string in ANSI */
- RtlInitUnicodeString(&wstrOut, _OutputString);
- nErrCode = RtlUnicodeStringToAnsiString(&strOut, &wstrOut, TRUE);
-
- if(!NT_SUCCESS(nErrCode))
- {
- /* Microsoft's kernel32.dll always prints something, even in case the conversion
fails */
- OutputDebugStringA("");
- }
- else
- {
- /* output the converted string */
- OutputDebugStringA(strOut.Buffer);
-
- /* free the converted string */
- RtlFreeAnsiString(&strOut);
- }
+ RtlInitUnicodeString(&UnicodeString, OutputString);
+ Status = RtlUnicodeStringToAnsiString(&AnsiString, &UnicodeString, TRUE);
+
+ /* OutputDebugStringW always prints something, even if conversion fails */
+ if (!NT_SUCCESS(Status)) AnsiString.Buffer = "";
+
+ /* Output the converted string */
+ OutputDebugStringA(AnsiString.Buffer);
+
+ /* free the converted string */
+ if (NT_SUCCESS(Status)) RtlFreeAnsiString(&AnsiString);
}
/* EOF */
4864
days inactive
4864
days old
0 comments
1 participants
participants (1)
-
ion＠svn.reactos.org