pid_t on 64-bit Windows

pid_t on 64-bit Windows

[Bruno Haible]

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

On 64-bit Windows, pid_t needs to be 64-bit large, i.e. intptr_t or 'long long'
or '__int64'.

This is because the return type of _spawnv* (when invoked with _P_NOWAIT) and
the argument of the _cwait function are 'intptr_t' (see [1][2]: "The return
value from an asynchronous _spawnvp or _wspawnvp (_P_NOWAIT or _P_NOWAITO
specified for mode) is the process handle."

On mingw, this is already the case. But on MSVC (and MSVC/clang), the 'pid_t'
type is nowhere defined. In this case, gnulib defines it through config.h.

[1] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/spawnvp-wspawnvp
[2] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/cwait


2020-08-23  Bruno Haible  <bruno@clisp.org>

	tests: Don't assume that pid_t fits in an 'int'.
	* tests/test-nonblocking-pipe-main.c (main): Use type 'pid_t' instead
	of 'int'.
	* tests/test-nonblocking-socket-main.c (main): Likewise.

	sys_types: Fix definition of pid_t on 64-bit MSVC.
	* m4/pid_t.m4: New file.
	* modules/sys_types (Files): Add it.
	* modules/dirent (Files): Likewise.
	* modules/fcntl-h (Files): Likewise.
	* modules/sched (Files): Likewise.
	* modules/signal-h (Files): Likewise.
	* modules/spawn (Files): Likewise.
	* modules/sys_stat (Files): Likewise.
	* modules/sys_wait (Files): Likewise.
	* modules/termios (Files): Likewise.
	* modules/unistd (Files): Likewise.


[-- Attachment #2: 0001-sys_types-Fix-definition-of-pid_t-on-64-bit-MSVC.patch --]
[-- Type: text/x-patch, Size: 5783 bytes --]

From efe32f6a916b6addcfe60d2283cef4cebc6177ff Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Mon, 24 Aug 2020 01:19:18 +0200
Subject: [PATCH 1/2] sys_types: Fix definition of pid_t on 64-bit MSVC.

* m4/pid_t.m4: New file.
* modules/sys_types (Files): Add it.
* modules/dirent (Files): Likewise.
* modules/fcntl-h (Files): Likewise.
* modules/sched (Files): Likewise.
* modules/signal-h (Files): Likewise.
* modules/spawn (Files): Likewise.
* modules/sys_stat (Files): Likewise.
* modules/sys_wait (Files): Likewise.
* modules/termios (Files): Likewise.
* modules/unistd (Files): Likewise.
---
 ChangeLog         | 15 +++++++++++++++
 m4/pid_t.m4       | 33 +++++++++++++++++++++++++++++++++
 modules/dirent    |  1 +
 modules/fcntl-h   |  1 +
 modules/sched     |  1 +
 modules/signal-h  |  1 +
 modules/spawn     |  1 +
 modules/sys_stat  |  1 +
 modules/sys_types |  1 +
 modules/sys_wait  |  1 +
 modules/termios   |  1 +
 modules/unistd    |  1 +
 12 files changed, 58 insertions(+)
 create mode 100644 m4/pid_t.m4

diff --git a/ChangeLog b/ChangeLog
index e54c89f..9c2dcdc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
 2020-08-23  Bruno Haible  <bruno@clisp.org>
 
+	sys_types: Fix definition of pid_t on 64-bit MSVC.
+	* m4/pid_t.m4: New file.
+	* modules/sys_types (Files): Add it.
+	* modules/dirent (Files): Likewise.
+	* modules/fcntl-h (Files): Likewise.
+	* modules/sched (Files): Likewise.
+	* modules/signal-h (Files): Likewise.
+	* modules/spawn (Files): Likewise.
+	* modules/sys_stat (Files): Likewise.
+	* modules/sys_wait (Files): Likewise.
+	* modules/termios (Files): Likewise.
+	* modules/unistd (Files): Likewise.
+
+2020-08-23  Bruno Haible  <bruno@clisp.org>
+
 	inttypes: Fix {PRI,SCN}*PTR on 32-bit native Windows (regr. 2020-07-21).
 	* m4/inttypes.m4 (gl_INTTYPES_PRI_SCN): Fix syntax error in test
 	program.
diff --git a/m4/pid_t.m4 b/m4/pid_t.m4
new file mode 100644
index 0000000..321082d
--- /dev/null
+++ b/m4/pid_t.m4
@@ -0,0 +1,33 @@
+# pid_t.m4 serial 1
+dnl Copyright (C) 2020 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+dnl Define pid_t if the headers don't define it.
+AC_DEFUN([AC_TYPE_PID_T],
+[
+  AC_CHECK_TYPE([pid_t],
+    [],
+    [dnl On 64-bit native Windows, define it to the equivalent of 'intptr_t'
+     dnl (= 'long long' = '__int64'), because that is the return type
+     dnl of the _spawnv* functions
+     dnl <https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/spawnvp-wspawnvp>
+     dnl and the argument type of the _cwait function
+     dnl <https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/cwait>.
+     dnl Otherwise (on 32-bit Windows and on old Unix platforms), define it
+     dnl to 'int'.
+     AC_COMPILE_IFELSE(
+       [AC_LANG_PROGRAM([[
+          #if defined _WIN64 && !defined __CYGWIN__
+          LLP64
+          #endif
+          ]])
+       ],
+       [gl_pid_type='int'],
+       [gl_pid_type='__int64'])
+     AC_DEFINE_UNQUOTED([pid_t], [$gl_pid_type],
+       [Define as a signed integer type capable of holding a process identifier.])
+    ],
+    [AC_INCLUDES_DEFAULT])
+])
diff --git a/modules/dirent b/modules/dirent
index 6f615b4..08a6326 100644
--- a/modules/dirent
+++ b/modules/dirent
@@ -5,6 +5,7 @@ Files:
 lib/dirent.in.h
 m4/dirent_h.m4
 m4/unistd_h.m4
+m4/pid_t.m4
 
 Depends-on:
 include_next
diff --git a/modules/fcntl-h b/modules/fcntl-h
index ff74dec..2ef0106 100644
--- a/modules/fcntl-h
+++ b/modules/fcntl-h
@@ -5,6 +5,7 @@ Files:
 lib/fcntl.in.h
 m4/fcntl_h.m4
 m4/fcntl-o.m4
+m4/pid_t.m4
 
 Depends-on:
 extensions
diff --git a/modules/sched b/modules/sched
index 0b6bcae..572c57b 100644
--- a/modules/sched
+++ b/modules/sched
@@ -4,6 +4,7 @@ A <sched.h> include file.
 Files:
 lib/sched.in.h
 m4/sched_h.m4
+m4/pid_t.m4
 
 Depends-on:
 include_next
diff --git a/modules/signal-h b/modules/signal-h
index 810e2bf..2f38e9b 100644
--- a/modules/signal-h
+++ b/modules/signal-h
@@ -4,6 +4,7 @@ A GNU-like <signal.h>.
 Files:
 lib/signal.in.h
 m4/signal_h.m4
+m4/pid_t.m4
 
 Depends-on:
 include_next
diff --git a/modules/spawn b/modules/spawn
index 6e03294..f1b99ca 100644
--- a/modules/spawn
+++ b/modules/spawn
@@ -4,6 +4,7 @@ A POSIX compliant <spawn.h>.
 Files:
 lib/spawn.in.h
 m4/spawn_h.m4
+m4/pid_t.m4
 
 Depends-on:
 include_next
diff --git a/modules/sys_stat b/modules/sys_stat
index af276ab..42ae932 100644
--- a/modules/sys_stat
+++ b/modules/sys_stat
@@ -5,6 +5,7 @@ Files:
 lib/sys_stat.in.h
 m4/sys_stat_h.m4
 m4/unistd_h.m4
+m4/pid_t.m4
 
 Depends-on:
 include_next
diff --git a/modules/sys_types b/modules/sys_types
index 81a11aa..c00862c 100644
--- a/modules/sys_types
+++ b/modules/sys_types
@@ -5,6 +5,7 @@ Files:
 lib/sys_types.in.h
 m4/sys_types_h.m4
 m4/off_t.m4
+m4/pid_t.m4
 
 Depends-on:
 include_next
diff --git a/modules/sys_wait b/modules/sys_wait
index bedd7e1..73ce50b 100644
--- a/modules/sys_wait
+++ b/modules/sys_wait
@@ -4,6 +4,7 @@ A <sys/wait.h> for systems with missing declarations.
 Files:
 lib/sys_wait.in.h
 m4/sys_wait_h.m4
+m4/pid_t.m4
 
 Depends-on:
 include_next
diff --git a/modules/termios b/modules/termios
index 88056cc..8cb73e4 100644
--- a/modules/termios
+++ b/modules/termios
@@ -4,6 +4,7 @@ A <termios.h> that works around platform issues.
 Files:
 lib/termios.in.h
 m4/termios_h.m4
+m4/pid_t.m4
 
 Depends-on:
 include_next
diff --git a/modules/unistd b/modules/unistd
index b14faaa..18b91c4 100644
--- a/modules/unistd
+++ b/modules/unistd
@@ -6,6 +6,7 @@ m4/unistd_h.m4
 lib/unistd.c
 lib/unistd.in.h
 m4/off_t.m4
+m4/pid_t.m4
 
 Depends-on:
 extern-inline
-- 
2.7.4


[-- Attachment #3: 0002-tests-Don-t-assume-that-pid_t-fits-in-an-int.patch --]
[-- Type: text/x-patch, Size: 1772 bytes --]

From 83f2c832a760bd7e5d32e2d2ab8298f697cf3f70 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Mon, 24 Aug 2020 01:22:49 +0200
Subject: [PATCH 2/2] tests: Don't assume that pid_t fits in an 'int'.

* tests/test-nonblocking-pipe-main.c (main): Use type 'pid_t' instead
of 'int'.
* tests/test-nonblocking-socket-main.c (main): Likewise.
---
 ChangeLog                            | 5 +++++
 tests/test-nonblocking-pipe-main.c   | 2 +-
 tests/test-nonblocking-socket-main.c | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9c2dcdc..073f968 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2020-08-23  Bruno Haible  <bruno@clisp.org>
 
+	tests: Don't assume that pid_t fits in an 'int'.
+	* tests/test-nonblocking-pipe-main.c (main): Use type 'pid_t' instead
+	of 'int'.
+	* tests/test-nonblocking-socket-main.c (main): Likewise.
+
 	sys_types: Fix definition of pid_t on 64-bit MSVC.
 	* m4/pid_t.m4: New file.
 	* modules/sys_types (Files): Add it.
diff --git a/tests/test-nonblocking-pipe-main.c b/tests/test-nonblocking-pipe-main.c
index 0e132f0..ec19be5 100644
--- a/tests/test-nonblocking-pipe-main.c
+++ b/tests/test-nonblocking-pipe-main.c
@@ -44,7 +44,7 @@ main (int argc, char *argv[])
   const char *child_path;
   int test;
   int fd[2];
-  int child;
+  pid_t child;
   int exitcode;
 
   child_path = argv[1];
diff --git a/tests/test-nonblocking-socket-main.c b/tests/test-nonblocking-socket-main.c
index 500479d..d2cf50e 100644
--- a/tests/test-nonblocking-socket-main.c
+++ b/tests/test-nonblocking-socket-main.c
@@ -47,7 +47,7 @@ main (int argc, char *argv[])
   int test;
   int server;
   int port;
-  int child;
+  pid_t child;
   int server_socket;
   int exitcode;
 
-- 
2.7.4

Re: pid_t on 64-bit Windows

[Oberzalek Martin]

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

Hi,

Am Montag, den 24.08.2020, 01:30 +0200 schrieb Bruno Haible:


On 64-bit Windows, pid_t needs to be 64-bit large, i.e. intptr_t or 'long long'or
'__int64'.





This is because the return type of _spawnv* (when invoked with _P_NOWAIT) and
the argument of the _cwait function are 'intptr_t' (see [1][2]: "The return
value from an asynchronous _spawnvp or _wspawnvp (_P_NOWAIT or _P_NOWAITO
specified for mode) is the process handle."



_spawnvp(), or _wspawnvp() are not returning a pid. It is a process handle.

        intptr_t ret = _spawnvp( _P_NOWAIT, argv[2], args );
        DWORD pid = GetProcessId( (HANDLE)ret );
        printf( "ret: %d pid: %d\n", (int)ret, (int)pid );

GetProcessId() returns a DWORD as GetCurrentProcessId() does

[3] https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreads
api-getprocessid
[4] https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreads
api-getcurrentprocessid

So I guess pit_t should be defined as int, or DWORD, or int32_t



On mingw, this is already the case. But on MSVC (and MSVC/clang), the 'pid_t'
type is nowhere defined. In this case, gnulib defines it through config.h.





[1] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/spawnvp-wspawnvp
[2] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/cwait



Regards, Martin



--

Ing. Martin Oberzalek | Senior Service Technician WAMAS 3 | IT Business Operations

SSI SCHÄFER | SSI Schäfer IT Solutions GmbH | Friesachstraße 15 | 8114 Friesach bei Graz | Austria

Phone +43 3127 200-410 | Fax +43 3127 200-22

martin.oberzalek@ssi-schaefer.com<mailto:firstname.surname@ssi-schaefer.com>

Website<http://www.ssi-schaefer.com/> | Blog<http://www.ssi-schaefer.de/blog/> | YouTube<http://www.youtube.com/user/lagerlogistik1> | Facebook<https://www.facebook.com/SSI.SCHAEFER.DE>

SSI Schäfer IT Solutions GmbH | Friesachstrasse 15 | 8114 Friesach | Austria
Registered Office: Friesach | Commercial Register: 49324 K | VAT no. ATU28654300
Commercial Court: Landesgericht für Zivilrechtssachen Graz
Unsere Hinweise zum Umgang mit personenbezogenen Daten finden Sie hier<https://www.ssi-schaefer.com/de-at/datenschutz-49548>.
You can find our information on the handling of personal data here<https://www.ssi-schaefer.com/en-at/privacy-13258>.

[-- Attachment #2: Type: text/html, Size: 4274 bytes --]

Re: pid_t on 64-bit Windows

[Oberzalek Martin]

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

Hi,

Am Montag, den 24.08.2020, 01:30 +0200 schrieb Bruno Haible:


On 64-bit Windows, pid_t needs to be 64-bit large, i.e. intptr_t or 'long long'or
'__int64'.





This is because the return type of _spawnv* (when invoked with _P_NOWAIT) and
the argument of the _cwait function are 'intptr_t' (see [1][2]: "The return
value from an asynchronous _spawnvp or _wspawnvp (_P_NOWAIT or _P_NOWAITO
specified for mode) is the process handle."



_spawnvp(), or _wspawnvp() are not returning a pid. It is a process handle.

        intptr_t ret = _spawnvp( _P_NOWAIT, argv[2], args );
        DWORD pid = GetProcessId( (HANDLE)ret );
        printf( "ret: %d pid: %d\n", (int)ret, (int)pid );

GetProcessId() returns a DWORD as GetCurrentProcessId() does

[3] https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreads
api-getprocessid
[4] https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreads
api-getcurrentprocessid

So I guess pit_t should be defined as int, or DWORD, or int32_t



On mingw, this is already the case. But on MSVC (and MSVC/clang), the 'pid_t'
type is nowhere defined. In this case, gnulib defines it through config.h.





[1] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/spawnvp-wspawnvp
[2] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/cwait



Regards, Martin




SSI Schäfer IT Solutions GmbH | Friesachstrasse 15 | 8114 Friesach | Austria
Registered Office: Friesach | Commercial Register: 49324 K | VAT no. ATU28654300
Commercial Court: Landesgericht für Zivilrechtssachen Graz
Unsere Hinweise zum Umgang mit personenbezogenen Daten finden Sie hier<https://www.ssi-schaefer.com/de-at/datenschutz-49548>.
You can find our information on the handling of personal data here<https://www.ssi-schaefer.com/en-at/privacy-13258>.

[-- Attachment #2: Type: text/html, Size: 3137 bytes --]

Re: pid_t on 64-bit Windows

[Bruno Haible]

Hi Martin,

Your email is hardly readable, because
  1. it is a HTML email,
  2. when viewing it as plain text - which is the only secure way to read mail
     [1] - the cited text is not indented.
Please configure your mailer for plain-text mail [2], if you want to continue
to write to mailing lists. And indent or otherwise mark cited text.


> _spawnvp(), or _wspawnvp() are not returning a pid. It is a process handle.

No one claimed that _spawnvp() is returning a pid.

>         intptr_t ret = _spawnvp( _P_NOWAIT, argv[2], args );
>         DWORD pid = GetProcessId( (HANDLE)ret );
>         printf( "ret: %d pid: %d\n", (int)ret, (int)pid );

What does this program print?

And this program proves nothing, because you cast away the upper 32 bits
of 'ret' before printing.

Bruno

[1] https://www.scientificamerican.com/article/the-only-safe-e-mail-is-text-only-e-mail/
[2] https://useplaintext.email/

Re: pid_t on 64-bit Windows

[Martin Oberzalek]

Hi Bruno,

> Your email is hardly readable, because

I'm sorry for that. I hope it is fixed now.

> > _spawnvp(), or _wspawnvp() are not returning a pid. It is a process handle.
> 
> No one claimed that _spawnvp() is returning a pid.

What I wan't to point out is that in gnulib on WIN32 API pid_t is not a process id 
like it is on linux. Functions in gnulib that are using pid_t eg waitpid() accepting
a process handle instead.

I'm using parity[1] in an gentoo prefix environment to compile in linux like style
win32 as win64 applications. 

parity is a wrapper around the visual stdio compiler and it defines pid_t as int.
Because getpid() return also int. And GetCurrentProcessId() as well.

So if I compile a project that is using gnulib I might get a compiler error because
of the different definition of pid_t. (In best case. Depending on the situation, the
programm will simple crash)

For my use case a better solution would be a compile time test in the configure script
that tests the existance of pid_t.

Another solution would be not using pid_t in gnulib at all. Just redefining it in a way like

#if ( defined(_WIN64) || _defined(_WIN32) ) && !defined(__CYGWIN__)
intprt_t GNULIB_PID_HANDLE
#else
pid_t GNULIB_PID_HANDLE
#endif

What do you think?

Martin

[1] https://github.com/mduft/parity 

Re: pid_t on 64-bit Windows

[Bruno Haible]

Martin Oberzalek wrote:
> What I wan't to point out is that in gnulib on WIN32 API pid_t is not a process id 
> like it is on linux. Functions in gnulib that are using pid_t eg waitpid() accepting
> a process handle instead.

Correct.

When an OS offers process handles, instead of pids, we better should use that,
because it eliminates race conditions. (When a program uses waitpid() or kill() with
a pid argument, there is the risk that the intended process was already killed and
the pid was reused by another process. The probability that this happens is small,
which is why the problem is ignored in the Unix word. Nevertheless, a handle should
be more reliable.)

On Windows, the basis of waitpid() is '_cwait', which is essentially the same as
WaitForSingleObject. No race condition.

> I'm using parity[1] in an gentoo prefix environment to compile in linux like style
> win32 as win64 applications. 
> 
> parity is a wrapper around the visual stdio compiler

That's all ok...

> and it defines pid_t as int.

This isn't ok. As explained above, the more reliable implementation of waitpid
takes a 64-bit HANDLE (on _WIN64) as argument, not a 32-bit int.

> Because getpid() return also int. And GetCurrentProcessId() as well.

This is a red herring, because no one will call waitpid() to wait for the
current process. waitpid() is always used to wait for a different process,
typically even subprocesses.

mingw defines pid_t as 64-bit on _WIN64. Mingw make-alikes like parity should do
the same.

Bruno