From 71f53a2f6254e4f47891cd58ab562220547d01a2 Mon Sep 17 00:00:00 2001 From: Christopher Faylor Date: Mon, 8 Jun 2009 03:53:40 +0000 Subject: [PATCH] * cygheap.h (mini_cygheap): New struct. (init_cygheap): Inherit locale field via mini_cygheap. * cygheap.cc (cygheap_at_start): Define new variable. (cygheap): Initialize as cygheap_at_start so that locale information is always available. (cygheap_init): Initialize cygheap iff it is set to cygheap_at_start. * shared_info.h (memory_init): Accommodate argument change. * memory.cc (memory_init): Accept an argument indicating whether cygheap should be initialized or not. * dcrt0.cc (child_info_fork::handle_fork): Pass false to memory_init(). (child_info_spawn::handle_spawn): Ditto. (dll_crt0_0): Pass true to memory_init when not forking or execing. * cygheap.h (cygheap_types::HEAP_2_DLL): New enum. * dll_init.h (dll): Remove unused namelen field. (dll_list::load_after_fork): Accommodate change in arguments. * dll_init.cc (dll_list::alloc): Allocate dll information in the cygwin heap. (dll_list::detach): Free dll information from the cygwin heap. (dll_list::load_after_fork): Use dll information in the cygwin heap directly rather than querying parent. * fork.cc (frok::first_dll): Delete. (frok::child): Don't report on first_dll. Don't pass it to load_on_fork. (frok::parent): Don't set first_dll. (fork): Ditto. --- winsup/cygwin/ChangeLog | 29 ++++++ winsup/cygwin/cygheap.cc | 19 ++-- winsup/cygwin/cygheap.h | 9 +- winsup/cygwin/dcrt0.cc | 6 +- winsup/cygwin/dll_init.cc | 197 ++++++++---------------------------- winsup/cygwin/dll_init.h | 5 +- winsup/cygwin/fork.cc | 16 ++- winsup/cygwin/shared.cc | 6 +- winsup/cygwin/shared_info.h | 2 +- 9 files changed, 104 insertions(+), 185 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 07389daf8..17b387b70 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,32 @@ +2009-06-07 Christopher Faylor + + * cygheap.h (mini_cygheap): New struct. + (init_cygheap): Inherit locale field via mini_cygheap. + * cygheap.cc (cygheap_at_start): Define new variable. + (cygheap): Initialize as cygheap_at_start so that locale information is + always available. + (cygheap_init): Initialize cygheap iff it is set to cygheap_at_start. + * shared_info.h (memory_init): Accommodate argument change. + * memory.cc (memory_init): Accept an argument indicating whether + cygheap should be initialized or not. + * dcrt0.cc (child_info_fork::handle_fork): Pass false to memory_init(). + (child_info_spawn::handle_spawn): Ditto. + (dll_crt0_0): Pass true to memory_init when not forking or execing. + + * cygheap.h (cygheap_types::HEAP_2_DLL): New enum. + * dll_init.h (dll): Remove unused namelen field. + (dll_list::load_after_fork): Accommodate change in arguments. + * dll_init.cc (dll_list::alloc): Allocate dll information in the cygwin + heap. + (dll_list::detach): Free dll information from the cygwin heap. + (dll_list::load_after_fork): Use dll information in the cygwin heap + directly rather than querying parent. + * fork.cc (frok::first_dll): Delete. + (frok::child): Don't report on first_dll. Don't pass it to + load_on_fork. + (frok::parent): Don't set first_dll. + (fork): Ditto. + 2009-06-06 Corinna Vinschen * dll_init.cc (dll_list::alloc): Allocate memory using a section diff --git a/winsup/cygwin/cygheap.cc b/winsup/cygwin/cygheap.cc index 1a25b6f8e..4ab1e3323 100644 --- a/winsup/cygwin/cygheap.cc +++ b/winsup/cygwin/cygheap.cc @@ -24,7 +24,12 @@ #include #include -init_cygheap NO_COPY *cygheap; +static mini_cygheap NO_COPY cygheap_at_start = +{ + {__utf8_mbtowc, __utf8_wctomb} +}; + +init_cygheap NO_COPY *cygheap = (init_cygheap *) &cygheap_at_start; void NO_COPY *cygheap_max; extern "C" char _cygheap_mid[] __attribute__((section(".cygheap"))); @@ -33,11 +38,11 @@ extern "C" char _cygheap_end[]; static NO_COPY muto cygheap_protect; struct cygheap_entry - { - int type; - struct cygheap_entry *next; - char data[0]; - }; +{ + int type; + struct cygheap_entry *next; + char data[0]; +}; #define NBUCKETS (sizeof (cygheap->buckets) / sizeof (cygheap->buckets[0])) #define N0 ((_cmalloc_entry *) NULL) @@ -150,7 +155,7 @@ extern "C" void __stdcall cygheap_init () { cygheap_protect.init ("cygheap_protect"); - if (!cygheap) + if (cygheap == &cygheap_at_start) { cygheap = (init_cygheap *) memset (_cygheap_start, 0, _cygheap_mid - _cygheap_start); diff --git a/winsup/cygwin/cygheap.h b/winsup/cygwin/cygheap.h index 2c878f0c9..d58f176f7 100644 --- a/winsup/cygwin/cygheap.h +++ b/winsup/cygwin/cygheap.h @@ -31,6 +31,7 @@ enum cygheap_types HEAP_1_EXEC, HEAP_1_MAX = 100, HEAP_2_STR, + HEAP_2_DLL, HEAP_MMAP = 200 }; @@ -282,13 +283,17 @@ struct hook_chain struct hook_chain *next; }; -struct init_cygheap +struct mini_cygheap +{ + cygheap_locale locale; +}; + +struct init_cygheap: public mini_cygheap { _cmalloc_entry *chain; char *buckets[32]; cygheap_root root; cygheap_user user; - cygheap_locale locale; user_heap_info user_heap; mode_t umask; HANDLE console_h; diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc index 3088b5020..f31bf937f 100644 --- a/winsup/cygwin/dcrt0.cc +++ b/winsup/cygwin/dcrt0.cc @@ -571,7 +571,7 @@ void child_info_fork::handle_fork () { cygheap_fixup_in_child (false); - memory_init (); + memory_init (false); myself.thisproc (NULL); myself->uid = cygheap->user.real_uid; myself->gid = cygheap->user.real_gid; @@ -598,7 +598,7 @@ child_info_spawn::handle_spawn () extern void fixup_lockf_after_exec (); HANDLE h; cygheap_fixup_in_child (true); - memory_init (); + memory_init (false); if (!moreinfo->myself_pinfo || !DuplicateHandle (hMainProc, moreinfo->myself_pinfo, hMainProc, &h, 0, FALSE, DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)) @@ -702,7 +702,7 @@ dll_crt0_0 () child_proc_info = get_cygwin_startup_info (); if (!child_proc_info) - memory_init (); + memory_init (true); else { cygwin_user_h = child_proc_info->user_h; diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc index 5b026c772..6743d2601 100644 --- a/winsup/cygwin/dll_init.cc +++ b/winsup/cygwin/dll_init.cc @@ -20,14 +20,10 @@ details. */ #include "pinfo.h" #include "cygtls.h" #include -#include -#include -#include -#include "ntdll.h" extern void __stdcall check_sanity_and_sync (per_process *); -dll_list NO_COPY dlls; +dll_list dlls; static bool dll_global_dtors_recorded; @@ -107,7 +103,7 @@ dll_list::operator[] (const PWCHAR name) #define RETRIES 1000 -/* Allocate space for a dll struct contiguous with the just-loaded dll. */ +/* Allocate space for a dll struct. */ dll * dll_list::alloc (HINSTANCE h, per_process *p, dll_type type) { @@ -118,96 +114,16 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type) dll *d = dlls[name]; if (d) { - d->count++; /* Yes. Bump the usage count. */ + if (!in_forkee) + d->count++; /* Yes. Bump the usage count. */ return d; /* Return previously allocated pointer. */ } - void *s = p->bss_end; - size_t d_size = sizeof (dll) + namelen * sizeof (WCHAR); - - MEMORY_BASIC_INFORMATION m; - NTSTATUS status = 0; - HANDLE sect_h; - OBJECT_ATTRIBUTES oa; - InitializeObjectAttributes (&oa, NULL, 0, NULL, - sec_none.lpSecurityDescriptor); - - /* Search for space after the DLL */ - for (int i = 0; i <= RETRIES; i++, s = (char *) m.BaseAddress + m.RegionSize) - { - if (!VirtualQuery (s, &m, sizeof (m))) - return NULL; /* Can't do it. */ - if (m.State == MEM_FREE) - { - /* Couldn't find any. Uh oh. FIXME: Issue an error? */ - if (i == RETRIES) - return NULL; /* Oh well. Couldn't locate free space. */ - - d = (dll *) m.BaseAddress; - /* Instead of calling VirtualAlloc, which always allocates memory - on a 64K boundary, we allocate the memory using a section - object. The disadvantage of the 64K boundary in this case is - the fact that that boundary could be easily the start address - of another DLL yet to load into memory. - - On x86, using a section object allows us to allocate the struct - dll into a memory slot in the remainder of the last 64K slot of - the DLL. This memory slot will never be used for anything - else. Given that the struct dll will fit into a single page - 99.99% of the time anyway, this is a neat way to avoid DLL load - address collisions in most cases. - - Of course, this doesn't help if the DLL needs all of the 64K - memory slot but there's only a 1 in 16 chance for that. - - And, alas, it won't work on 64 bit systems because the - AT_ROUND_TO_PAGE flag required to make a page-aligned allocation - isn't supported under WOW64. So, as with VirtualAlloc, ensure - that address is rounded up to next 64K allocation boundary if - running under WOW64. */ - if (wincap.is_wow64 ()) - d = (dll *) roundup2 ((uintptr_t) d, getpagesize ()); - - LARGE_INTEGER so = { QuadPart: d_size }; - status = NtCreateSection (§_h, SECTION_ALL_ACCESS, &oa, &so, - PAGE_READWRITE, SEC_COMMIT, NULL); - if (NT_SUCCESS (status)) - { - ULONG viewsize = 0; - so.QuadPart = 0; - status = NtMapViewOfSection (sect_h, GetCurrentProcess (), - (void **) &d, 0, d_size, &so, - &viewsize, ViewUnmap, - wincap.is_wow64 () - ? 0 : AT_ROUND_TO_PAGE, - PAGE_READWRITE); -#ifdef DEBUGGING - if (!NT_SUCCESS (status)) - system_printf ("NtMapViewOfSection failed, %p", status); -#endif - NtClose (sect_h); - } -#ifdef DEBUGGING - else - system_printf ("NtCreateSection failed, %p", status); -#endif - - if (NT_SUCCESS (status)) - break; - } - } - - /* Did we succeed? */ - if (!NT_SUCCESS (status)) - { /* Nope. */ - __seterrno_from_nt_status (status); - return NULL; - } + d = (dll *) cmalloc (HEAP_2_DLL, sizeof (*d) + (namelen * sizeof (*name))); /* Now we've allocated a block of information. Fill it in with the supplied info about this DLL. */ d->count = 1; - d->namelen = namelen; wcscpy (d->name, name); d->handle = h; d->p = p; @@ -251,7 +167,7 @@ dll_list::detach (void *retaddr) loaded_dlls--; if (end == d) end = d->prev; - NtUnmapViewOfSection (GetCurrentProcess (), d); + cfree (d); break; } } @@ -321,82 +237,51 @@ release_upto (const PWCHAR name, DWORD here) } /* Reload DLLs after a fork. Iterates over the list of dynamically loaded - DLLs and attempts to load them in the same place as they were loaded in - the parent. */ + DLLs and attempts to load them in the same place as they were loaded in the + parent. */ void -dll_list::load_after_fork (HANDLE parent, dll *first) +dll_list::load_after_fork (HANDLE parent) { - int try2 = 0; - dll *d = (dll *) alloca (sizeof (dll) + (NT_MAX_PATH - 1) * sizeof (WCHAR)); - - void *next = first; - while (next) - { - DWORD nb; - /* Read 4K of the dll structure from the parent. A full page has - been allocated anyway and this covers most, if not all DLL paths. - Only if d->namelen indicates that more than 4K are required, - read them in a second step. */ - if (!ReadProcessMemory (parent, next, d, getsystempagesize (), &nb) - || nb != getsystempagesize ()) - return; - size_t namelen = d->namelen * sizeof (WCHAR); - if (namelen >= getsystempagesize () - sizeof (dll) - && (!ReadProcessMemory (parent, next, d->name, namelen, &nb) - || nb != namelen)) - return; - - /* We're only interested in dynamically loaded dlls. - Hopefully, this function wouldn't even have been called unless - the parent had some of those. */ - if (d->type == DLL_LOAD) + for (dll *d = &dlls.start; (d = d->next) != NULL; ) + if (d->type == DLL_LOAD) + for (int i = 0; i < 2; i++) { - bool unload = true; - HMODULE h = LoadLibraryExW (d->name, NULL, - DONT_RESOLVE_DLL_REFERENCES); - - if (!h) - system_printf ("can't reload %W", d->name); /* See if DLL will load in proper place. If so, free it and reload it the right way. - It sort of stinks that we can't invert the order of the - FreeLibrary and LoadLibrary since Microsoft documentation seems - to imply that that should do what we want. However, since the - library was loaded above, the second LoadLibrary does not execute - it's startup code unless it is first unloaded. */ - else if (h == d->handle) - { - if (unload) - { - FreeLibrary (h); - LoadLibraryW (d->name); - } - } - else if (try2) - api_fatal ("unable to remap %W to same address as parent(%p) != %p", - d->name, d->handle, h); + It stinks that we can't invert the order of the initial LoadLibrary + and FreeLibrar since Microsoft documentation seems to imply that + should do what we want. However, once a library is loaded as + above, the second LoadLibrary will not execute its startup code + unless it is first unloaded. */ + HMODULE h = LoadLibraryExW (d->name, NULL, DONT_RESOLVE_DLL_REFERENCES); + + if (!h) + system_printf ("can't reload %W, %E", d->name); else { - /* It loaded in the wrong place. Dunno why this happens but it - always seems to happen when there are multiple DLLs attempting - to load into the same address space. In the "forked" process, - the second DLL always loads into a different location. */ FreeLibrary (h); - /* Block all of the memory up to the new load address. */ - reserve_upto (d->name, (DWORD) d->handle); - try2 = 1; /* And try */ - continue; /* again. */ - } - /* If we reached here, and try2 is set, then there is a lot of - memory to release. */ - if (try2) - { - release_upto (d->name, (DWORD) d->handle); - try2 = 0; + if (h == d->handle) + h = LoadLibraryW (d->name); } + /* If we reached here on the second iteration of the for loop + then there is a lot of memory to release. */ + if (i > 0) + release_upto (d->name, (DWORD) d->handle); + if (h == d->handle) + break; /* Success */ + + if (i > 0) + /* We tried once to relocate the dll and it failed. */ + api_fatal ("unable to remap %W to same address as parent: %p != %p", + d->name, d->handle, h); + + /* Dll loaded in the wrong place. Dunno why this happens but it + always seems to happen when there are multiple DLLs attempting to + load into the same address space. In the "forked" process, the + second DLL always loads into a different location. So, block all + of the memory up to the new load address and try again. */ + reserve_upto (d->name, (DWORD) d->handle); } - next = d->next; /* Get the address of the next DLL. */ - } in_forkee = false; } diff --git a/winsup/cygwin/dll_init.h b/winsup/cygwin/dll_init.h index b6c24b2fb..9a712a3cc 100644 --- a/winsup/cygwin/dll_init.h +++ b/winsup/cygwin/dll_init.h @@ -51,8 +51,7 @@ struct dll HMODULE handle; int count; dll_type type; - int namelen; - WCHAR name[ANYSIZE_ARRAY]; + WCHAR name[1]; void detach (); int init (); }; @@ -73,7 +72,7 @@ public: dll *alloc (HINSTANCE, per_process *, dll_type); void detach (void *); void init (); - void load_after_fork (HANDLE, dll *); + void load_after_fork (HANDLE); dll *inext () { while ((hold = hold->next)) diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc index 6887448df..3692a7a54 100644 --- a/winsup/cygwin/fork.cc +++ b/winsup/cygwin/fork.cc @@ -34,7 +34,6 @@ details. */ class frok { - dll *first_dll; bool load_dlls; child_info_fork ch; const char *error; @@ -174,8 +173,7 @@ frok::child (volatile char * volatile here) myself->pid, myself->ppid, __builtin_frame_address (0)); sync_with_parent ("after longjmp", true); - sigproc_printf ("hParent %p, child 1 first_dll %p, load_dlls %d", hParent, - first_dll, load_dlls); + sigproc_printf ("hParent %p, load_dlls %d", hParent, load_dlls); /* If we've played with the stack, stacksize != 0. That means that fork() was invoked from other than the main thread. Make sure that @@ -230,7 +228,7 @@ frok::child (volatile char * volatile here) } else { - dlls.load_after_fork (hParent, first_dll); + dlls.load_after_fork (hParent); cygheap->fdtab.fixup_after_fork (hParent); sync_with_parent ("loaded dlls", true); } @@ -304,14 +302,13 @@ frok::parent (volatile char * volatile stack_here) else c_flags |= DETACHED_PROCESS; - /* Remember the address of the first loaded dll and decide - if we need to load dlls. We do this here so that this - information will be available in the parent and, when - the stack is copied, in the child. */ - first_dll = dlls.start.next; + /* Remember if we need to load dynamically linked dlls. + We do this here so that this information will be available + in the parent and, when the stack is copied, in the child. */ load_dlls = dlls.reload_on_fork && dlls.loaded_dlls; /* This will help some of the confusion. */ + /* FIXME: Is this really appropriate? What if stdout is closed? */ fflush (stdout); forker_finished = CreateEvent (&sec_all, FALSE, FALSE, NULL); @@ -570,7 +567,6 @@ fork () frok grouped; debug_printf ("entering"); - grouped.first_dll = NULL; grouped.load_dlls = 0; int res; diff --git a/winsup/cygwin/shared.cc b/winsup/cygwin/shared.cc index 20f61456f..aad6c9499 100644 --- a/winsup/cygwin/shared.cc +++ b/winsup/cygwin/shared.cc @@ -361,13 +361,13 @@ shared_info::initialize () SHARED_INFO_CB, cb); } -void __stdcall -memory_init () +void +memory_init (bool init_cygheap) { getpagesize (); /* Initialize the Cygwin heap, if necessary */ - if (!cygheap) + if (init_cygheap) { cygheap_init (); cygheap->user.init (); diff --git a/winsup/cygwin/shared_info.h b/winsup/cygwin/shared_info.h index e332829df..c92358f2c 100644 --- a/winsup/cygwin/shared_info.h +++ b/winsup/cygwin/shared_info.h @@ -79,7 +79,7 @@ enum shared_locations }; -void __stdcall memory_init (); +void memory_init (bool) __attribute__ ((regparm(1))); void __stdcall shared_destroy (); #define shared_align_past(p) \