[WIN32K]

Add back support for allocator stack backtraces for GDI objects and dump a list of BTs, when the GDI object table is exhausted. Disabled by default.

svn path=/trunk/; revision=66374

reactos/win32ss/gdi/ntgdi/gdidbg.c

@@ -15,6 +15,7 @@
 
 extern ULONG gulFirstFree;
 extern ULONG gulFirstUnused;
+extern PENTRY gpentHmgr;;
 
 ULONG gulLogUnique = 0;
 
@@ -94,21 +95,64 @@ DBG_CHANNEL DbgChannels[DbgChCount]={
     {L"UserWnd", DbgChUserWnd}
 };
 
-#ifdef GDI_DEBUG
+ULONG
-#if 0
+NTAPI
+DbgCaptureStackBackTace(
+    _Out_writes_(cFramesToCapture) PVOID* ppvFrames,
+    _In_ ULONG cFramesToSkip,
+    _In_ ULONG cFramesToCapture)
+{
+    ULONG cFrameCount;
+    PVOID apvTemp[30];
+    NT_ASSERT(cFramesToCapture <= _countof(apvTemp));
+
+    /* Zero it out */
+    RtlZeroMemory(ppvFrames, cFramesToCapture * sizeof(PVOID));
+
+    /* Capture kernel stack */
+    cFrameCount = RtlWalkFrameChain(apvTemp, _countof(apvTemp), 0);
+
+    /* If we should skip more than we have, we are done */
+    if (cFramesToSkip > cFrameCount)
+        return 0;
+
+    /* Copy, but skip frames */
+    cFrameCount -= cFramesToSkip;
+    cFrameCount = min(cFrameCount, cFramesToCapture);
+    RtlCopyMemory(ppvFrames, &apvTemp[cFramesToSkip], cFrameCount * sizeof(PVOID));
+
+    /* Check if there is still space left */
+    if (cFrameCount < cFramesToCapture)
+    {
+        /* Capture user stack */
+        cFrameCount += RtlWalkFrameChain(&ppvFrames[cFrameCount],
+                                         cFramesToCapture - cFrameCount,
+                                         1);
+    }
+
+    return cFrameCount;
+}
+
+#if DBG_ENABLE_GDIOBJ_BACKTRACES
+
 static
 BOOL
