From 6fc77d3e7560233d2722ab8fc09565a39ba64ebb Mon Sep 17 00:00:00 2001 From: Christopher Faylor Date: Fri, 30 Aug 2013 21:02:02 +0000 Subject: [PATCH] * cygheap.h (user_heap_info::sbrk): Declare new function. (user_heap_info::init): Ditto. * heap.cc (user_heap_info::init): Rename from heap_init(). Avoid explictly using cygheap->user_heap. (sbrk): Use user_heap_info method via cygheap->user_heap. (user_heap_info::sbrk): Renamed from sbrk(). Eliminate explicit use of cygheap->user_heap. Change some pointer arithmetic to use (char *) for consistency. * shared.cc (shared_info::initialize): Change heap_init call to cygheap->user_heap.init. --- winsup/cygwin/ChangeLog | 13 ++++ winsup/cygwin/cygheap.h | 2 + winsup/cygwin/heap.cc | 130 +++++++++++++++++++--------------------- winsup/cygwin/shared.cc | 3 +- 4 files changed, 78 insertions(+), 70 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index de274c522..cee6e8da7 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,16 @@ +2013-08-30 Christopher Faylor + + * cygheap.h (user_heap_info::sbrk): Declare new function. + (user_heap_info::init): Ditto. + * heap.cc (user_heap_info::init): Rename from heap_init(). Avoid + explictly using cygheap->user_heap. + (sbrk): Use user_heap_info method via cygheap->user_heap. + (user_heap_info::sbrk): Renamed from sbrk(). Eliminate explicit use of + cygheap->user_heap. Change some pointer arithmetic to use (char *) for + consistency. + * shared.cc (shared_info::initialize): Change heap_init call to + cygheap->user_heap.init. + 2013-08-30 Corinna Vinschen * heap.cc (sbrk): Add a FIXME comment to VirtualFree call. Fix memory diff --git a/winsup/cygwin/cygheap.h b/winsup/cygwin/cygheap.h index ed645b7d6..b4c478ffb 100644 --- a/winsup/cygwin/cygheap.h +++ b/winsup/cygwin/cygheap.h @@ -351,6 +351,8 @@ struct user_heap_info void *top; void *max; SIZE_T chunk; + void __reg2 *sbrk (ptrdiff_t); + void __reg1 init (); }; struct hook_chain diff --git a/winsup/cygwin/heap.cc b/winsup/cygwin/heap.cc index b8f72bd26..276b77367 100644 --- a/winsup/cygwin/heap.cc +++ b/winsup/cygwin/heap.cc @@ -102,14 +102,14 @@ eval_initial_heap_size () /* Initialize the heap at process start up. */ void -heap_init () +user_heap_info::init () { const DWORD alloctype = MEM_RESERVE; /* If we're the forkee, we must allocate the heap at exactly the same place as our parent. If not, we (almost) don't care where it ends up. */ page_const = wincap.page_size (); - if (!cygheap->user_heap.base) + if (!base) { uintptr_t start_address = eval_start_address (); PVOID largest_found = NULL; @@ -117,13 +117,12 @@ heap_init () SIZE_T ret; MEMORY_BASIC_INFORMATION mbi; - cygheap->user_heap.chunk = eval_initial_heap_size (); + chunk = eval_initial_heap_size (); do { - cygheap->user_heap.base = VirtualAlloc ((LPVOID) start_address, - cygheap->user_heap.chunk, - alloctype, PAGE_NOACCESS); - if (cygheap->user_heap.base) + base = VirtualAlloc ((LPVOID) start_address, chunk, alloctype, + PAGE_NOACCESS); + if (base) break; /* Ok, so we are at the 1% which didn't work with 0x20000000 out @@ -136,7 +135,7 @@ heap_init () { if (mbi.State == MEM_FREE) { - if (mbi.RegionSize >= cygheap->user_heap.chunk) + if (mbi.RegionSize >= chunk) break; if (mbi.RegionSize > largest_found_size) { @@ -158,40 +157,34 @@ heap_init () region, unless it's smaller than the minimum size. */ if (largest_found_size >= MINHEAP_SIZE) { - cygheap->user_heap.chunk = largest_found_size; - cygheap->user_heap.base = - VirtualAlloc (largest_found, cygheap->user_heap.chunk, - alloctype, PAGE_NOACCESS); + chunk = largest_found_size; + base = VirtualAlloc (largest_found, chunk, alloctype, + PAGE_NOACCESS); } /* Last resort (but actually we are probably broken anyway): Use the minimal heap size and let the system decide. */ - if (!cygheap->user_heap.base) + if (!base) { - cygheap->user_heap.chunk = MINHEAP_SIZE; - cygheap->user_heap.base = - VirtualAlloc (NULL, cygheap->user_heap.chunk, - alloctype, PAGE_NOACCESS); + chunk = MINHEAP_SIZE; + base = VirtualAlloc (NULL, chunk, alloctype, PAGE_NOACCESS); } } } - while (!cygheap->user_heap.base && ret); - if (cygheap->user_heap.base == NULL) + while (!base && ret); + if (base == NULL) api_fatal ("unable to allocate heap, heap_chunk_size %ly, %E", - cygheap->user_heap.chunk); - cygheap->user_heap.ptr = cygheap->user_heap.top = cygheap->user_heap.base; - cygheap->user_heap.max = (char *) cygheap->user_heap.base - + cygheap->user_heap.chunk; + chunk); + ptr = top = base; + max = (char *) base + chunk; } else { - SIZE_T chunk = cygheap->user_heap.chunk; /* allocation chunk */ /* total size commited in parent */ - SIZE_T allocsize = (char *) cygheap->user_heap.top - - (char *) cygheap->user_heap.base; + SIZE_T allocsize = (char *) top - (char *) base; /* Loop until we've managed to reserve an adequate amount of memory. */ - char *p; SIZE_T reserve_size = chunk * ((allocsize + (chunk - 1)) / chunk); + /* With ptmalloc3 there's a good chance that there has been no memory allocated on the heap. If we don't check that, reserve_size will be 0 and from there, the below loop will end up overallocating due @@ -199,10 +192,11 @@ heap_init () if (!reserve_size) reserve_size = chunk; + char *p; while (1) { - p = (char *) VirtualAlloc (cygheap->user_heap.base, reserve_size, - alloctype, PAGE_READWRITE); + p = (char *) VirtualAlloc (base, reserve_size, alloctype, + PAGE_READWRITE); if (p) break; if ((reserve_size -= page_const) < allocsize) @@ -211,12 +205,12 @@ heap_init () if (!p && in_forkee && !fork_info->abort (NULL)) api_fatal ("couldn't allocate heap, %E, base %p, top %p, " "reserve_size %ld, allocsize %ld, page_const %d", - cygheap->user_heap.base, cygheap->user_heap.top, + base, top, reserve_size, allocsize, page_const); - if (p != cygheap->user_heap.base) + if (p != base) api_fatal ("heap allocated at wrong address %p (mapped) " - "!= %p (expected)", p, cygheap->user_heap.base); - if (allocsize && !VirtualAlloc (cygheap->user_heap.base, allocsize, + "!= %p (expected)", p, base); + if (allocsize && !VirtualAlloc (base, allocsize, MEM_COMMIT, PAGE_READWRITE)) api_fatal ("MEM_COMMIT failed, %E"); } @@ -228,39 +222,44 @@ heap_init () heap_init is called early, the heap size is printed pretty much at the start of the strace output, so there isn't anything lost. */ debug_printf ("heap base %p, heap top %p, heap size %ly (%lu)", - cygheap->user_heap.base, cygheap->user_heap.top, - cygheap->user_heap.chunk, cygheap->user_heap.chunk); + base, top, chunk, chunk); page_const--; // malloc_init (); } #define pround(n) (((size_t)(n) + page_const) & ~page_const) - -/* FIXME: This function no longer handles "split heaps". */ - /* Linux defines n to be intptr_t, newlib defines it to be ptrdiff_t. It shouldn't matter much, though, since the function is not standarized and sizeof(ptrdiff_t) == sizeof(intptr_t) anyway. */ extern "C" void * sbrk (ptrdiff_t n) { + return cygheap->user_heap.sbrk (n); +} + +void __reg2 * +user_heap_info::sbrk (ptrdiff_t n) +{ +/* FIXME: This function no longer handles "split heaps". */ + char *newtop, *newbrk; SIZE_T commitbytes, newbrksize, reservebytes; if (n == 0) - return cygheap->user_heap.ptr; /* Just wanted to find current cygheap->user_heap.ptr address */ + return ptr; /* Just wanted to find current ptr + address */ - newbrk = (char *) cygheap->user_heap.ptr + n; /* Where new cygheap->user_heap.ptr will be */ + newbrk = (char *) ptr + n; /* Where new cptr will be */ newtop = (char *) pround (newbrk); /* Actual top of allocated memory - on page boundary */ - if (newtop == cygheap->user_heap.top) + if (newtop == top) goto good; if (n < 0) { /* Freeing memory */ - assert (newtop < cygheap->user_heap.top); - n = (char *) cygheap->user_heap.top - newtop; + assert (newtop < top); + n = (char *) top - newtop; /* FIXME: This doesn't work if we cross a virtual memory reservation border. If that happens, we have to free the space in multiple VirtualFree calls, aligned to the former reservation borders. */ @@ -269,59 +268,52 @@ sbrk (ptrdiff_t n) goto err; /* Didn't take */ } - assert (newtop > cygheap->user_heap.top); + assert (newtop > top); /* Find the number of bytes to commit, rounded up to the nearest page. */ - commitbytes = pround (newtop - (char *) cygheap->user_heap.top); + commitbytes = pround (newtop - (char *) top); /* Need to grab more pages from the OS. If this fails it may be because we have used up previously reserved memory. Or, we're just plumb out of memory. Only attempt to commit memory that we know we've previously reserved. */ - if (newtop <= cygheap->user_heap.max) - { - if (VirtualAlloc (cygheap->user_heap.top, commitbytes, - MEM_COMMIT, PAGE_READWRITE) != NULL) - goto good; - } + if (newtop <= max && VirtualAlloc (top, commitbytes, MEM_COMMIT, + PAGE_READWRITE)) + goto good; - /* The remainder of the existing heap is too small to fulfil the memory + /* The remainder of the existing heap is too small to fulfill the memory request. We have to extend the heap, so we reserve some more memory and then commit the remainder of the old heap, if any, and the rest of the required space from the extended heap. */ /* Reserve either the maximum of the standard heap chunk size or the requested amount. Then attempt to actually allocate it. */ - reservebytes = commitbytes - ((ptrdiff_t) cygheap->user_heap.max - - (ptrdiff_t) cygheap->user_heap.top); + reservebytes = commitbytes - ((char *) max - (char *) top); commitbytes -= reservebytes; - if ((newbrksize = cygheap->user_heap.chunk) < reservebytes) + if ((newbrksize = chunk) < reservebytes) newbrksize = reservebytes; - if (VirtualAlloc (cygheap->user_heap.max, newbrksize, - MEM_RESERVE, PAGE_NOACCESS) - || VirtualAlloc (cygheap->user_heap.max, newbrksize = reservebytes, - MEM_RESERVE, PAGE_NOACCESS)) + if (VirtualAlloc (max, newbrksize, MEM_RESERVE, PAGE_NOACCESS) + || VirtualAlloc (max, newbrksize = reservebytes, MEM_RESERVE, + PAGE_NOACCESS)) { /* Now commit the requested memory. Windows keeps all virtual reservations separate, so we can't commit the two regions in a single, combined call or we suffer an ERROR_INVALID_ADDRESS. The same error is returned when trying to VirtualAlloc 0 bytes, which would occur if the existing heap was already full. */ - if ((!commitbytes || VirtualAlloc (cygheap->user_heap.top, commitbytes, - MEM_COMMIT, PAGE_READWRITE)) - && VirtualAlloc (cygheap->user_heap.max, reservebytes, - MEM_COMMIT, PAGE_READWRITE)) + if ((!commitbytes || VirtualAlloc (top, commitbytes, MEM_COMMIT, + PAGE_READWRITE)) + && VirtualAlloc (max, reservebytes, MEM_COMMIT, PAGE_READWRITE)) { - cygheap->user_heap.max = (char *) cygheap->user_heap.max - + pround (newbrksize); + max = (char *) max + pround (newbrksize); goto good; } /* If committing the memory failed, we must free the extendend reserved region, otherwise any other try to fetch memory (for instance by using mmap) may fail just because we still reserve memory we don't even know about. */ - VirtualFree (cygheap->user_heap.max, newbrksize, MEM_RELEASE); + VirtualFree (max, newbrksize, MEM_RELEASE); } err: @@ -329,8 +321,8 @@ err: return (void *) -1; good: - void *oldbrk = cygheap->user_heap.ptr; - cygheap->user_heap.ptr = newbrk; - cygheap->user_heap.top = newtop; + void *oldbrk = ptr; + ptr = newbrk; + top = newtop; return oldbrk; } diff --git a/winsup/cygwin/shared.cc b/winsup/cygwin/shared.cc index 9cc84593c..ef25c592a 100644 --- a/winsup/cygwin/shared.cc +++ b/winsup/cygwin/shared.cc @@ -341,7 +341,8 @@ shared_info::initialize () else if (cb != sizeof (*this)) system_printf ("size of shared memory region changed from %lu to %u", sizeof (*this), cb); - heap_init (); + /* FIXME? Shouldn't this be in memory_init? */ + cygheap->user_heap.init (); } void