[PATCH 0/1] MR2879: loader: In macOS preloader, don't unmap the reservation for builtin DLLs.

[Tim Clem (＠tclem)]

This seems to be a holdover from when ntdll was a Mach-O winelib
binary, so the reservation needed to be removed before it was
dlopen'd. Now that it's PE, unmapping the reservation is unnecessary
and leaves time for the system to reclaim it, thus requiring the
potentially problematic relocation of ntdll.

[PATCH 1/1] loader: In macOS preloader, don't unmap the reservation for builtin DLLs.

[Tim Clem]

From: Tim Clem

This seems to be a holdover from when ntdll was a Mach-O winelib
binary, so the reservation needed to be removed before it was
dlopen'd. Now that it's PE, unmapping the reservation is unnecessary
and leaves time for the system to reclaim it, thus requiring the
potentially problematic relocation of ntdll.
---
 loader/preloader_mac.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/loader/preloader_mac.c b/loader/preloader_mac.c
index 8e249a7dc4d..da666b20551 100644
--- a/loader/preloader_mac.c
+++ b/loader/preloader_mac.c
@@ -90,10 +90,12 @@ static struct wine_preload_info preload_info[] =
     { (void *)0x00001000, 0x0000f000 },  /* low 64k */
     { (void *)0x00010000, 0x00100000 },  /* DOS area */
     { (void *)0x00110000, 0x67ef0000 },  /* low memory area */
+    { (void *)0x7a000000, 0x02000000 },  /* builtin DLLs (ntdll, kernel32) */
     { (void *)0x7f000000, 0x03000000 },  /* top-down allocations + shared heap + virtual heap */
 #else  /* __i386__ */
     { (void *)0x000000010000, 0x00100000 },  /* DOS area */
     { (void *)0x000000110000, 0x67ef0000 },  /* low memory area */
+    { (void *)0x00007a000000, 0x02000000 },  /* builtin DLLs (ntdll, kernel32) */
     { (void *)0x00007ff00000, 0x000f0000 },  /* shared user data */
     { (void *)0x000100000000, 0x14000000 },  /* WINE_4GB_RESERVE section */
     { (void *)0x7ff000000000, 0x01ff0000 },  /* top-down allocations + virtual heap */
@@ -653,7 +655,6 @@ static void set_program_vars( void *stack, void *mod )

void *wld_start( void *stack, int *is_unix_thread )
 {
-    struct wine_preload_info builtin_dlls = { (void *)0x7a000000, 0x02000000 };
     struct wine_preload_info **wine_main_preload_info;
     char **argv, **p, *reserve = NULL;
     struct target_mach_header *mh;
@@ -692,16 +693,10 @@ void *wld_start( void *stack, int *is_unix_thread )
         }
     }

-    if (!map_region( &builtin_dlls ))
-        builtin_dlls.size = 0;
-
     /* load the main binary */
     if (!(mod = pdlopen( argv[1], RTLD_NOW )))
         fatal_error( "%s: could not load binary\n", argv[1] );

-    if (builtin_dlls.size)
-        wld_munmap( builtin_dlls.addr, builtin_dlls.size );
-
     /* store pointer to the preload info into the appropriate main binary variable */
     wine_main_preload_info = pdlsym( mod, "wine_main_preload_info" );
     if (wine_main_preload_info) *wine_main_preload_info = preload_info;

[Zebediah Figura (＠zfigura)]

Are we dropping support for non-PE builds yet?

[Alexandre Julliard (＠julliard)]

> Are we dropping support for non-PE builds yet?

I don't think we can support non-PE builds for 32-bit macOS.

[Zebediah Figura (＠zfigura)]

> I don't think we can support non-PE builds for 32-bit macOS.

Doesn't this affect 64-bit as well? Or am I misreading?

(Granted, I can see the argument that 64-bit-only Wine is not interesting enough to retain support for. Or even that non-PE Wine is not interesting enough to retain support for in general. I'd just like to make it clear what requirements we are imposing.)

[Alexandre Julliard (＠julliard)]

> Doesn't this affect 64-bit as well? Or am I misreading?

The 64-bit ntdll and kernel32 are loaded somewhere else, so it won't make any difference for them.

It also shouldn't make any difference for non-PE 32-bit, since that address range is apparently unavailable already.

[Tim Clem (＠tclem)]

> Doesn't this affect 64-bit as well? Or am I misreading?

I added the range to both the 32- and 64-bit reservations for two reasons: it was happening unconditionally before this patch, and I assumed (perhaps wrongly) it's useful for wow.
> It also shouldn't make any difference for non-PE 32-bit, since that address range is apparently unavailable already.

For whatever reason, non-PE 32-bit ntdll does tend to get put where it wants, but I was testing that with an old Wine version and it's nondeterministic anyway. It does seem that even in that case it's unmapped early enough that there's a period where the system can reclaim the space.

[Tim Clem (＠tclem)]

Actually, this has some unforeseen consequences in 64-bit. Early heap allocations now find the `builtin_dlls` reserved area (since it's large enough for them) and will claim bits of it before ntdll is mapped. Oof... will update with a way to handle that.

[PATCH 0/1] MR2879: loader: In macOS preloader, don't unmap the reservation for builtin DLLs. - closed

[Tim Clem (＠tclem)]

This merge request was closed by Tim Clem.

[Tim Clem (＠tclem)]

There's another (less controversial) change that needs to happen before this for WOW to not have issues. I'm going to close this, and will open another MR when the prerequisite is in place.