Hi David, On Feb 25 16:27, David Allsopp wrote: > This patch (below - I hope I have managed to format this email correctly) > alters the behaviour of dll_list::topsort to preserve the order of dlopen'd > units. > > The load order of unrelated DLLs is reversed every time fork is called, > since dll_list::topsort finds the tail of the list and then unwinds to > reinsert items. My change takes advantage of what should be undefined > behaviour in dll_list::populate_deps (ndeps non-zero and ndeps and deps not > initialised) to allow the deps field to be initialised prior to the call and > appended to, rather than overwritten. > > All DLLs which have been dlopen'd have their deps list initialised with the > list of all previously dlopen'd units. These extra dependencies mean that > the unwind preserves the order of dlopen'd units. > > The motivation for this is the FlexDLL linker used in OCaml. The FlexDLL > linker allows a dlopen'd unit to refer to symbols in previously dlopen'd > units and it resolves these symbols in DllMain before anything else has > initialised (including the Cygwin DLL). This means that dependencies may > exist between dlopen'd units (which the OCaml runtime system understands) > but which Windows is unaware of. During fork, the process-level table which > FlexDLL uses to get the symbol table of each DLL is copied over but because > the load order of dlopen'd DLLs is reversed, it is possible for FlexDLL to > attempt to access memory in the DLL before it has been loaded and hence it > fails with an access violation. Because the list is reversed on each call to > fork, it means that a subsequent call to fork puts the DLLs back into the > correct order, hence "even" invocations of fork work! > > An interesting side-effect is that this only occurs if the DLLs load at > their preferred base address - if they have to be rebased, then FlexDLL > works because at the time that the dependent unit is loaded out of order, > there is still in memory the "dummy" DONT_RESOLVE_DLL_REFERENCES version of > the dependency which, as it happens, will contain the correct symbol table > in the data section. For my tests, this initially appeared to be an x86-only > problem, but that was only because the two DLLs on x64 should have been > rebased. > > I'm very happy to include the complete detail for this and, for the > extremely keen, the relevant Git branch in OCaml which demonstrates this > problem. Given the way in which FlexDLL operates, I would contend that this > is a sensible change of behaviour for the Cygwin DLL, though not a bug fix. > I'd be extremely happy to see this patch integrated, as the workaround > necessary in FlexDLL to support Cygwin's fork is horrible (and > non-transparent to the library user). > > This patch is licensed under 2-clause BSD as per winsup/CONTRIBUTORS, > Copyright (c) 2017, MetaStack Solutions Ltd.
First of all, I think this makes perfect sense. I just have a few questions in terms of the patch itself. - Your browser inserts undesired line breaks, so the patch is broken. Can you please resend the `git format-patch' output as attachment? - While you're at it, please reformat your patch so the line length is not longer than 80 chars. - Last but not least. You add code to topsort so the loaded DLLs are handled first. The subsequent code is untouched. However, shouldn't the next loop then restrict calling populate_deps to the linked DLLs only, at least for performance? Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat
signature.asc
Description: PGP signature