[newlib-cygwin] Preserve order of dlopen'd modules in dll_list::topsort

[Corinna Vinschen <corinna at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=226f69422a44bbd177da6d8f3e1771f3b786fb0c

commit 226f69422a44bbd177da6d8f3e1771f3b786fb0c
Author: David Allsopp <david.allsopp@metastack.com>
Date:   Mon Feb 27 17:06:34 2017 +0000

    Preserve order of dlopen'd modules in dll_list::topsort
    
    This patch 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.
    
    Signed-off-by: David Allsopp <david.allsopp@metastack.com>

Diff:
---
 winsup/CONTRIBUTORS         |  1 +
 winsup/cygwin/dll_init.cc   | 53 ++++++++++++++++++++++++++++++++++++++++-----
 winsup/cygwin/release/2.7.1 |  2 ++
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/winsup/CONTRIBUTORS b/winsup/CONTRIBUTORS
index 84b4fa8..635d8f5 100644
--- a/winsup/CONTRIBUTORS
+++ b/winsup/CONTRIBUTORS
@@ -34,6 +34,7 @@ POSSIBILITY OF SUCH DAMAGE.
 
 Individuals submitting their patches under 2-clause BSD:
 
+David Allsopp				David.Allsopp@cl.cam.ac.uk
 Erik M. Bray				erik.bray@lri.fr
 
 =========================================================================
diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc
index 0fe5714..86776f2 100644
--- a/winsup/cygwin/dll_init.cc
+++ b/winsup/cygwin/dll_init.cc
@@ -271,9 +271,16 @@ void dll_list::populate_deps (dll* d)
   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;
+  long maxdeps;
+  if (!d->ndeps)
+    {
+      maxdeps = 4;
+      d->deps = (dll**) cmalloc (HEAP_2_DLL, maxdeps*sizeof (dll*));
+    }
+  else
+    {
+      maxdeps = d->ndeps;
+    }
   for (PIMAGE_IMPORT_DESCRIPTOR id=
 	(PIMAGE_IMPORT_DESCRIPTOR) pef->rva (dd->VirtualAddress);
       dd->Size && id->Name;
@@ -306,9 +313,45 @@ dll_list::topsort ()
 
   /* make sure we have all the deps available */
   dll* d = &start;
+  dll** dlopen_deps = NULL;
+  long maxdeps = 4;
+  long dlopen_ndeps = 0;
+
+  if (loaded_dlls > 0)
+    dlopen_deps = (dll**) cmalloc (HEAP_2_DLL, maxdeps*sizeof (dll*));
+
   while ((d = d->next))
-    if (!d->ndeps)
-      populate_deps (d);
+    {
+      if (!d->ndeps)
+        {
+          /* Ensure that all dlopen'd DLLs depend on previously dlopen'd DLLs.
+             This prevents topsort from reversing the order of dlopen'd DLLs on
+             calls to fork. */
+          if (d->type == DLL_LOAD)
+            {
+              /* Initialise d->deps with all previously dlopen'd DLLs. */
+              if (dlopen_ndeps)
+                {
+                  d->ndeps = dlopen_ndeps;
+                  d->deps = (dll**) cmalloc (HEAP_2_DLL,
+                                             dlopen_ndeps*sizeof (dll*));
+                  memcpy (d->deps, dlopen_deps, dlopen_ndeps*sizeof (dll*));
+                }
+              /* Add this DLL to the list of previously dlopen'd DLLs. */
+              if (dlopen_ndeps >= maxdeps)
+                {
+                  maxdeps = 2*(1+maxdeps);
+                  dlopen_deps = (dll**) crealloc (dlopen_deps,
+						  maxdeps*sizeof (dll*));
+                }
+              dlopen_deps[dlopen_ndeps++] = d;
+            }
+          populate_deps (d);
+        }
+    }
+
+  if (loaded_dlls > 0)
+    cfree (dlopen_deps);
 
   /* unlink head and tail pointers so the sort can rebuild the list */
   d = start.next;
diff --git a/winsup/cygwin/release/2.7.1 b/winsup/cygwin/release/2.7.1
index 54e1100..411a0ae 100644
--- a/winsup/cygwin/release/2.7.1
+++ b/winsup/cygwin/release/2.7.1
@@ -8,6 +8,8 @@ What changed:
 - cygcheck and strace now always generate output with Unix LF line endings,
   rather than with DOS/Windows CR LF line endings.
 
+- fork now preserves the load order of unrelated dlopen'd modules.
+
 
 Bug Fixes
 ---------