-CompareBacktraces(ULONG idx1, ULONG idx2)
+CompareBacktraces(
+    USHORT idx1,
+    USHORT idx2)
 {
+    POBJ pobj1, pobj2;
     ULONG iLevel;
 
+    /* Get the objects */
+    pobj1 = gpentHmgr[idx1].einfo.pobj;
+    pobj2 = gpentHmgr[idx2].einfo.pobj;
+
     /* Loop all stack levels */
-    for (iLevel = 0; iLevel < GDI_STACK_LEVELS; iLevel++)
+    for (iLevel = 0; iLevel < GDI_OBJECT_STACK_LEVELS; iLevel++)
     {
-        if (GDIHandleAllocator[idx1][iLevel]
+        /* If one level doesn't match we are done */
-                != GDIHandleAllocator[idx2][iLevel])
+        if (pobj1->apvBackTrace[iLevel] != pobj2->apvBackTrace[iLevel])
-//        if (GDIHandleShareLocker[idx1][iLevel]
-//                != GDIHandleShareLocker[idx2][iLevel])
         {
             return FALSE;
         }
@@ -117,23 +161,38 @@ CompareBacktraces(ULONG idx1, ULONG idx2)
     return TRUE;
 }
 
+typedef struct
+{
+    USHORT idx;
+    USHORT iCount;
+} GDI_DBG_HANDLE_BT;
+
 VOID
 NTAPI
 DbgDumpGdiHandleTableWithBT(void)
 {
-    static int leak_reported = 0;
+    static BOOL bLeakReported = FALSE;
-    int i, j, idx, nTraces = 0;
+    ULONG idx, j;
+    BOOL bAlreadyPresent;
+    GDI_DBG_HANDLE_BT aBacktraceTable[GDI_DBG_MAX_BTS];
+    USHORT iCount;
     KIRQL OldIrql;
+    POBJ pobj;
+    ULONG iLevel, ulObj;
 
-    if (leak_reported)
+    /* Only report once */
+    if (bLeakReported)
     {
         DPRINT1("GDI handle abusers already reported!\n");
         return;
     }
 
-    leak_reported = 1;
+    bLeakReported = TRUE;
     DPRINT1("Reporting GDI handle abusers:\n");
 
+    /* Zero out the table */
+    RtlZeroMemory(aBacktraceTable, sizeof(aBacktraceTable));
+
     /* We've got serious business to do */
     KeRaiseIrql(DISPATCH_LEVEL, &OldIrql);
 
@@ -141,94 +200,84 @@ DbgDumpGdiHandleTableWithBT(void)
     for (idx = RESERVE_ENTRIES_COUNT; idx < GDI_HANDLE_COUNT; idx++)
     {
         /* If the handle is free, continue */
-        if (!IS_HANDLE_VALID(idx)) continue;
+        if (gpentHmgr[idx].einfo.pobj == 0) continue;
 
-        /* Step through all previous backtraces */
+        /* Check if this backtrace is already covered */
-        for (j = 0; j < nTraces; j++)
+        bAlreadyPresent = FALSE;
+        for (j = RESERVE_ENTRIES_COUNT; j < idx; j++)
         {
-            /* Check if the backtrace matches */
+            if (CompareBacktraces(idx, j))
-            if (CompareBacktraces(idx, AllocatorTable[j].idx))
             {
-                /* It matches, increment count and break out */
+                bAlreadyPresent = TRUE;
-                AllocatorTable[j].count++;
                 break;
             }
         }
 
-        /* Did we find a new backtrace? */
+        if (bAlreadyPresent) continue;
-        if (j == nTraces)
-        {
-            /* Break out, if we reached the maximum */
-            if (nTraces == MAX_BACKTRACES) break;
 
-            /* Initialize this entry */
+        /* We don't have this BT yet, count how often it is present */
-            AllocatorTable[j].idx = idx;
+        iCount = 1;
-            AllocatorTable[j].count = 1;
+        for (j = idx + 1; j < GDI_HANDLE_COUNT; j++)
-            nTraces++;
+        {
+            if (CompareBacktraces(idx, j))
+            {
+                iCount++;
             }
         }
 
-    /* bubble sort time! weeeeee!! */
+        /* Now add this backtrace */
-    for (i = 0; i < nTraces-1; i++)
+        for (j = 0; j < GDI_DBG_MAX_BTS; j++)
         {
-        if (AllocatorTable[i].count < AllocatorTable[i+1].count)
+            /* Insert it below the next smaller count */
+            if (aBacktraceTable[j].iCount < iCount)
             {
-            struct DbgOpenGDIHandle temp;
+                /* Check if there are entries above */
+                if (j < GDI_DBG_MAX_BTS - 1)
+                {
+                    /* Move the following entries up by 1 */
+                    RtlMoveMemory(&aBacktraceTable[j],
+                                  &aBacktraceTable[j + 1],
+                                  GDI_DBG_MAX_BTS - j - 1);
+                }
 
-            temp = AllocatorTable[i+1];
+                /* Set this entry */
-            AllocatorTable[i+1] = AllocatorTable[i];
+                aBacktraceTable[j].idx = idx;
-            j = i;
+                aBacktraceTable[j].iCount = iCount;
-            while (j > 0 && AllocatorTable[j-1].count < temp.count)
+
-                j--;
+                /* We are done here */
-            AllocatorTable[j] = temp;
+                break;
+            }
         }
     }
 
     /* Print the worst offenders... */
-    DbgPrint("Worst GDI Handle leak offenders (out of %i unique locations):\n", nTraces);
+    DbgPrint("Count Handle   Backtrace\n");
-    for (i = 0; i < nTraces && AllocatorTable[i].count > 1; i++)
+    DbgPrint("------------------------------------------------\n");
+    for (j = 0; j < GDI_DBG_MAX_BTS; j++)
     {
-        /* Print out the allocation count */
+        idx = aBacktraceTable[j].idx;
-        DbgPrint(" %i allocs, type = 0x%lx:\n",
+        if (idx == 0)
-                 AllocatorTable[i].count,
+            break;
-                 GdiHandleTable->Entries[AllocatorTable[i].idx].Type);
 
-        /* Dump the frames */
+        ulObj = ((ULONG)gpentHmgr[idx].FullUnique << 16) | idx;
-        KeRosDumpStackFrames(GDIHandleAllocator[AllocatorTable[i].idx], GDI_STACK_LEVELS);
+        pobj = gpentHmgr[idx].einfo.pobj;
-        //KeRosDumpStackFrames(GDIHandleShareLocker[AllocatorTable[i].idx], GDI_STACK_LEVELS);
 
-        /* Print new line for better readability */
+        DbgPrint("%5d %08lx ", aBacktraceTable[j].iCount, ulObj);
+        for (iLevel = 0; iLevel < GDI_OBJECT_STACK_LEVELS; iLevel++)
+        {
+            DbgPrint("%p,", pobj->apvBackTrace[iLevel]);
+        }
         DbgPrint("\n");
     }
 
-    if (i < nTraces)
+    __debugbreak();
-        DbgPrint("(List terminated - the remaining entries have 1 allocation only)\n");
 
     KeLowerIrql(OldIrql);
-
-    ASSERT(FALSE);
 }
-#endif
 
-ULONG
+#endif /* DBG_ENABLE_GDIOBJ_BACKTRACES */
-NTAPI
-DbgCaptureStackBackTace(PVOID* pFrames, ULONG nFramesToCapture)
-{
-    ULONG nFrameCount;
 
-    memset(pFrames, 0x00, (nFramesToCapture + 1) * sizeof(PVOID));
+#if DBG
-
-    nFrameCount = RtlWalkFrameChain(pFrames, nFramesToCapture, 0);
-
-    if (nFrameCount < nFramesToCapture)
-    {
-        nFrameCount += RtlWalkFrameChain(pFrames + nFrameCount,
-                                         nFramesToCapture - nFrameCount,
-                                         1);
-    }
-
-    return nFrameCount;
-}
 
 BOOL
 NTAPI
@@ -352,32 +401,10 @@ DbgGdiHTIntegrityCheck()
 	return r;
 }
 
-#endif /* GDI_DEBUG */
+#endif /* DBG */
 
-VOID
-NTAPI
-DbgDumpLockedGdiHandles()
-{
-#if 0
-    ULONG i;
 
-    for (i = RESERVE_ENTRIES_COUNT; i < GDI_HANDLE_COUNT; i++)
+#if DBG_ENABLE_EVENT_LOGGING
-    {
-        PENTRY pentry = &gpentHmgr[i];
-
-        if (pentry->Objt)
-        {
-            POBJ pobj = pentry->einfo.pobj;
-            if (pobj->cExclusiveLock > 0)
-            {
-                DPRINT1("Locked object: %lx, type = %lx. allocated from:\n",
-                        i, pentry->Objt);
-                DBG_DUMP_EVENT_LIST(&pobj->slhLog);
-            }
-        }
-    }
-#endif
-}
 
 VOID
 NTAPI
@@ -400,7 +427,7 @@ DbgLogEvent(PSLIST_HEADER pslh, LOG_EVENT_TYPE nEventType, LPARAM lParam)
     pLogEntry->lParam = lParam;
 
     /* Capture a backtrace */
-    DbgCaptureStackBackTace(pLogEntry->apvBackTrace, 20);
+    DbgCaptureStackBackTace(pLogEntry->apvBackTrace, 1, 20);
 
     switch (nEventType)
     {
@@ -483,6 +510,33 @@ DbgCleanupEventList(PSLIST_HEADER pslh)
     }
 }
 
