[PATCH] libiberty: On Windows pass a >32k cmdline through a response file.

[PATCH] libiberty: On Windows pass a >32k cmdline through a response file.

[Costas Argyris]

[-- Attachment #1.1: Type: text/plain, Size: 335 bytes --]

Currently on Windows, when CreateProcess is called with a command-line
that exceeds the 32k Windows limit, we get a very bad error:

"CreateProcess: No such file or directory"

This patch detects the case where this would happen and writes the
long command-line to a temporary response file and calls CreateProcess
with @file instead.

[-- Attachment #2: 0001-libiberty-On-Windows-pass-a-32k-cmdline-through-a-re.patch --]
[-- Type: text/x-patch, Size: 5493 bytes --]

From 5c7237c102cdaca34e5907cd25c31610bda51919 Mon Sep 17 00:00:00 2001
From: Costas Argyris <costas.argyris@gmail.com>
Date: Mon, 22 May 2023 13:55:56 +0100
Subject: [PATCH] libiberty: On Windows, pass a >32k cmdline through a response
 file.

pex-win32.c (win32_spawn): If the command line for CreateProcess
exceeds the 32k Windows limit, try to store it in a temporary
response file and call CreateProcess with @file instead (PR71850).

Signed-off-by: Costas Argyris <costas.argyris@gmail.com>
---
 libiberty/pex-win32.c | 57 +++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 13 deletions(-)

diff --git a/libiberty/pex-win32.c b/libiberty/pex-win32.c
index 23c6c190a2c..0fd8b38734c 100644
--- a/libiberty/pex-win32.c
+++ b/libiberty/pex-win32.c
@@ -569,7 +569,8 @@ env_compare (const void *a_ptr, const void *b_ptr)
  * target is not actually an executable, such as if it is a shell script. */
 
 static pid_t
-win32_spawn (const char *executable,
+win32_spawn (struct pex_obj *obj,
+         const char *executable,
 	     BOOL search,
 	     char *const *argv,
              char *const *env, /* array of strings of the form: VAR=VALUE */
@@ -624,8 +625,37 @@ win32_spawn (const char *executable,
   cmdline = argv_to_cmdline (argv);
   if (!cmdline)
     goto exit;
-    
-  /* Create the child process.  */  
+  /* If cmdline is too large, CreateProcess will fail with a bad
+     'No such file or directory' error. Try passing it through a
+     temporary response file instead.  */
+  if (strlen (cmdline) > 32767)
+    {
+      char *response_file = make_temp_file ("");
+      /* Register the file for deletion by pex_free.  */
+      ++obj->remove_count;
+      obj->remove = XRESIZEVEC (char *, obj->remove, obj->remove_count);
+      obj->remove[obj->remove_count - 1] = response_file;
+      int fd = pex_win32_open_write (obj, response_file, 0, 0);
+      if (fd == -1)
+        goto exit;
+      FILE *f = pex_win32_fdopenw (obj, fd, 0);
+      /* Don't write argv[0] (program name) to the response file.  */
+      if (writeargv (&argv[1], f))
+        {
+          fclose (f);
+          goto exit;
+        }
+      fclose (f); /* Also closes fd and the underlying OS handle.  */
+      char *response_arg = concat ("@", response_file, NULL);
+      char *response_argv[3] = {argv[0], response_arg, NULL};
+      free (cmdline);
+      cmdline = argv_to_cmdline (response_argv);
+      free (response_arg);
+      if (!cmdline)
+        goto exit;
+    }
+  
+  /* Create the child process.  */
   if (CreateProcess (full_executable, cmdline,
 		      /*lpProcessAttributes=*/NULL,
 		      /*lpThreadAttributes=*/NULL,
@@ -645,7 +675,7 @@ win32_spawn (const char *executable,
   free (env_block);
   free (cmdline);
   free (full_executable);
-
+  
   return pid;
 }
 
@@ -653,7 +683,8 @@ win32_spawn (const char *executable,
    This function is called as a fallback if win32_spawn fails. */
 
 static pid_t
-spawn_script (const char *executable, char *const *argv,
+spawn_script (struct pex_obj *obj,
+              const char *executable, char *const *argv,
               char* const *env,
 	      DWORD dwCreationFlags,
 	      LPSTARTUPINFO si,
@@ -703,20 +734,20 @@ spawn_script (const char *executable, char *const *argv,
 	      executable = strrchr (executable1, '\\') + 1;
 	      if (!executable)
 		executable = executable1;
-	      pid = win32_spawn (executable, TRUE, argv, env,
+	      pid = win32_spawn (obj, executable, TRUE, argv, env,
 				 dwCreationFlags, si, pi);
 #else
 	      if (strchr (executable1, '\\') == NULL)
-		pid = win32_spawn (executable1, TRUE, argv, env,
+		pid = win32_spawn (obj, executable1, TRUE, argv, env,
 				   dwCreationFlags, si, pi);
 	      else if (executable1[0] != '\\')
-		pid = win32_spawn (executable1, FALSE, argv, env,
+		pid = win32_spawn (obj, executable1, FALSE, argv, env,
 				   dwCreationFlags, si, pi);
 	      else
 		{
 		  const char *newex = mingw_rootify (executable1);
 		  *avhere = newex;
-		  pid = win32_spawn (newex, FALSE, argv, env,
+		  pid = win32_spawn (obj, newex, FALSE, argv, env,
 				     dwCreationFlags, si, pi);
 		  if (executable1 != newex)
 		    free ((char *) newex);
@@ -726,7 +757,7 @@ spawn_script (const char *executable, char *const *argv,
 		      if (newex != executable1)
 			{
 			  *avhere = newex;
-			  pid = win32_spawn (newex, FALSE, argv, env,
+			  pid = win32_spawn (obj, newex, FALSE, argv, env,
 					     dwCreationFlags, si, pi);
 			  free ((char *) newex);
 			}
@@ -745,7 +776,7 @@ spawn_script (const char *executable, char *const *argv,
 /* Execute a child.  */
 
 static pid_t
-pex_win32_exec_child (struct pex_obj *obj ATTRIBUTE_UNUSED, int flags,
+pex_win32_exec_child (struct pex_obj *obj, int flags,
 		      const char *executable, char * const * argv,
                       char* const* env,
 		      int in, int out, int errdes,
@@ -841,10 +872,10 @@ pex_win32_exec_child (struct pex_obj *obj ATTRIBUTE_UNUSED, int flags,
   si.hStdError = stderr_handle;
 
   /* Create the child process.  */  
-  pid = win32_spawn (executable, (flags & PEX_SEARCH) != 0,
+  pid = win32_spawn (obj, executable, (flags & PEX_SEARCH) != 0,
 		     argv, env, dwCreationFlags, &si, &pi);
   if (pid == (pid_t) -1)
-    pid = spawn_script (executable, argv, env, dwCreationFlags,
+    pid = spawn_script (obj, executable, argv, env, dwCreationFlags,
                         &si, &pi);
   if (pid == (pid_t) -1)
     {
-- 
2.30.2

Re: [PATCH] libiberty: On Windows pass a >32k cmdline through a response file.

[Jonathan Yong]

On 5/22/23 13:25, Costas Argyris wrote:
> Currently on Windows, when CreateProcess is called with a command-line
> that exceeds the 32k Windows limit, we get a very bad error:
> 
> "CreateProcess: No such file or directory"
> 
> This patch detects the case where this would happen and writes the
> long command-line to a temporary response file and calls CreateProcess
> with @file instead.
> 

Looks OK to me.

I will commit it around next week if there are no objections.

Re: [PATCH] libiberty: On Windows pass a >32k cmdline through a response file.

[Jonathan Yong]

On 5/23/23 08:21, Jonathan Yong wrote:
> On 5/22/23 13:25, Costas Argyris wrote:
>> Currently on Windows, when CreateProcess is called with a command-line
>> that exceeds the 32k Windows limit, we get a very bad error:
>>
>> "CreateProcess: No such file or directory"
>>
>> This patch detects the case where this would happen and writes the
>> long command-line to a temporary response file and calls CreateProcess
>> with @file instead.
>>
> 
> Looks OK to me.
> 
> I will commit it around next week if there are no objections.
> 

Done, pushed to master, thanks.

Re: [PATCH] libiberty: On Windows pass a >32k cmdline through a response file.

[Costas Argyris]

[-- Attachment #1.1: Type: text/plain, Size: 746 bytes --]

Thanks, here is the follow up patch for a couple typos in the same file.

On Mon, 5 Jun 2023 at 09:12, Jonathan Yong <10walls@gmail.com> wrote:

> On 5/23/23 08:21, Jonathan Yong wrote:
> > On 5/22/23 13:25, Costas Argyris wrote:
> >> Currently on Windows, when CreateProcess is called with a command-line
> >> that exceeds the 32k Windows limit, we get a very bad error:
> >>
> >> "CreateProcess: No such file or directory"
> >>
> >> This patch detects the case where this would happen and writes the
> >> long command-line to a temporary response file and calls CreateProcess
> >> with @file instead.
> >>
> >
> > Looks OK to me.
> >
> > I will commit it around next week if there are no objections.
> >
>
> Done, pushed to master, thanks.
>
>

[-- Attachment #2: 0001-libiberty-pex-win32.c-Fix-some-typos.patch --]
[-- Type: text/x-patch, Size: 1293 bytes --]

From 45c18cf113585aa0b03512a459e757c7aaef69ce Mon Sep 17 00:00:00 2001
From: Costas Argyris <costas.argyris@gmail.com>
Date: Mon, 5 Jun 2023 10:03:11 +0100
Subject: [PATCH] libiberty: pex-win32.c: Fix some typos.

Signed-off-by: Costas Argyris <costas.argyris@gmail.com>
---
 libiberty/pex-win32.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libiberty/pex-win32.c b/libiberty/pex-win32.c
index 0fd8b38734c..f7fe306036b 100644
--- a/libiberty/pex-win32.c
+++ b/libiberty/pex-win32.c
@@ -351,7 +351,7 @@ argv_to_cmdline (char *const *argv)
 	 prevent wasting 2 chars per argument of the CreateProcess 32k char
 	 limit.  We need only escape embedded double-quotes and immediately
 	 preceeding backslash characters.  A sequence of backslach characters
-	 that is not follwed by a double quote character will not be
+	 that is not followed by a double quote character will not be
 	 escaped.  */
       needs_quotes = 0;
       for (j = 0; argv[i][j]; j++)
@@ -366,7 +366,7 @@ argv_to_cmdline (char *const *argv)
 	      /* Escape preceeding backslashes.  */
 	      for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
 		cmdline_len++;
-	      /* Escape the qote character.  */
+	      /* Escape the quote character.  */
 	      cmdline_len++;
 	    }
 	}
-- 
2.30.2

Re: [PATCH] libiberty: On Windows pass a >32k cmdline through a response file.

[Jonathan Yong]

On 6/5/23 09:22, Costas Argyris wrote:
> Thanks, here is the follow up patch for a couple typos in the same file.
> 

Thanks, pushed as obvious.