Re: [gdbserver] Automatically add an ".exe" suffix on failure.

[Pedro Alves <pedro_alves at portugalmail dot pt>]

Pedro Alves wrote:
> Hi,
>
> I was running the testsuite against a local gdbserver on Cygwin, and 
> noticed
> that gdbserver as opposed to a native gdb, doesn't append the ".exe" 
> suffix to
> the executable name and retry if creating the inferior process failed.  
> When
> building natively, an .exe suffix is appended to the executable filename if
> there isn't one already.  Most the testcases don't specify it.  This
> results in gdbserver trying to look for an executable called break for 
> instance,
> while there is only a break.exe.  This isn't noticeable when cross testing,
> since (I think) that on that case the suffix isn't appended automatically.
>
> (Gdb does the retrying in exec.c:exec_file_attach.)
>
> Tested on a local gdbserver which doesn't look that bad compared to a 
> native
> test run.  Unfortunately the test results aren't 100% consistent across 
> runs,
> on both native and remote, to have a good comparison...
>
> I had to hack win32-nat.c to be able to test against a local gdbserver 
> on native
> Cygwin, because currently, a Cygwin hosted gdb will isn't able to use 
> the new
> gdbserver dll support.  This happens because a Cygwin hosted gdb will 
> always use
> the current_target_so_ops set in win32-nat.c:init_win32_ops, even when 
> remote
> debugging.
>
> The patch to use solib-target.c on native Cygwin would fix this too.  
> Daniel,
> I'm going to work on rewriting it to not be so aggressive fetching the 
> list of
> sos from the target, by keeping a list of dlls on win32-nat.c like it is
> currently.  The patch should turn up much smaller and safer.  Maybe it 
> can then
> go into 6.7 (?),  but I wouldn't mind if it didn't either.


Here is an updated patch.
We should only retry with ".exe" if the CreateProcess call fails because the
file doesn't exist.  I then noticed that the setenv call (Cygwin only) between
the create_process call and the error call always sets the last error as
ERROR_FILE_NOT_FOUND, thus masking the real error (eg: ERROR_BAD_EXE_FORMAT)...

    if (!ret && err == ERROR_FILE_NOT_FOUND)
      {
        char *exename = alloca (strlen (program) + 5);
        strcat (strcpy (exename, program), ".exe");
        ret = create_process (exename, args, flags, &pi);
        /* GetLastError() here returns lets say, 193 (ERROR_BAD_EXE_FORMAT)... */
      }

    if (orig_path)
      {
        setenv ("PATH", orig_path, 1);

        /* ... and here GetLastError() here always returns 2
	(ERROR_FILE_NOT_FOUND). */
      }

    if (!ret)
      {
       /* which means we now print a confusing error message. */
        DWORD err = GetLastError ();
        error ("Error creating process \"%s%s\", (error %d): %s\n",
	     program, args, (int) err, strwinerror (err));
      }

Cheers,
Pedro Alves


2007-08-12  Pedro Alves  <pedro_alves@portugalmail.pt>

	* win32-low.c (create_process): New.
	(win32_create_inferior): Use create_process instead of
	CreateProcess.  If create_process failed retry appending an ".exe"
	suffix.  Store the GetLastError result immediatelly after
	create_process calls and use it on the call to error.

---
 gdb/gdbserver/win32-low.c |   94 ++++++++++++++++++++++++++++------------------
 1 file changed, 59 insertions(+), 35 deletions(-)

Index: src/gdb/gdbserver/win32-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-low.c	2007-08-11 18:37:10.000000000 +0100
+++ src/gdb/gdbserver/win32-low.c	2007-08-12 17:10:46.000000000 +0100
@@ -375,6 +375,55 @@ strwinerror (DWORD error)
   return buf;
 }
 
