On 05/04/2011 17:21, Christopher Faylor wrote:
> On Tue, Apr 05, 2011 at 05:03:43PM +0100, Jon TURNEY wrote:
>> On 04/04/2011 15:39, Christopher Faylor wrote:
> I'm trying to imagine a scenario where it would screw up to just do the
> reserve_upto + "reserve the low block" and I can't think of one. It's
> potentially a little more work, of course, but I think it may catch the
> more common failing conditions so it shouldn't be too noticeable.
>
>>> If so, it seems like we're allocating and freeing the space up to the DLL
>>> more
>>> than once. I think we could avoid doing that.
>>
>> For performance reasons, I think you are right. Or do you mean there is a
>> correctness issue with that?
>>
>> If you indicate your preferences I'll respin the patch.
>>
>> 1) Combine passes 2 and 3
>
> I'd prefer this. If we can get people test the snapshot maybe we an
> figure out if a separate loop is useful.
Updated patch attached.
2011-04-06 Jon TURNEY <jon.tur...@dronecode.org.uk>
* dll_init.cc (reserve_at, release_at): New functions.
(load_after_fork): If the DLL was loaded higher than the required
address, assume that it loaded at it's base address and also reserve
memory there to force it to be relocated.
Index: cygwin/dll_init.cc
===================================================================
--- cygwin/dll_init.cc.orig 2011-04-06 10:32:08.242187500 +0100
+++ cygwin/dll_init.cc 2011-04-06 10:44:33.250000000 +0100
@@ -257,12 +257,44 @@
}
}
+/* Mark one page at "here" as reserved. This may force
+ Windows NT to load a DLL elsewhere. */
+static DWORD
+reserve_at (const PWCHAR name, DWORD here)
+{
+ DWORD size;
+ MEMORY_BASIC_INFORMATION mb;
+
+ if (!VirtualQuery ((void *) here, &mb, sizeof (mb)))
+ size = 64 * 1024;
+
+ if (mb.State != MEM_FREE)
+ return 0;
+
+ size = mb.RegionSize;
+ 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);
+ return here;
+}
+
+/* Release the memory previously allocated by "reserve_at" above. */
+static void
+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);
+}
+
/* 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. */
void
dll_list::load_after_fork (HANDLE parent)
{
+ DWORD preferred_block = 0;
+
for (dll *d = &dlls.start; (d = d->next) != NULL; )
if (d->type == DLL_LOAD)
for (int i = 0; i < 2; i++)
@@ -284,10 +316,18 @@
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);
+ {
+ release_upto (d->name, (DWORD) d->handle);
+
+ if (preferred_block)
+ release_at(d->name, preferred_block);
+ preferred_block = 0;
+ }
+
if (h == d->handle)
break; /* Success */
@@ -297,11 +337,20 @@
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. */
+ 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);
+
}
in_forkee = false;
}
