From 977ad5434cc09034e7276127e6aa10c4f88d2e3e Mon Sep 17 00:00:00 2001 From: Christopher Faylor Date: Mon, 30 May 2011 06:52:12 +0000 Subject: [PATCH] * dll_init.cc (dll_list::find_by_modname): New function to search the dll list for a module name only (no path). (dll_list::alloc): Initialize newly-added members of struct dll. (dll_list::append): New function to factor out the append operation (used by dll_list::topsort). (dll_list::populate_deps): New function to identify dll dependencies. (dll_list::topsort): New function to sort the dll list topologically by dependencies. (dll_list::topsort_visit): New helper function for the above. * dll_init.h (dll::ndeps): New class member. (dll::deps): Ditto. (dll::modname): Ditto. (dll_list::find_by_modname): New function related to topsort. (dll_list::populate_all_deps): Ditto. (dll_list::populate_deps): Ditto. (dll_list::topsort): Ditto. (dll_list::topsort_visit): Ditto. (dll_list::append): Ditto. (pefile): New struct allowing simple introspection of dll images. * fork.cc (fork): Topologically sort the dll list before forking * child_info.h (CURR_CHILD_INFO_MAGIC): Refresh. (child_info::refresh_cygheap): New function. * spawn.cc (spawn_guts): Call refresh_cygheap before creating a new process to ensure that cygheap_max is up-to-date. * fork.cc (frok::parent): Ditto. --- winsup/cygwin/ChangeLog | 45 ++++++++-- winsup/cygwin/child_info.h | 3 +- winsup/cygwin/dll_init.cc | 176 +++++++++++++++++++++++++++++++------ winsup/cygwin/dll_init.h | 10 +++ winsup/cygwin/fork.cc | 7 +- winsup/cygwin/spawn.cc | 1 + 6 files changed, 207 insertions(+), 35 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 5d377ebbc..3dcca6997 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,34 @@ +2011-05-30 Ryan Johnson + + * dll_init.cc (dll_list::find_by_modname): New function to search the + dll list for a module name only (no path). + (dll_list::alloc): Initialize newly-added members of struct dll. + (dll_list::append): New function to factor out the append operation + (used by dll_list::topsort). + (dll_list::populate_deps): New function to identify dll dependencies. + (dll_list::topsort): New function to sort the dll list topologically by + dependencies. + (dll_list::topsort_visit): New helper function for the above. + * dll_init.h (dll::ndeps): New class member. + (dll::deps): Ditto. + (dll::modname): Ditto. + (dll_list::find_by_modname): New function related to topsort. + (dll_list::populate_all_deps): Ditto. + (dll_list::populate_deps): Ditto. + (dll_list::topsort): Ditto. + (dll_list::topsort_visit): Ditto. + (dll_list::append): Ditto. + (pefile): New struct allowing simple introspection of dll images. + * fork.cc (fork): Topologically sort the dll list before forking + +2011-05-30 Christopher Faylor + + * child_info.h (CURR_CHILD_INFO_MAGIC): Refresh. + (child_info::refresh_cygheap): New function. + * spawn.cc (spawn_guts): Call refresh_cygheap before creating a new + process to ensure that cygheap_max is up-to-date. + * fork.cc (frok::parent): Ditto. + 2011-05-30 Christopher Faylor * cygheap.cc (cygheap_dummy): Rename from cygheap_at_start. @@ -926,7 +957,7 @@ * fhandler.h (fhandler_dev_tape::_lock): Add bool parameter. * fhandler_tape.cc (lock): Call _lock with false argument. - (_lock): Take bool cancelable parameter. Handle O_NONBLOCK. + (_lock): Take bool cancelable parameter. Handle O_NONBLOCK. Make cancelable if cancelabe parameter is true. (fhandler_dev_tape::raw_read): Call _lock with true argument. (fhandler_dev_tape::raw_write): Ditto. @@ -1125,7 +1156,7 @@ (close): Ditto. (fsync): Ditto. * termios.cc (tcdrain): Ditto. - * thread.cc: Align list of cancellation points with above changes. + * thread.cc: Align list of cancellation points with above changes. Mark not-implemented functions, too. (cancelable_wait): Don't set unused object indices to WAIT_FAILED since that could result in wrong behaviour. Set them to the invalid @@ -1387,7 +1418,7 @@ * path.h (isproc_dev): Use FH_PROC_MAX_MINOR rather than FH_PROC. 2011-04-19 Peter Rosin - + * select.cc (pipe_cleanup): Make sure that device_specific_pipe is always deleted regardless of whether it has a unique thread associated with it. @@ -1422,7 +1453,7 @@ (get_registry_hive_path): Use RtlQueryRegistryValues to fetch path from registry. (load_registry_hive): Drop useless check for user hive being available. - Load hive using NtLoadKey. + Load hive using NtLoadKey. * registry.h: Accommodate above changes. * sched.cc (sched_rr_get_interval): Use wide char arguments in reg_key functions. @@ -2574,7 +2605,7 @@ to implement ondemand partition locking. (fhandler_dev_floppy::write_file): Call lock_partition from here if writing failed due to a potential write restriction on a disk - partition. + partition. (fhandler_dev_floppy::open): Don't lock partitions here. (fhandler_dev_floppy::close): Keep track of partition handle reference count. Close handles and remove partitions pointer ony if count is 0. @@ -2588,7 +2619,7 @@ method. * fhandler_floppy.cc (fhandler_dev_floppy::fhandler_dev_floppy): Zero out partitions array. - (fhandler_dev_floppy::open): Fix "entire disk" condition for call to + (fhandler_dev_floppy::open): Fix "entire disk" condition for call to DeviceIoControl (FSCTL_ALLOW_EXTENDED_DASD_IO). When opening disks for writing, call DeviceIoControl (FSCTL_LOCK_VOLUME) on all affected disk partitions starting with Vista. @@ -2611,4 +2642,4 @@ 2011-01-02 Christopher Faylor * ChangeLog-2010: Create from ChangeLog. - * ChangeLog: Start fresh. + * ChangeLog: Start fresh. diff --git a/winsup/cygwin/child_info.h b/winsup/cygwin/child_info.h index 806cd0842..ae236dbd3 100644 --- a/winsup/cygwin/child_info.h +++ b/winsup/cygwin/child_info.h @@ -38,7 +38,7 @@ enum child_status #define EXEC_MAGIC_SIZE sizeof(child_info) /* Change this value if you get a message indicating that it is out-of-sync. */ -#define CURR_CHILD_INFO_MAGIC 0x76ca2aaeU +#define CURR_CHILD_INFO_MAGIC 0xeef5640dU /* NOTE: Do not make gratuitous changes to the names or organization of the below class. The layout is checksummed to determine compatibility between @@ -65,6 +65,7 @@ public: child_info (unsigned, child_info_types, bool); child_info (): subproc_ready (NULL), parent (NULL) {} ~child_info (); + void refresh_cygheap () { cygheap_max = ::cygheap_max; } void ready (bool); bool sync (int, HANDLE&, DWORD) __attribute__ ((regparm (3))); DWORD proc_retry (HANDLE) __attribute__ ((regparm (2))); diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc index 060ff497e..44d7b7a52 100644 --- a/winsup/cygwin/dll_init.cc +++ b/winsup/cygwin/dll_init.cc @@ -116,6 +116,18 @@ dll_list::operator[] (const PWCHAR name) return NULL; } +/* Look for a dll based on is short name only (no path) */ +dll * +dll_list::find_by_modname (const PWCHAR name) +{ + dll *d = &start; + while ((d = d->next) != NULL) + if (!wcscasecmp (name, d->modname)) + return d; + + return NULL; +} + #define RETRIES 1000 /* Allocate space for a dll struct. */ @@ -161,15 +173,14 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type) d->handle = h; d->has_dtors = true; d->p = p; + d->ndeps = 0; + d->deps = NULL; + d->modname = wcsrchr (d->name, L'\\'); + if (d->modname) + d->modname++; d->image_size = ((pefile*)h)->optional_hdr ()->SizeOfImage; d->type = type; - if (end == NULL) - end = &start; /* Point to "end" of dll chain. */ - end->next = d; /* Standard linked list stuff. */ - d->next = NULL; - d->prev = end; - end = d; - tot++; + append (d); if (type == DLL_LOAD) loaded_dlls++; } @@ -178,6 +189,119 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type) return d; } +void +dll_list::append (dll* d) +{ + if (end == NULL) + end = &start; /* Point to "end" of dll chain. */ + end->next = d; /* Standard linked list stuff. */ + d->next = NULL; + d->prev = end; + end = d; + tot++; +} + +void dll_list::populate_deps (dll* d) +{ + WCHAR wmodname[NT_MAX_PATH]; + pefile* pef = (pefile*) d->handle; + PIMAGE_DATA_DIRECTORY dd = pef->idata_dir (IMAGE_DIRECTORY_ENTRY_IMPORT); + /* Annoyance: calling crealloc with a NULL pointer will use the + wrong heap and crash, so we have to replicate some code */ + long maxdeps = 4; + d->deps = (dll**) cmalloc (HEAP_2_DLL, maxdeps*sizeof (dll*)); + d->ndeps = 0; + for (PIMAGE_IMPORT_DESCRIPTOR id= + (PIMAGE_IMPORT_DESCRIPTOR) pef->rva (dd->VirtualAddress); + dd->Size && id->Name; + id++) + { + char* modname = pef->rva (id->Name); + sys_mbstowcs (wmodname, NT_MAX_PATH, modname); + if (dll* dep = find_by_modname (wmodname)) + { + if (d->ndeps >= maxdeps) + { + maxdeps = 2*(1+maxdeps); + d->deps = (dll**) crealloc (d->deps, maxdeps*sizeof (dll*)); + } + d->deps[d->ndeps++] = dep; + } + } + + /* add one to differentiate no deps from unknown */ + d->ndeps++; +} + + +void +dll_list::topsort () +{ + /* Anything to do? */ + if (!end) + return; + + /* make sure we have all the deps available */ + dll* d = &start; + while ((d = d->next)) + if (!d->ndeps) + populate_deps (d); + + /* unlink head and tail pointers so the sort can rebuild the list */ + d = start.next; + start.next = end = NULL; + topsort_visit (d, true); + + /* clear node markings made by the sort */ + d = &start; + while ((d = d->next)) + { + debug_printf ("%W", d->modname); + for (int i=1; i < -d->ndeps; i++) + debug_printf ("-> %W", d->deps[i-1]->modname); + + /* It would be really nice to be able to keep this information + around for next time, but we don't have an easy way to + invalidate cached dependencies when a module unloads. */ + d->ndeps = 0; + cfree (d->deps); + d->deps = NULL; + } +} + +/* A recursive in-place topological sort. The result is ordered so that + dependencies of a dll appear before it in the list. + + NOTE: this algorithm is guaranteed to terminate with a "partial + order" of dlls but does not do anything smart about cycles: an + arbitrary dependent dll will necessarily appear first. Perhaps not + surprisingly, Windows ships several dlls containing dependency + cycles, including SspiCli/RPCRT4.dll and a lovely tangle involving + USP10/LPK/GDI32/USER32.dll). Fortunately, we don't care about + Windows DLLs here, and cygwin dlls should behave better */ +void +dll_list::topsort_visit (dll* d, bool seek_tail) +{ + /* Recurse to the end of the dll chain, then visit nodes as we + unwind. We do this because once we start visiting nodes we can no + longer trust any _next_ pointers. + + We "mark" visited nodes (to avoid revisiting them) by negating + ndeps (undone once the sort completes). */ + if (seek_tail && d->next) + topsort_visit (d->next, true); + + if (d->ndeps > 0) + { + d->ndeps = -d->ndeps; + for (long i=1; i < -d->ndeps; i++) + topsort_visit (d->deps[i-1], false); + + append (d); + } +} + + dll * dll_list::find (void *retaddr) { @@ -322,7 +446,7 @@ reserve_at (const PWCHAR name, DWORD here, DWORD dll_base, DWORD dll_size) size = end - here; if (!VirtualAlloc ((void *) here, size, MEM_RESERVE, PAGE_NOACCESS)) api_fatal ("couldn't allocate memory %p(%d) for '%W' alignment, %E\n", - here, size, name); + here, size, name); return here; } @@ -332,7 +456,7 @@ release_at (const PWCHAR name, DWORD here) { if (!VirtualFree ((void *) here, 0, MEM_RELEASE)) api_fatal ("couldn't release memory %p for '%W' alignment, %E\n", - here, name); + here, name); } /* Reload DLLs after a fork. Iterates over the list of dynamically loaded @@ -368,13 +492,13 @@ dll_list::load_after_fork (HANDLE parent) /* 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); + { + release_upto (d->name, (DWORD) d->handle); - if (preferred_block) - release_at (d->name, preferred_block); - preferred_block = 0; - } + if (preferred_block) + release_at (d->name, preferred_block); + preferred_block = 0; + } if (h == d->handle) break; /* Success */ @@ -385,19 +509,19 @@ dll_list::load_after_fork (HANDLE parent) 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 with the - same base address. In the "forked" process, the relocated DLL - may load at a different address. So, block all of the memory up - to the relocated load address and try again. */ + always seems to happen when there are multiple DLLs with the + same base address. In the "forked" process, the relocated DLL + may load at a different address. So, block all of the memory up + to the relocated load address and try again. */ reserve_upto (d->name, (DWORD) d->handle); - /* Also, if the DLL loaded at a higher address than wanted (probably - it's base address), reserve the memory at that address. This can - happen if it couldn't load at the preferred base in the parent, but - can in the child, due to differences in the load ordering. - Block memory at it's preferred address and try again. */ - if ((DWORD) h > (DWORD) d->handle) - preferred_block = reserve_at (d->name, (DWORD) h, + /* Also, if the DLL loaded at a higher address than wanted (probably + it's base address), reserve the memory at that address. This can + happen if it couldn't load at the preferred base in the parent, but + can in the child, due to differences in the load ordering. + Block memory at it's preferred address and try again. */ + if ((DWORD) h > (DWORD) d->handle) + preferred_block = reserve_at (d->name, (DWORD) h, (DWORD) d->handle, d->image_size); } diff --git a/winsup/cygwin/dll_init.h b/winsup/cygwin/dll_init.h index dc5c57038..ea3f628a2 100644 --- a/winsup/cygwin/dll_init.h +++ b/winsup/cygwin/dll_init.h @@ -52,6 +52,9 @@ struct dll int count; bool has_dtors; dll_type type; + long ndeps; + dll** deps; + PWCHAR modname; DWORD image_size; WCHAR name[1]; void detach (); @@ -85,6 +88,13 @@ public: void detach (void *); void init (); void load_after_fork (HANDLE); + dll *find_by_modname (const PWCHAR name); + void populate_all_deps (); + void populate_deps (dll* d); + void topsort (); + void topsort_visit (dll* d, bool goto_tail); + void append (dll* d); + dll *inext () { while ((hold = hold->next)) diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc index 9e34da60c..1dbfebeed 100644 --- a/winsup/cygwin/fork.cc +++ b/winsup/cygwin/fork.cc @@ -397,6 +397,7 @@ frok::parent (volatile char * volatile stack_here) /* Remove impersonation */ cygheap->user.deimpersonate (); fix_impersonation = true; + ch.refresh_cygheap (); while (1) { @@ -601,7 +602,6 @@ extern "C" int fork () { frok grouped; - /* No cygheap allocation beyond this point. */ debug_printf ("entering"); grouped.load_dlls = 0; @@ -635,6 +635,11 @@ fork () return -1; } + /* Put the dll list in topological dependency ordering, in + hopes that the child will have a better shot at loading dlls + properly if it only has to deal with one at a time. */ + dlls.topsort (); + ischild = !!setjmp (grouped.ch.jmp); volatile char * volatile esp; diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc index df1eb1a68..970f0af3a 100644 --- a/winsup/cygwin/spawn.cc +++ b/winsup/cygwin/spawn.cc @@ -560,6 +560,7 @@ spawn_guts (const char *prog_arg, const char *const *argv, || cygheap->fdtab.need_fixup_before ())) c_flags |= CREATE_SUSPENDED; + ch.refresh_cygheap (); /* When ruid != euid we create the new process under the current original account and impersonate in child, this way maintaining the different effective vs. real ids. -- 2.43.5