Improvements to fork handling (3/5)

Improvements to fork handling (3/5)

[Ryan Johnson]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

Hi all,

This patch fixes a bug in the reserve_at function which caused it to 
sometimes reserve space needed by the dll it was supposed to help land. 
This happens when the dll tries to land in a free region which overlaps 
the desired location. The new code exploits the image introspection 
(patch #2) to get the dll's image size and avoids the corner cases.

Ryan


[-- Attachment #2: fork-reserve-at.patch --]
[-- Type: text/plain, Size: 2405 bytes --]

diff --git a/dll_init.cc b/dll_init.cc
--- a/dll_init.cc
+++ b/dll_init.cc
@@ -164,6 +164,7 @@ dll_list::alloc (HINSTANCE h, per_proces
       d->handle = h;
       d->has_dtors = true;
       d->p = p;
+      d->image_size = ((pefile*)h)->optional_hdr ()->SizeOfImage;
       d->ndeps = 0;
       d->deps = NULL;
       d->modname = wcsrchr (d->name, L'\\');
@@ -407,21 +408,33 @@ release_upto (const PWCHAR name, DWORD h
       }
 }
 
-/* Mark one page at "here" as reserved.  This may force
-   Windows NT to load a DLL elsewhere. */
+/* Reserve the chunk of free address space starting _here_ and (usually)
+   covering at least _dll_size_ bytes. However, we must take care not
+   to clobber the dll's target address range because it often overlaps.
+ */
 static DWORD
-reserve_at (const PWCHAR name, DWORD here)
+reserve_at (const PWCHAR name, DWORD here, DWORD dll_base, DWORD dll_size)
 {
   DWORD size;
   MEMORY_BASIC_INFORMATION mb;
 
   if (!VirtualQuery ((void *) here, &mb, sizeof (mb)))
-    size = 64 * 1024;
-
+    api_fatal ("couldn't examine memory at %08lx while mapping %W, %E",
+	       here, name);
   if (mb.State != MEM_FREE)
     return 0;
 
   size = mb.RegionSize;
+  
+  // don't clobber the space where we want the dll to land
+  DWORD end = here + size;
+  DWORD dll_end = dll_base + dll_size;
+  if (dll_base < here && dll_end > here)
+      here = dll_end; // the dll straddles our left edge
+  else if (dll_base >= here && dll_base < end)
+      end = dll_base; // the dll overlaps partly or fully to our right
+  
+  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);
@@ -499,7 +512,8 @@ dll_list::load_after_fork (HANDLE parent
              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);
+            preferred_block = reserve_at (d->name, (DWORD) h,
+					  (DWORD) d->handle, d->image_size);
 
 	}
 }
diff --git a/dll_init.h b/dll_init.h
--- a/dll_init.h
+++ b/dll_init.h
@@ -52,6 +52,7 @@ struct dll
   int count;
   bool has_dtors;
   dll_type type;
+  DWORD image_size;
   long ndeps;
   dll** deps;
   PWCHAR modname;

[-- Attachment #3: fork-reserve-at.changes --]
[-- Type: text/plain, Size: 341 bytes --]

        * dll_init.cc (dll_list::alloc): initialize dll::image_size.
        (reserve_at): no longer reserves space needed by the target dll if
        the latter overlaps the free region to be blocked.
        (dll_list::load_after_fork): use new version of reserve_at.
        * dll_init.h (struct dll): add new members to track dll size.

Re: Improvements to fork handling (3/5)

[Christopher Faylor]

On Wed, May 11, 2011 at 02:32:09PM -0400, Ryan Johnson wrote:
>This patch fixes a bug in the reserve_at function which caused it to 
>sometimes reserve space needed by the dll it was supposed to help land. 
>This happens when the dll tries to land in a free region which overlaps 
>the desired location. The new code exploits the image introspection 
>(patch #2) to get the dll's image size and avoids the corner cases.

I've installed this patch, eliminating any depencencies on patch 2/5.

Btw, please reread the guidelines for ChangeLogs and model your changelog
entries on what you see in the current ChangeLog and the submissions you
see here.

The ChangeLog shouldn't be sent as an attachment.  It should have the
"header" including your name and date.  The tense shouldn't be "Changed
x" but "Change X".  Start entries after the colon with an uppercase
letter.  The changelog shouldn't be excessively wordy (that's for
comments:

http://www.gnu.org/prep/standards/html_node/Change-Logs.html

Thanks for the patch.

cgf