+#endif /* DBG_ENABLE_EVENT_LOGGING */
+
+#if 1 || DBG_ENABLE_SERVICE_HOOKS
+
+VOID
+NTAPI
+DbgDumpLockedGdiHandles()
+{
+    ULONG i;
+
+    for (i = RESERVE_ENTRIES_COUNT; i < GDI_HANDLE_COUNT; i++)
+    {
+        PENTRY pentry = &gpentHmgr[i];
+
+        if (pentry->Objt)
+        {
+            POBJ pobj = pentry->einfo.pobj;
+            if (pobj->cExclusiveLock > 0)
+            {
+                DPRINT1("Locked object: %lx, type = %lx. allocated from:\n",
+                        i, pentry->Objt);
+                DBG_DUMP_EVENT_LIST(&pobj->slhLog);
+            }
+        }
+    }
+}
+
 void
 NTAPI
 GdiDbgPreServiceHook(ULONG ulSyscallId, PULONG_PTR pulArguments)
@@ -513,6 +567,9 @@ GdiDbgPostServiceHook(ULONG ulSyscallId, ULONG_PTR ulResult)
     return ulResult;
 }
 
+#endif /* DBG_ENABLE_SERVICE_HOOKS */
+
+
 NTSTATUS NTAPI
 QueryEnvironmentVariable(PUNICODE_STRING Name,
                          PUNICODE_STRING Value)

