[PATCH] Prevent restart of crashing non-Cygwin exe

[PATCH] Prevent restart of crashing non-Cygwin exe

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

If a non-Cygwin .exe started from a Cygwin shell window segfaults, 
Cygwin restarts the .exe 5 times.

Testcase:

$ cat crash.c
#include <stdio.h>

int main()
{
   printf("Hello, "); fflush(stdout);
   *(char *)0 = 42;
   printf("World\n");
   return 0;
}

$ gcc -o crash-c crash.c

$ ./crash-c
Hello, Segmentation fault (core dumped)

$ i686-w64-mingw32-gcc -o crash-w crash.c

$ ./crash-w
Hello, Hello, Hello, Hello, Hello, Hello,

(The repeated outputs are not be visible on 1.7.9-1 when shell runs in a 
Windows console without CYGWIN=tty)

The problem is that Cygwin retries CreateProcess() if process aborts 
with an unknown 0xc0000XXXX exit code also for non-Cygwin programs. The 
attached patch fixes this.

Christian


[-- Attachment #2: spawn-no-retry.patch --]
[-- Type: text/x-diff, Size: 1234 bytes --]

2011-06-23  Christian Franke  <franke@computer.org>

	* sigproc.cc (child_info::sync): Add exit_code to debug
	message.
	(child_info::proc_retry): Don't retry on unknown exit_code
	from non-cygwin programs.

diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 2f42db2..1e57876 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -883,7 +883,8 @@ child_info::sync (pid_t pid, HANDLE& hProcess, DWORD howlong)
 	      hProcess = NULL;
 	    }
 	}
-      sigproc_printf ("pid %u, WFMO returned %d, res %d", pid, x, res);
+      sigproc_printf ("pid %u, WFMO returned %d, exit_code 0x%x, res %d",
+		      pid, x, exit_code, res);
     }
   return res;
 }
@@ -915,11 +916,11 @@ child_info::proc_retry (HANDLE h)
     case EXITCODE_FORK_FAILED: /* windows prevented us from forking */
       break;
 
-    /* Count down non-recognized exit codes more quickly since they aren't
-       due to known conditions.  */
     default:
-      if (!iscygwin () && (exit_code & 0xffff0000) != 0xc0000000)
+      if (!iscygwin ())
 	break;
+      /* Count down non-recognized exit codes more quickly since they aren't
+         due to known conditions.  */
       if ((retry -= 2) < 0)
 	retry = 0;
       else

Re: [PATCH] Prevent restart of crashing non-Cygwin exe

[Corinna Vinschen]

Hi Christian,

On Jun 23 19:52, Christian Franke wrote:
> If a non-Cygwin .exe started from a Cygwin shell window segfaults,
> Cygwin restarts the .exe 5 times.
> [...l]
> 	* sigproc.cc (child_info::sync): Add exit_code to debug
> 	message.
> 	(child_info::proc_retry): Don't retry on unknown exit_code
> 	from non-cygwin programs.

This looks ok to me, but cgf should have a say here.  He's on vacation
for another week, though.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Prevent restart of crashing non-Cygwin exe

[Christian Franke]

On Jun 24, Corinna Vinschen wrote:
> Hi Christian,
>
> On Jun 23 19:52, Christian Franke wrote:
>> If a non-Cygwin .exe started from a Cygwin shell window segfaults,
>> Cygwin restarts the .exe 5 times.
>> [...l]
>> 	* sigproc.cc (child_info::sync): Add exit_code to debug
>> 	message.
>> 	(child_info::proc_retry): Don't retry on unknown exit_code
>> 	from non-cygwin programs.
> This looks ok to me, but cgf should have a say here.  He's on vacation
> for another week, though.
>

Problem can still be reproduced with current CVS. Patch is still valid.

Christian

Re: [PATCH] Prevent restart of crashing non-Cygwin exe

[Corinna Vinschen]

On Nov  2 20:53, Christian Franke wrote:
> On Jun 24, Corinna Vinschen wrote:
> >Hi Christian,
> >
> >On Jun 23 19:52, Christian Franke wrote:
> >>If a non-Cygwin .exe started from a Cygwin shell window segfaults,
> >>Cygwin restarts the .exe 5 times.
> >>[...l]
> >>	* sigproc.cc (child_info::sync): Add exit_code to debug
> >>	message.
> >>	(child_info::proc_retry): Don't retry on unknown exit_code
> >>	from non-cygwin programs.
> >This looks ok to me, but cgf should have a say here.  He's on vacation
> >for another week, though.
> >
> 
> Problem can still be reproduced with current CVS. Patch is still valid.

Sorry, I forgot about this patch entirely.  Chris, is that patch ok with
you as well?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Prevent restart of crashing non-Cygwin exe

[Christopher Faylor]

On Thu, Nov 03, 2011 at 01:07:20PM +0100, Corinna Vinschen wrote:
>On Nov  2 20:53, Christian Franke wrote:
>> On Jun 24, Corinna Vinschen wrote:
>> >Hi Christian,
>> >
>> >On Jun 23 19:52, Christian Franke wrote:
>> >>If a non-Cygwin .exe started from a Cygwin shell window segfaults,
>> >>Cygwin restarts the .exe 5 times.
>> >>[...l]
>> >>	* sigproc.cc (child_info::sync): Add exit_code to debug
>> >>	message.
>> >>	(child_info::proc_retry): Don't retry on unknown exit_code
>> >>	from non-cygwin programs.
>> >This looks ok to me, but cgf should have a say here.  He's on vacation
>> >for another week, though.
>> >
>> 
>>Problem can still be reproduced with current CVS.  Patch is still
>>valid.
>
>Sorry, I forgot about this patch entirely.  Chris, is that patch ok
>with you as well?

