Re: [PATCH] Implement open-process and related functions on MinGW

[Mark H Weaver <mhw@netris.org>]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Mark H Weaver <mhw@netris.org>
>> Cc: ludo@gnu.org,  guile-devel@gnu.org
>> Date: Fri, 28 Feb 2014 02:22:19 -0500
>> 
>> However, I don't want to use the "0xC0000200 + SIGNAL" convention
>> "between Guile and itself" that you've invented.  Instead, based on what
>> you wrote, I guess that we should make WIFSIGNALED(x) return true for
>> any status code of the form 0xC0000xxx, or maybe some larger set.  I
>> guess that WIFEXITED(x) should return the logical not of WIFSIGNALED(x).
>> WIFSTOPPED(x) presumably should always return false.
>> 
>> In the code you wrote:
>> 
>> > # define WIFEXITED(stat_val)   (((stat_val) & 0xC0000000) == 0)
>> > # define WIFSIGNALED(stat_val) (((stat_val) & 0xC0000000) != 0)
>> 
>> WIFSIGNALED(x) returns true if either of two high bits are 1.  This
>> seems a bit too loose to me.
>
> Strictly speaking, any of these two bits or both of them could be set,
[...]
> In general, a
> program launched by Guile could have a debugger attached, and then it
> could potentially get an exception like 0x40010005 (Ctrl-C was pressed
> into a debugged program).

Okay, in that case perhaps the definitions above are the best ones.
I'll leave it to your best judgment what definitions are most likely to
be "future proof", i.e. to correctly distinguish normal exits from
abnormal termination in future versions of Windows.

>> IMO, it would be reasonable to just return SIGTERM in all cases, like
>> Gnulib does, but perhaps we should map some known values to specific
>> signals.  You mentioned that the equivalent of SIGSEGV on Windows
>> results in a 0xC0000005 status code.  Any others worth mapping?
>
> Are you still talking about a program that crashed, or are you talking
> about a program that Guile forcibly killed?

I think we should try to consider both of these cases, and also a third
case: programs that are forcibly killed by something other than Guile.

> My reasoning for not producing SIGTERM
> unconditionally was that it might be confusing for a Guile program
> that requested termination via some signal other than SIGTERM to see
> that the program was terminated by SIGTERM.  I wanted WTERMSIG to
> return the same signal that was used to kill the program.

I suspect that this is relatively unimportant.  I suspect that it's more
important to do our best to follow the status code conventions used on
Windows, because:

* When Guile kills a process, the status code of the killed process
  should make sense to its parent, even if that parent is not Guile.

* When a Guile subprocess is killed by something other than Guile,
  The Guile program should be able to make sense of the status code.

> As for mapping these values to Posix signals: yes, this is possible.
> Here's the suggested mapping of values I consider useful:
>
>  0xC0000005 (access to invalid address)  SIGSEGV
>  0xC0000008 (invalid handle)             SIGSEGV
>  0xC000001D (illegal instruction)        SIGILL
>  0xC0000025 (non-continuable exception)  SIGILL
>  0xC000008C (array bounds exceeded)      SIGSEGV
>  0xC000008D (float denormal)             SIGFPE
>  0xC000008E (float divide by zero)       SIGFPE
>  0xC000008F (float inexact)              SIGFPE
>  0xC0000090 (float invalid operation)    SIGFPE
>  0xC0000091 (float overflow)             SIGFPE
>  0xC0000092 (float stack check)          SIGFPE
>  0xC0000093 (float underflow)            SIGFPE
>  0xC0000094 (integer divide by zero)     SIGFPE
>  0xC0000095 (integer overflow)           SIGFPE
>  0xC0000096 (privileged instruction)     SIGILL
>  0xC00000FD (stack overflow)             SIGSEGV
>  0xC000013A (Ctrl-C exit)                SIGINT

This looks good to me.  We'll also have to decide what WTERMSIG should
return by default, when the status code is none of the values above.

Also, what if the top bits are something other than 0xCxxx?  Should we
mask those off before looking up the code in the table above?

> You will see that there's no mapping for SIGTERM here.  If having that
> is important, we could map the last one to SIGTERM, since a program is
> frequently forcibly terminated on Windows by simulating a Ctrl-C
> keypress to it.

Yes, I think that's probably wise.

BTW, on the stable-2.0 branch in git, I recently imported the following
modules from Gnulib:

  link fsync readlink rename mkdir rmdir unistd

and removed the corresponding "#ifdef HAVE_xxx" around the associated
Scheme procedure definitions.  For details, see:

  http://git.savannah.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=3243fffcc19144fc0b30e983c3e50d1d1fc19ef4
  http://git.savannah.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=ca6adcc6df462f325dfa7b099295fd6212d02b43

I nearly imported the 'pipe-posix' Gnulib module as well, but I noticed
that they implemented it differently than you did.  You wrote:

> # define pipe(f)               _pipe(f, 0, O_NOINHERIT)

Whereas they do: _pipe(f, 4096, _O_BINARY).

Can you help me understand the pros and cons to these two approaches?
One thing: I'm fairly sure that we'll want _O_BINARY, and I think we
should also make sure that the pipe buffer is no less than 4096 bytes.

What do you think?

    Regards,
      Mark