reactos/win32ss/gdi/ntgdi/gdidebug.h

@@ -1,5 +1,62 @@
 #pragma once
 
+#define GDI_DBG_MAX_BTS 10
+
+#if DBG
+#define ASSERT_NOGDILOCKS() GdiDbgAssertNoLocks(__FILE__,__LINE__)
+#define KeRosDumpStackFrames(Frames, Count) \
+    KdSystemDebugControl('DsoR', (PVOID)Frames, Count, NULL, 0, NULL, KernelMode)
+#else
+#define ASSERT_NOGDILOCKS()
+#define KeRosDumpStackFrames(Frames, Count)
+#endif
+
+NTSYSAPI
+ULONG
+APIENTRY
+RtlWalkFrameChain(
+    _Out_ PVOID *Callers,
+    _In_ ULONG Count,
+    _In_ ULONG Flags);
+
+ULONG
+NTAPI
+DbgCaptureStackBackTace(
+    _Out_writes_(cFramesToCapture) PVOID* ppvFrames,
+    _In_ ULONG cFramesToSkip,
+    _In_ ULONG cFramesToCapture);
+
+BOOL
+NTAPI
+DbgGdiHTIntegrityCheck(
+    VOID);
+
+VOID
+NTAPI
+DbgDumpLockedGdiHandles(
+    VOID);
+
+#if DBG_ENABLE_GDIOBJ_BACKTRACES
+
+VOID
+NTAPI
+DbgDumpGdiHandleTableWithBT(VOID);
+
+#endif
+
+#if KDBG
+
+BOOLEAN
+NTAPI
+DbgGdiKdbgCliCallback(
+    _In_ PCHAR Command,
+    _In_ ULONG Argc,
+    _In_ PCH Argv[]);
+
+#endif
+
+#if DBG_ENABLE_EVENT_LOGGING
+
 typedef enum _LOG_EVENT_TYPE
 {
     EVENT_ALLOCATE,
@@ -28,40 +85,42 @@ typedef struct _LOGENTRY
     } data;
 } LOGENTRY, *PLOGENTRY;
 
-#if KDBG
+VOID
-BOOLEAN
 NTAPI
-DbgGdiKdbgCliCallback(
+DbgDumpEventList(
-    IN PCHAR Command,
+    _Inout_ PSLIST_HEADER pslh);
-    IN ULONG Argc,
+
-    IN PCH Argv[]);
+VOID
-#endif
+NTAPI
+DbgLogEvent(
+    _Inout_ PSLIST_HEADER pslh,
+    _In_ LOG_EVENT_TYPE nEventType,
+    _In_ LPARAM lParam);
+
+VOID
+NTAPI
+DbgCleanupEventList(
+    _Inout_ PSLIST_HEADER pslh);
+
+VOID
+NTAPI
+DbgPrintEvent(
+    _Inout_ PLOGENTRY pLogEntry);
 
-#if DBG_ENABLE_EVENT_LOGGING
-VOID NTAPI DbgDumpEventList(PSLIST_HEADER pslh);
-VOID NTAPI DbgLogEvent(PSLIST_HEADER pslh, LOG_EVENT_TYPE nEventType, LPARAM lParam);
-VOID NTAPI DbgCleanupEventList(PSLIST_HEADER pslh);
-VOID NTAPI DbgPrintEvent(PLOGENTRY pLogEntry);
 #define DBG_LOGEVENT(pslh, type, val) DbgLogEvent(pslh, type, (ULONG_PTR)val)
 #define DBG_INITLOG(pslh) InitializeSListHead(pslh)
 #define DBG_DUMP_EVENT_LIST(pslh) DbgDumpEventList(pslh)
 #define DBG_CLEANUP_EVENT_LIST(pslh) DbgCleanupEventList(pslh)
+
 #else
+
 #define DBG_LOGEVENT(pslh, type, val) ((void)(val))
 #define DBG_INITLOG(pslh)
 #define DBG_DUMP_EVENT_LIST(pslh)
 #define DBG_CLEANUP_EVENT_LIST(pslh)
+
 #endif
 