No, it isn't.  Sorry for not stating this earlier.  The problem that
this code was intended to solve was actually a transient exit codes from
a non-Cygwin process which began with 0xc...

I don't believe that I ever saw STATUS_ACCESS_VIOLATION in any of my
testing though so adding that earlier in the switch would fix this
particular problem.  I'll do that.

cgf

Re: [PATCH] Prevent restart of crashing non-Cygwin exe

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

Christopher Faylor wrote:
> On Thu, Nov 03, 2011 at 01:07:20PM +0100, Corinna Vinschen wrote:
>> On Nov  2 20:53, Christian Franke wrote:
>>> On Jun 24, Corinna Vinschen wrote:
>>>> Hi Christian,
>>>>
>>>> On Jun 23 19:52, Christian Franke wrote:
>>>>> If a non-Cygwin .exe started from a Cygwin shell window segfaults,
>>>>> Cygwin restarts the .exe 5 times.
>>>>> [...l]
>>>>> 	* sigproc.cc (child_info::sync): Add exit_code to debug
>>>>> 	message.
>>>>> 	(child_info::proc_retry): Don't retry on unknown exit_code
>>>>> 	from non-cygwin programs.
>>>> This looks ok to me, but cgf should have a say here.  He's on vacation
>>>> for another week, though.
>>>>
>>> Problem can still be reproduced with current CVS.  Patch is still
>>> valid.
>> Sorry, I forgot about this patch entirely.  Chris, is that patch ok
>> with you as well?
> No, it isn't.  Sorry for not stating this earlier.  The problem that
> this code was intended to solve was actually a transient exit codes from
> a non-Cygwin process which began with 0xc...
>
> I don't believe that I ever saw STATUS_ACCESS_VIOLATION in any of my
> testing though so adding that earlier in the switch would fix this
> particular problem.  I'll do that.

Works as expected with testcase from my first mail:

$ i686-w64-mingw32-gcc -o crash-w crash.c

$ ./crash-w
Hello,


A drawback is that non-Cygwin programs crash silently.
I attached an experimental patch which sets WTERMSIG(status) = SIGSEGV:

$ ./crash-w
Hello, Segmentation fault

Christian


[-- Attachment #2: fake-sigsegv.patch --]
[-- Type: text/x-patch, Size: 591 bytes --]

diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 5a77d8f..7d60d62 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -148,9 +148,13 @@ status_exit (DWORD x)
 void
 pinfo::set_exit_code (DWORD x)
 {
-  if (x >= 0xc0000000UL)
+  int sig = sigExeced;
+  if (x == STATUS_ACCESS_VIOLATION && !sig)
+    /* Report segfault to parent process.  */
+    sig = SIGSEGV;
+  else if (x >= 0xc0000000UL)
     x = status_exit (x);
-  self->exitcode = EXITCODE_SET | (sigExeced ?: (x & 0xff) << 8);
+  self->exitcode = EXITCODE_SET | (sig ?: (x & 0xff) << 8);
 }
 
 void

Re: [PATCH] Prevent restart of crashing non-Cygwin exe

[Christopher Faylor]

On Sat, Nov 05, 2011 at 06:00:25PM +0100, Christian Franke wrote:
>Christopher Faylor wrote:
>> On Thu, Nov 03, 2011 at 01:07:20PM +0100, Corinna Vinschen wrote:
>>> On Nov  2 20:53, Christian Franke wrote:
>>>> On Jun 24, Corinna Vinschen wrote:
>>>>> Hi Christian,
>>>>>
>>>>> On Jun 23 19:52, Christian Franke wrote:
>>>>>> If a non-Cygwin .exe started from a Cygwin shell window segfaults,
>>>>>> Cygwin restarts the .exe 5 times.
>>>>>> [...l]
>>>>>> 	* sigproc.cc (child_info::sync): Add exit_code to debug
>>>>>> 	message.
>>>>>> 	(child_info::proc_retry): Don't retry on unknown exit_code
>>>>>> 	from non-cygwin programs.
>>>>> This looks ok to me, but cgf should have a say here.  He's on vacation
>>>>> for another week, though.
>>>>>
>>>> Problem can still be reproduced with current CVS.  Patch is still
>>>> valid.
>>> Sorry, I forgot about this patch entirely.  Chris, is that patch ok
>>> with you as well?
>> No, it isn't.  Sorry for not stating this earlier.  The problem that
>> this code was intended to solve was actually a transient exit codes from
>> a non-Cygwin process which began with 0xc...
>>
>> I don't believe that I ever saw STATUS_ACCESS_VIOLATION in any of my
>> testing though so adding that earlier in the switch would fix this
>> particular problem.  I'll do that.
>
>Works as expected with testcase from my first mail:
>
>$ i686-w64-mingw32-gcc -o crash-w crash.c
>
>$ ./crash-w
>Hello,
>
>
>A drawback is that non-Cygwin programs crash silently.

Thanks for the patch but I chose to do this in a different way.  I've
checked in a change which should accomplish the task of having bash
report the correct error message while allowing easy future extension.

cgf