[PATCH 0/1] MR1713: loader: In macOS preloader, set NXArgc, NXArgv, and environ on i386 and x86_64.

[Brendan Shanks (＠bshanks)]

This resolves a bug seen on i386 where `environ` was not being correctly set in the launched process, causing `environ[0]` to be lost. Setting `NXArgc`/`NXArgv` doesn't fix any known bug but seems like a good idea.

See https://gitlab.winehq.org/bshanks/mac-preloader-testbed for a standalone test app.

[PATCH 1/1] loader: In macOS preloader, set NXArgc, NXArgv, and environ on i386 and x86_64.

[Brendan Shanks]

From: Brendan Shanks

---
 loader/preloader_mac.c | 49 ++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/loader/preloader_mac.c b/loader/preloader_mac.c
index bb9336ead2c..5df185d0960 100644
--- a/loader/preloader_mac.c
+++ b/loader/preloader_mac.c
@@ -640,6 +640,34 @@ static void fixup_stack( void *stack )
     memmove(&argv[0], &argv[1], (char *)apple_end - (char *)&argv[1]);
 }

+static void set_program_vars( void *stack, void *mod )
+{
+    int *pargc;
+    char **argv, **env;
+    int *wine_NXArgc = pdlsym( mod, "NXArgc" );
+    char ***wine_NXArgv = pdlsym( mod, "NXArgv" );
+    char ***wine_environ = pdlsym( mod, "environ" );
+
+    pargc = stack;
+    argv = (char **)pargc + 1;
+    env = &argv[*pargc-1] + 2;
+
+    if (wine_NXArgc)
+        *wine_NXArgc = *pargc;
+    else
+        wld_printf( "preloader: Warning: failed to set NXArgc\n" );
+
+    if (wine_NXArgv)
+        *wine_NXArgv = argv;
+    else
+        wld_printf( "preloader: Warning: failed to set NXArgv\n" );
+
+    if (wine_environ)
+        *wine_environ = env;
+    else
+        wld_printf( "preloader: Warning: failed to set environ\n" );
+}
+
 void *wld_start( void *stack, int *is_unix_thread )
 {
     struct wine_preload_info builtin_dlls = { (void *)0x7a000000, 0x02000000 };
@@ -704,21 +732,14 @@ void *wld_start( void *stack, int *is_unix_thread )
     /* decrement argc and "remove" argv[0] */
     fixup_stack(stack);

-#if defined(__x86_64__)
-    /* For LC_UNIXTHREAD binaries on Monterey and later, 'environ' is not set and is NULL.
-     * Set the correct value here.
+    /* Set NXArgc, NXArgv, and environ in the new binary.
+     * On different configurations these were either NULL/0 or still had their
+     * values from this preloader's launch.
+     *
+     * In particular, environ was not being updated, resulting in environ[0] being lost.
+     * And for LC_UNIXTHREAD binaries on Monterey and later, environ was just NULL.
      */
-    if (*is_unix_thread)
-    {
-        char **env, ***wine_environ = pdlsym( mod, "environ" );
-
-        pargc = stack;
-        argv = (char **)pargc + 1;
-        env = &argv[*pargc-1] + 2;
-
-        *wine_environ = env;
-    }
-#endif
+    set_program_vars( stack, mod );

return entry;
 }

[PATCH 1/1] loader: In macOS preloader, set NXArgc, NXArgv, and environ on i386 and x86_64.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=127277

Your paranoid android.

=== debian11 (32 bit report) ===

winhttp:
notification.c:122: Test failed: 943: expected callback to be called from the same thread
notification.c:122: Test failed: 948: expected callback to be called from the same thread
notification.c:122: Test failed: 953: expected callback to be called from the same thread
notification.c:997: Test failed: got 4
notification.c:1006: Test failed: got 0
notification.c:1010: Test failed: unexpected data 0 at 0
notification.c:1017: Test failed: got 0
notification.c:1018: Test failed: got 1002
notification.c:1019: Test failed: got 18
notification.c:1030: Test failed: got 1002
notification.c:122: Test failed: 937: expected callback to be called from the same thread
notification.c:122: Test failed: 943: expected callback to be called from the same thread
notification.c:122: Test failed: 948: expected callback to be called from the same thread
notification.c:122: Test failed: 953: expected callback to be called from the same thread

[PATCH 0/1] MR1713: loader: In macOS preloader, set NXArgc, NXArgv, and environ on i386 and x86_64. - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.