-
-VOID NTAPI DbgDumpGdiHandleTableWithBT(VOID);
-ULONG NTAPI DbgCaptureStackBackTace(PVOID* pFrames, ULONG nFramesToCapture);
-BOOL NTAPI DbgGdiHTIntegrityCheck(VOID);
-VOID NTAPI DbgDumpLockedGdiHandles(VOID);
-
-#define KeRosDumpStackFrames(Frames, Count) KdSystemDebugControl('DsoR', (PVOID)Frames, Count, NULL, 0, NULL, KernelMode)
-NTSYSAPI ULONG APIENTRY RtlWalkFrameChain(OUT PVOID *Callers, IN ULONG Count, IN ULONG Flags);
-
 #if DBG
 void
 NTAPI
@@ -88,9 +147,6 @@ GdiDbgAssertNoLocks(char * pszFile, ULONG nLine)
         ASSERT(FALSE);
     }
 }
-
-#define ASSERT_NOGDILOCKS() GdiDbgAssertNoLocks(__FILE__,__LINE__)
-#else
-#define ASSERT_NOGDILOCKS()
 #endif
 
+

reactos/win32ss/gdi/ntgdi/gdiobj.c

@@ -339,6 +339,10 @@ ENTRY_pentPopFreeEntry(VOID)
             if (iFirst >= GDI_HANDLE_COUNT)
             {
                 DPRINT1("No more GDI handles left!\n");
+#if DBG_ENABLE_GDIOBJ_BACKTRACES
+                DbgDumpGdiHandleTableWithBT();
+#endif
+                InterlockedDecrement((LONG*)&gulFirstUnused);
                 return 0;
             }
 
@@ -350,7 +354,7 @@ ENTRY_pentPopFreeEntry(VOID)
         pentFree = &gpentHmgr[iFirst & GDI_HANDLE_INDEX_MASK];
 
         /* Create a new value with an increased sequence number */
-        iNext = (USHORT)(ULONG_PTR)pentFree->einfo.pobj;
+        iNext = GDI_HANDLE_GET_INDEX(pentFree->einfo.hFree);
         iNext |= (iFirst & ~GDI_HANDLE_INDEX_MASK) + 0x10000;
 
         /* Try to exchange the FirstFree value */
@@ -516,6 +520,9 @@ GDIOBJ_AllocateObject(UCHAR objt, ULONG cjSize, FLONG fl)
     pobj->BaseFlags = fl & 0xffff;
     DBG_INITLOG(&pobj->slhLog);
     DBG_LOGEVENT(&pobj->slhLog, EVENT_ALLOCATE, 0);
+#if DBG_ENABLE_GDIOBJ_BACKTRACES
+    DbgCaptureStackBackTace(pobj->apvBackTrace, 1, GDI_OBJECT_STACK_LEVELS);
+#endif /* GDI_DEBUG */
 
     return pobj;
 }
@@ -1497,9 +1504,7 @@ GDI_CleanupForProcess(struct _EPROCESS *Process)
     }
 
 #if DBG
-//#ifdef GDI_DEBUG
     DbgGdiHTIntegrityCheck();
-//#endif
 #endif
 
     ppi = PsGetCurrentProcessWin32Process();

reactos/win32ss/gdi/ntgdi/gdiobj.h

@@ -5,6 +5,8 @@
 
 #pragma once
 
+#define GDI_OBJECT_STACK_LEVELS 10
+
 /* The first 10 entries are never used in windows, they are empty */
 static const unsigned RESERVE_ENTRIES_COUNT = 10;
 
@@ -42,6 +44,9 @@ typedef struct _BASEOBJECT
     USHORT cExclusiveLock;
     USHORT BaseFlags;
     EX_PUSH_LOCK pushlock;
+#if DBG_ENABLE_GDIOBJ_BACKTRACES
+    PVOID apvBackTrace[GDI_OBJECT_STACK_LEVELS];
+#endif
 #if DBG_ENABLE_EVENT_LOGGING
     SLIST_HEADER slhLog;
 #endif

reactos/win32ss/win32kp.h

@@ -23,6 +23,7 @@
 
 /* Enable debugging features */
 #define GDI_DEBUG 0
+#define DBG_ENABLE_GDIOBJ_BACKTRACES 0
 #define DBG_ENABLE_EVENT_LOGGING 0
 #define DBG_ENABLE_SERVICE_HOOKS 0