From 4ef641326aa28c42fc43869fa9a21fee2e79f4cc Mon Sep 17 00:00:00 2001 From: Guy Albertelli Date: Sun, 23 Jan 2000 02:25:11 +0000 Subject: [PATCH] Fix problems with GlobalHandle and GlobalFree so results match Win98. Restructure HEAP_ValidateInUseArena, HeapValidate, and add HEAP_IsRealArena to eliminate *bogus* error messages. --- memory/global.c | 37 +++++++---- memory/heap.c | 171 ++++++++++++++++++++++++++++++------------------ 2 files changed, 132 insertions(+), 76 deletions(-) diff --git a/memory/global.c b/memory/global.c index aac4e4d6dd8..8de0835dafb 100644 --- a/memory/global.c +++ b/memory/global.c @@ -1142,26 +1142,37 @@ HGLOBAL WINAPI GlobalHandle( ) { HGLOBAL handle; HANDLE heap; + PGLOBAL32_INTERN maybe_intern; + LPCVOID test; if (!pmem) { SetLastError( ERROR_INVALID_PARAMETER ); return 0; } - heap = GLOBAL_GetHeap( POINTER_TO_HANDLE(pmem) ); - if (!HEAP_IsInsideHeap( heap, 0, pmem )) goto error; +/* note that if pmem is a pointer to a a block allocated by */ +/* GlobalAlloc with GMEM_MOVEABLE then magic test in HeapValidate */ +/* will fail. */ + if (ISPOINTER(pmem)) { + heap = GLOBAL_GetHeap( (HGLOBAL)pmem ); + if (HeapValidate( heap, 0, pmem )) + return (HGLOBAL)pmem; /* valid fixed block */ handle = POINTER_TO_HANDLE(pmem); - if (HEAP_IsInsideHeap( heap, 0, (LPCVOID)handle )) - { - if (HANDLE_TO_INTERN(handle)->Magic == MAGIC_GLOBAL_USED) + } else + handle = (HGLOBAL)pmem; + +/* Now test handle either passed in or retrieved from pointer */ + heap = GLOBAL_GetHeap( handle ); + maybe_intern = HANDLE_TO_INTERN( handle ); + if (maybe_intern->Magic == MAGIC_GLOBAL_USED) { + test = maybe_intern->Pointer; + if (HeapValidate( heap, 0, ((HGLOBAL *)test)-1 ) && + /* obj(-handle) valid arena? */ + HeapValidate( heap, 0, maybe_intern )) /* intern valid arena? */ return handle; /* valid moveable block */ } - /* maybe FIXED block */ - if (HeapValidate( heap, 0, pmem )) - return (HGLOBAL)pmem; /* valid fixed block */ -error: SetLastError( ERROR_INVALID_HANDLE ); return 0; } @@ -1289,8 +1300,12 @@ HGLOBAL WINAPI GlobalFree( if(pintern->Magic==MAGIC_GLOBAL_USED) { - if(pintern->LockCount!=0) - SetLastError(ERROR_INVALID_HANDLE); + +/* WIN98 does not make this test. That is you can free a */ +/* block you have not unlocked. Go figure!! */ + /* if(pintern->LockCount!=0) */ + /* SetLastError(ERROR_INVALID_HANDLE); */ + if(pintern->Pointer) if(!HeapFree(heap, 0, (char *)(pintern->Pointer)-sizeof(HGLOBAL))) diff --git a/memory/heap.c b/memory/heap.c index 4ce0ded99e2..ea153040664 100644 --- a/memory/heap.c +++ b/memory/heap.c @@ -53,6 +53,9 @@ typedef struct tagARENA_FREE #define ARENA_INUSE_FILLER 0x55 #define ARENA_FREE_FILLER 0xaa +#define QUIET 1 /* Suppress messages */ +#define NOISY 0 /* Report all errors */ + #define HEAP_NB_FREE_LISTS 4 /* Number of free lists */ /* Max size of the blocks on the free lists */ @@ -100,6 +103,7 @@ typedef struct tagHEAP HANDLE SystemHeap = 0; HANDLE SegptrHeap = 0; +static BOOL HEAP_IsRealArena( HANDLE heap, DWORD flags, LPCVOID block, BOOL quiet ); #ifdef __GNUC__ #define GET_EIP() (__builtin_return_address(0)) @@ -204,7 +208,7 @@ static HEAP *HEAP_GetPtr( SetLastError( ERROR_INVALID_HANDLE ); return NULL; } - if (TRACE_ON(heap) && !HeapValidate( heap, 0, NULL )) + if (TRACE_ON(heap) && !HEAP_IsRealArena( heap, 0, NULL, NOISY )) { HEAP_Dump( heapPtr ); assert( FALSE ); @@ -737,15 +741,24 @@ static BOOL HEAP_ValidateFreeArena( SUBHEAP *subheap, ARENA_FREE *pArena ) /*********************************************************************** * HEAP_ValidateInUseArena */ -static BOOL HEAP_ValidateInUseArena( SUBHEAP *subheap, ARENA_INUSE *pArena ) +static BOOL HEAP_ValidateInUseArena( SUBHEAP *subheap, ARENA_INUSE *pArena, BOOL quiet ) { char *heapEnd = (char *)subheap + subheap->size; /* Check magic number */ if (pArena->magic != ARENA_INUSE_MAGIC) { + if (quiet == NOISY) { ERR("Heap %08lx: invalid in-use arena magic for %08lx\n", (DWORD)subheap->heap, (DWORD)pArena ); + if (TRACE_ON(heap)) + HEAP_Dump( subheap->heap ); + } else if (WARN_ON(heap)) { + WARN("Heap %08lx: invalid in-use arena magic for %08lx\n", + (DWORD)subheap->heap, (DWORD)pArena ); + if (TRACE_ON(heap)) + HEAP_Dump( subheap->heap ); + } return FALSE; } /* Check size flags */ @@ -874,6 +887,93 @@ SEGPTR HEAP_GetSegptr( HANDLE heap, DWORD flags, LPCVOID ptr ) return ret; } +/*********************************************************************** + * HEAP_IsRealArena [Internal] + * Validates a block is a valid arena. + * + * RETURNS + * TRUE: Success + * FALSE: Failure + */ +static BOOL HEAP_IsRealArena( + HANDLE heap, /* [in] Handle to the heap */ + DWORD flags, /* [in] Bit flags that control access during operation */ + LPCVOID block, /* [in] Optional pointer to memory block to validate */ + BOOL quiet /* [in] Flag - if true, HEAP_ValidateInUseArena + * does not complain */ +) { + SUBHEAP *subheap; + HEAP *heapPtr = (HEAP *)(heap); + BOOL ret = TRUE; + + if (!heapPtr || (heapPtr->magic != HEAP_MAGIC)) + { + ERR("Invalid heap %08x!\n", heap ); + return FALSE; + } + + flags &= HEAP_NO_SERIALIZE; + flags |= heapPtr->flags; + /* calling HeapLock may result in infinite recursion, so do the critsect directly */ + if (!(flags & HEAP_NO_SERIALIZE)) + EnterCriticalSection( &heapPtr->critSection ); + + if (block) + { + /* Only check this single memory block */ + + /* The following code is really HEAP_IsInsideHeap * + * with serialization already done. */ + if (!(subheap = HEAP_FindSubHeap( heapPtr, block )) || + ((char *)block < (char *)subheap + subheap->headerSize + + sizeof(ARENA_INUSE))) + { + if (quiet == NOISY) + ERR("Heap %08lx: block %08lx is not inside heap\n", + (DWORD)heap, (DWORD)block ); + else if (WARN_ON(heap)) + WARN("Heap %08lx: block %08lx is not inside heap\n", + (DWORD)heap, (DWORD)block ); + ret = FALSE; + } else + ret = HEAP_ValidateInUseArena( subheap, (ARENA_INUSE *)block - 1, quiet ); + + if (!(flags & HEAP_NO_SERIALIZE)) + LeaveCriticalSection( &heapPtr->critSection ); + return ret; + } + + subheap = &heapPtr->subheap; + while (subheap && ret) + { + char *ptr = (char *)subheap + subheap->headerSize; + while (ptr < (char *)subheap + subheap->size) + { + if (*(DWORD *)ptr & ARENA_FLAG_FREE) + { + if (!HEAP_ValidateFreeArena( subheap, (ARENA_FREE *)ptr )) { + ret = FALSE; + break; + } + ptr += sizeof(ARENA_FREE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); + } + else + { + if (!HEAP_ValidateInUseArena( subheap, (ARENA_INUSE *)ptr, NOISY )) { + ret = FALSE; + break; + } + ptr += sizeof(ARENA_INUSE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); + } + } + subheap = subheap->next; + } + + if (!(flags & HEAP_NO_SERIALIZE)) + LeaveCriticalSection( &heapPtr->critSection ); + return ret; +} + /*********************************************************************** * HeapCreate (KERNEL32.336) @@ -1023,7 +1123,7 @@ BOOL WINAPI HeapFree( WARN("(%08x,%08lx,%08lx): asked to free NULL\n", heap, flags, (DWORD)ptr ); } - if (!ptr || !HeapValidate( heap, HEAP_NO_SERIALIZE, ptr )) + if (!ptr || !HEAP_IsRealArena( heap, HEAP_NO_SERIALIZE, ptr, QUIET )) { if (!(flags & HEAP_NO_SERIALIZE)) HeapUnlock( heap ); SetLastError( ERROR_INVALID_PARAMETER ); @@ -1076,7 +1176,7 @@ LPVOID WINAPI HeapReAlloc( if (size < HEAP_MIN_BLOCK_SIZE) size = HEAP_MIN_BLOCK_SIZE; if (!(flags & HEAP_NO_SERIALIZE)) HeapLock( heap ); - if (!HeapValidate( heap, HEAP_NO_SERIALIZE, ptr )) + if (!HEAP_IsRealArena( heap, HEAP_NO_SERIALIZE, ptr, QUIET )) { if (!(flags & HEAP_NO_SERIALIZE)) HeapUnlock( heap ); SetLastError( ERROR_INVALID_PARAMETER ); @@ -1234,7 +1334,7 @@ DWORD WINAPI HeapSize( flags &= HEAP_NO_SERIALIZE; flags |= heapPtr->flags; if (!(flags & HEAP_NO_SERIALIZE)) HeapLock( heap ); - if (!HeapValidate( heap, HEAP_NO_SERIALIZE, ptr )) + if (!HEAP_IsRealArena( heap, HEAP_NO_SERIALIZE, ptr, QUIET )) { SetLastError( ERROR_INVALID_PARAMETER ); ret = 0xffffffff; @@ -1268,67 +1368,8 @@ BOOL WINAPI HeapValidate( DWORD flags, /* [in] Bit flags that control access during operation */ LPCVOID block /* [in] Optional pointer to memory block to validate */ ) { - SUBHEAP *subheap; - HEAP *heapPtr = (HEAP *)(heap); - BOOL ret = TRUE; - if (!heapPtr || (heapPtr->magic != HEAP_MAGIC)) - { - ERR("Invalid heap %08x!\n", heap ); - return FALSE; - } - - flags &= HEAP_NO_SERIALIZE; - flags |= heapPtr->flags; - /* calling HeapLock may result in infinite recursion, so do the critsect directly */ - if (!(flags & HEAP_NO_SERIALIZE)) - EnterCriticalSection( &heapPtr->critSection ); - if (block) - { - /* Only check this single memory block */ - if (!(subheap = HEAP_FindSubHeap( heapPtr, block )) || - ((char *)block < (char *)subheap + subheap->headerSize - + sizeof(ARENA_INUSE))) - { - ERR("Heap %08lx: block %08lx is not inside heap\n", - (DWORD)heap, (DWORD)block ); - ret = FALSE; - } else - ret = HEAP_ValidateInUseArena( subheap, (ARENA_INUSE *)block - 1 ); - - if (!(flags & HEAP_NO_SERIALIZE)) - LeaveCriticalSection( &heapPtr->critSection ); - return ret; - } - - subheap = &heapPtr->subheap; - while (subheap && ret) - { - char *ptr = (char *)subheap + subheap->headerSize; - while (ptr < (char *)subheap + subheap->size) - { - if (*(DWORD *)ptr & ARENA_FLAG_FREE) - { - if (!HEAP_ValidateFreeArena( subheap, (ARENA_FREE *)ptr )) { - ret = FALSE; - break; - } - ptr += sizeof(ARENA_FREE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); - } - else - { - if (!HEAP_ValidateInUseArena( subheap, (ARENA_INUSE *)ptr )) { - ret = FALSE; - break; - } - ptr += sizeof(ARENA_INUSE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); - } - } - subheap = subheap->next; - } - if (!(flags & HEAP_NO_SERIALIZE)) - LeaveCriticalSection( &heapPtr->critSection ); - return ret; + return HEAP_IsRealArena( heap, flags, block, QUIET ); }