+static BOOL
+create_process (const char *program, char *args,
+		DWORD flags, PROCESS_INFORMATION *pi)
+{
+  BOOL ret;
+
+#ifdef _WIN32_WCE
+  wchar_t *p, *wprogram, *wargs;
+  size_t argslen;
+
+  wprogram = alloca ((strlen (program) + 1) * sizeof (wchar_t));
+  mbstowcs (wprogram, program, strlen (program) + 1);
+
+  for (p = wprogram; *p; ++p)
+    if (L'/' == *p)
+      *p = L'\\';
+
+  argslen = strlen (args);
+  wargs = alloca ((argslen + 1) * sizeof (wchar_t));
+  mbstowcs (wargs, args, argslen + 1);
+
+  ret = CreateProcessW (wprogram, /* image name */
+                        wargs,    /* command line */
+                        NULL,     /* security, not supported */
+                        NULL,     /* thread, not supported */
+                        FALSE,    /* inherit handles, not supported */
+                        flags,    /* start flags */
+                        NULL,     /* environment, not supported */
+                        NULL,     /* current directory, not supported */
+                        NULL,     /* start info, not supported */
+                        pi);      /* proc info */
+#else
+  STARTUPINFOA si = { sizeof (STARTUPINFOA) };
+
+  ret = CreateProcessA (program,  /* image name */
+			args,     /* command line */
+			NULL,     /* security */
+			NULL,     /* thread */
+			TRUE,     /* inherit handles */
+			flags,    /* start flags */
+			NULL,     /* environment */
+			NULL,     /* current directory */
+			&si,      /* start info */
+			pi);      /* proc info */
+#endif
+
+  return ret;
+}
+
 /* Start a new process.
    PROGRAM is a path to the program to execute.
    ARGS is a standard NULL-terminated array of arguments,
@@ -394,12 +443,7 @@ win32_create_inferior (char *program, ch
   int argslen;
   int argc;
   PROCESS_INFORMATION pi;
-#ifndef __MINGW32CE__
-  STARTUPINFOA si = { sizeof (STARTUPINFOA) };
-  char *winenv = NULL;
-#else
-  wchar_t *wargs, *wprogram;
-#endif
+  DWORD err;
 
   if (!program)
     error ("No executable specified, specify executable to debug.\n");
@@ -439,34 +483,15 @@ win32_create_inferior (char *program, ch
   flags |= CREATE_NEW_PROCESS_GROUP;
 #endif
 
-#ifdef __MINGW32CE__
-  to_back_slashes (program);
-  wargs = alloca (argslen * sizeof (wchar_t));
-  mbstowcs (wargs, args, argslen);
-  wprogram = alloca ((strlen (program) + 1) * sizeof (wchar_t));
-  mbstowcs (wprogram, program, strlen (program) + 1);
-  ret = CreateProcessW (wprogram, /* image name */
-                        wargs,    /* command line */
-                        NULL,     /* security, not supported */
-                        NULL,     /* thread, not supported */
-                        FALSE,    /* inherit handles, not supported */
-                        flags,    /* start flags */
-                        NULL,     /* environment, not supported */
-                        NULL,     /* current directory, not supported */
-                        NULL,     /* start info, not supported */
-                        &pi);     /* proc info */
-#else
-  ret = CreateProcessA (program,  /* image name */
-			args,     /* command line */
-			NULL,     /* security */
-			NULL,     /* thread */
-			TRUE,     /* inherit handles */
-			flags,    /* start flags */
-			winenv,   /* environment */
-			NULL,     /* current directory */
-			&si,      /* start info */
-			&pi);     /* proc info */
-#endif
+  ret = create_process (program, args, flags, &pi);
+  err = GetLastError ();
+  if (!ret && err == ERROR_FILE_NOT_FOUND)
+    {
+      char *exename = alloca (strlen (program) + 5);
+      strcat (strcpy (exename, program), ".exe");
+      ret = create_process (exename, args, flags, &pi);
+      err = GetLastError ();
+    }
 
 #ifndef USE_WIN32API
   if (orig_path)
@@ -475,7 +500,6 @@ win32_create_inferior (char *program, ch
 
   if (!ret)
     {
-      DWORD err = GetLastError ();
       error ("Error creating process \"%s%s\", (error %d): %s\n",
 	     program, args, (int) err, strwinerror (err));
     }