[gdb/build] Fix build breaker on mingw-w64
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
Simon reported a build breaker on mingw-w64:
...
In file included from gdb/cli/cli-cmds.c:58:
gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
gdbsupport/eintr.h:77:35: error: ‘::waitpid’ has not been declared; \
did you mean ‘gdb::waitpid’?
77 | return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
| ^~~~~~~
| gdb::waitpid
gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
75 | waitpid (pid_t pid, int *wstatus, int options)
| ^~~~~~~
...
This is a regression since commit 658a03e9e85 ("[gdbsupport] Add
gdb::{waitpid,read,write,close}"), where I moved the use of ::waitpid from
run_under_shell, where it was used conditionally:
...
#if defined(CANT_FORK) || \
(!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
...
#else
int ret = gdb::handle_eintr (-1, ::waitpid, pid, &status, 0);
...
to gdb::waitpid, where it's used unconditionally:
...
inline pid_t
waitpid (pid_t pid, int *wstatus, int options)
{
return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
}
...
Likewise for ::wait.
So, with what condition or conditions could we guard these uses?
According to gnulib [1][2], ::wait and ::waitpid are missing on platforms
mingw and MSVC 14. Translating this directly would have us using __MINGW32__,
__MINGW64__ and _MSC_VER, but it seems easier to use WIN32API.
Another possiblity would be to use HAVE_WAITPID and HAVE_WAIT, but:
- HAVE_WAIT is missing and would need adding, and
- HAVE_WAITPID is defined only in gdb and would have to be moved to gdbsupport
or duplicated to gdbserver.
I'm choosing the WIN32API one here, since it's the least amount of work. This
solution would stop working if we'd add waitpid to the gnulib imports, but for
now I think this is good enough.
Reproduced and tested by doing a mingw-w64 cross-build on x86_64-linux. Many
thanks to Simon for helping me set that up.
Reported-By: Simon Marchi <simark@simark.ca>
[1] https://www.gnu.org/software/gnulib/manual/html_node/waitpid.html
[2] https://www.gnu.org/software/gnulib/manual/html_node/wait.html
---
gdbsupport/eintr.h | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
base-commit: 57c526470bbf03220f928f0894a85c76a9659d35
Comments
On 2024-12-02 06:58, Tom de Vries wrote:
> Simon reported a build breaker on mingw-w64:
> ...
> In file included from gdb/cli/cli-cmds.c:58:
> gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
> gdbsupport/eintr.h:77:35: error: ‘::waitpid’ has not been declared; \
> did you mean ‘gdb::waitpid’?
> 77 | return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
> | ^~~~~~~
> | gdb::waitpid
> gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
> 75 | waitpid (pid_t pid, int *wstatus, int options)
> | ^~~~~~~
> ...
>
> This is a regression since commit 658a03e9e85 ("[gdbsupport] Add
> gdb::{waitpid,read,write,close}"), where I moved the use of ::waitpid from
> run_under_shell, where it was used conditionally:
> ...
> #if defined(CANT_FORK) || \
> (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
> ...
> #else
> int ret = gdb::handle_eintr (-1, ::waitpid, pid, &status, 0);
> ...
> to gdb::waitpid, where it's used unconditionally:
> ...
> inline pid_t
> waitpid (pid_t pid, int *wstatus, int options)
> {
> return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
> }
> ...
> Likewise for ::wait.
>
> So, with what condition or conditions could we guard these uses?
>
> According to gnulib [1][2], ::wait and ::waitpid are missing on platforms
> mingw and MSVC 14. Translating this directly would have us using __MINGW32__,
> __MINGW64__ and _MSC_VER, but it seems easier to use WIN32API.
>
> Another possiblity would be to use HAVE_WAITPID and HAVE_WAIT, but:
> - HAVE_WAIT is missing and would need adding, and
> - HAVE_WAITPID is defined only in gdb and would have to be moved to gdbsupport
> or duplicated to gdbserver.
>
> I'm choosing the WIN32API one here, since it's the least amount of work. This
> solution would stop working if we'd add waitpid to the gnulib imports, but for
> now I think this is good enough.
I would prefer the configure check. I think it's preferrable to probe
for the feature, when possible, rather than hardcode the
condition for when it's available or not available. I just gave it a
try and it's not too bad:
From 1bb04cca8e1f6985d47c7df68b3f8a56a1dc12a6 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Mon, 2 Dec 2024 11:42:56 -0500
Subject: [PATCH] fix
Change-Id: I0841aed01cb41329c4f4a0ee45d2232ea0157459
---
gdb/config.in | 3 +++
gdb/configure | 3 ++-
gdb/configure.ac | 1 -
gdbserver/config.in | 6 ++++++
gdbserver/configure | 2 ++
gdbsupport/common.m4 | 2 ++
gdbsupport/config.in | 6 ++++++
gdbsupport/configure | 2 ++
gdbsupport/eintr.h | 4 ++++
9 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/gdb/config.in b/gdb/config.in
index 187b42ca29b7..1b517f035f73 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -586,6 +586,9 @@
/* Define to 1 if you have the <vfork.h> header file. */
#undef HAVE_VFORK_H
+/* Define to 1 if you have the `wait' function. */
+#undef HAVE_WAIT
+
/* Define to 1 if you have the `waitpid' function. */
#undef HAVE_WAITPID
diff --git a/gdb/configure b/gdb/configure
index 138c2a153b7b..7fd52844d7eb 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -20065,6 +20065,8 @@ fi
sigprocmask \
sigtimedwait \
socketpair \
+ wait \
+ waitpid \
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
@@ -29924,7 +29926,6 @@ for ac_func in \
sigsetmask \
ttrace \
use_default_colors \
- waitpid \
wresize \
do :
diff --git a/gdb/configure.ac b/gdb/configure.ac
index d638b8f3c8e3..230c0be79c7c 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1389,7 +1389,6 @@ AC_CHECK_FUNCS([ \
sigsetmask \
ttrace \
use_default_colors \
- waitpid \
wresize \
])
diff --git a/gdbserver/config.in b/gdbserver/config.in
index 65f9ff6e6470..53aecf4afc4c 100644
--- a/gdbserver/config.in
+++ b/gdbserver/config.in
@@ -393,6 +393,12 @@
/* Define to 1 if you have the <vfork.h> header file. */
#undef HAVE_VFORK_H
+/* Define to 1 if you have the `wait' function. */
+#undef HAVE_WAIT
+
+/* Define to 1 if you have the `waitpid' function. */
+#undef HAVE_WAITPID
+
/* Define to 1 if you have the <wait.h> header file. */
#undef HAVE_WAIT_H
diff --git a/gdbserver/configure b/gdbserver/configure
index 32980e5017ed..48cbaccc4eb4 100755
--- a/gdbserver/configure
+++ b/gdbserver/configure
@@ -8772,6 +8772,8 @@ fi
sigprocmask \
sigtimedwait \
socketpair \
+ wait \
+ waitpid \
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
diff --git a/gdbsupport/common.m4 b/gdbsupport/common.m4
index 54290cc85e78..4d4060322bfb 100644
--- a/gdbsupport/common.m4
+++ b/gdbsupport/common.m4
@@ -82,6 +82,8 @@ AC_CHECK_HEADERS([ \
sigprocmask \
sigtimedwait \
socketpair \
+ wait \
+ waitpid \
])
# This is needed for RHEL 5 and uclibc-ng < 1.0.39.
diff --git a/gdbsupport/config.in b/gdbsupport/config.in
index 8467072752a6..0beacf22c057 100644
--- a/gdbsupport/config.in
+++ b/gdbsupport/config.in
@@ -319,6 +319,12 @@
/* Define to 1 if you have the <vfork.h> header file. */
#undef HAVE_VFORK_H
+/* Define to 1 if you have the `wait' function. */
+#undef HAVE_WAIT
+
+/* Define to 1 if you have the `waitpid' function. */
+#undef HAVE_WAITPID
+
/* Define to 1 if you have the <wait.h> header file. */
#undef HAVE_WAIT_H
diff --git a/gdbsupport/configure b/gdbsupport/configure
index 87980f6870f3..0d994933b7d9 100755
--- a/gdbsupport/configure
+++ b/gdbsupport/configure
@@ -11546,6 +11546,8 @@ fi
sigprocmask \
sigtimedwait \
socketpair \
+ wait \
+ waitpid \
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
index 3980e3f5ac1c..4cab8f9d48de 100644
--- a/gdbsupport/eintr.h
+++ b/gdbsupport/eintr.h
@@ -71,11 +71,13 @@ handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
return ret;
}
+#ifdef HAVE_WAITPID
inline pid_t
waitpid (pid_t pid, int *wstatus, int options)
{
return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
}
+#endif /* HAVE_WAITPID */
inline int
open (const char *pathname, int flags)
@@ -83,11 +85,13 @@ open (const char *pathname, int flags)
return gdb::handle_eintr (-1, ::open, pathname, flags);
}
+#ifdef HAVE_WAIT
inline pid_t
wait (int *wstatus)
{
return gdb::handle_eintr (-1, ::wait, wstatus);
}
+#endif /* HAVE_WAIT */
inline int
close (int fd)
base-commit: 3eccfdce99e030be759ea20c66d6f78978b11a25
On 12/2/24 18:48, Simon Marchi wrote:
>
>
> On 2024-12-02 06:58, Tom de Vries wrote:
>> Simon reported a build breaker on mingw-w64:
>> ...
>> In file included from gdb/cli/cli-cmds.c:58:
>> gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
>> gdbsupport/eintr.h:77:35: error: ‘::waitpid’ has not been declared; \
>> did you mean ‘gdb::waitpid’?
>> 77 | return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
>> | ^~~~~~~
>> | gdb::waitpid
>> gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
>> 75 | waitpid (pid_t pid, int *wstatus, int options)
>> | ^~~~~~~
>> ...
>>
>> This is a regression since commit 658a03e9e85 ("[gdbsupport] Add
>> gdb::{waitpid,read,write,close}"), where I moved the use of ::waitpid from
>> run_under_shell, where it was used conditionally:
>> ...
>> #if defined(CANT_FORK) || \
>> (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
>> ...
>> #else
>> int ret = gdb::handle_eintr (-1, ::waitpid, pid, &status, 0);
>> ...
>> to gdb::waitpid, where it's used unconditionally:
>> ...
>> inline pid_t
>> waitpid (pid_t pid, int *wstatus, int options)
>> {
>> return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
>> }
>> ...
>> Likewise for ::wait.
>>
>> So, with what condition or conditions could we guard these uses?
>>
>> According to gnulib [1][2], ::wait and ::waitpid are missing on platforms
>> mingw and MSVC 14. Translating this directly would have us using __MINGW32__,
>> __MINGW64__ and _MSC_VER, but it seems easier to use WIN32API.
>>
>> Another possiblity would be to use HAVE_WAITPID and HAVE_WAIT, but:
>> - HAVE_WAIT is missing and would need adding, and
>> - HAVE_WAITPID is defined only in gdb and would have to be moved to gdbsupport
>> or duplicated to gdbserver.
>>
>> I'm choosing the WIN32API one here, since it's the least amount of work. This
>> solution would stop working if we'd add waitpid to the gnulib imports, but for
>> now I think this is good enough.
>
> I would prefer the configure check. I think it's preferrable to probe
> for the feature, when possible, rather than hardcode the
> condition for when it's available or not available. I just gave it a
> try and it's not too bad:
>
>
Hi Simon,
thanks for helping out, LGTM.
FWIW, I'm a bit surprised that it's necessary to add the configure
checks in gdbsupport, gdb and gdbserver, I would have expected that
adding it in gdbsupport would have been enough, but I guess things work
differently.
Submitted your patch as a v2 (
https://sourceware.org/pipermail/gdb-patches/2024-December/213725.html
). [ But of course feel free to ignore that and submit it yourself. ]
Thanks,
- Tom
> From 1bb04cca8e1f6985d47c7df68b3f8a56a1dc12a6 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Mon, 2 Dec 2024 11:42:56 -0500
> Subject: [PATCH] fix
>
> Change-Id: I0841aed01cb41329c4f4a0ee45d2232ea0157459
> ---
> gdb/config.in | 3 +++
> gdb/configure | 3 ++-
> gdb/configure.ac | 1 -
> gdbserver/config.in | 6 ++++++
> gdbserver/configure | 2 ++
> gdbsupport/common.m4 | 2 ++
> gdbsupport/config.in | 6 ++++++
> gdbsupport/configure | 2 ++
> gdbsupport/eintr.h | 4 ++++
> 9 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/config.in b/gdb/config.in
> index 187b42ca29b7..1b517f035f73 100644
> --- a/gdb/config.in
> +++ b/gdb/config.in
> @@ -586,6 +586,9 @@
> /* Define to 1 if you have the <vfork.h> header file. */
> #undef HAVE_VFORK_H
>
> +/* Define to 1 if you have the `wait' function. */
> +#undef HAVE_WAIT
> +
> /* Define to 1 if you have the `waitpid' function. */
> #undef HAVE_WAITPID
>
> diff --git a/gdb/configure b/gdb/configure
> index 138c2a153b7b..7fd52844d7eb 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -20065,6 +20065,8 @@ fi
> sigprocmask \
> sigtimedwait \
> socketpair \
> + wait \
> + waitpid \
>
> do :
> as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
> @@ -29924,7 +29926,6 @@ for ac_func in \
> sigsetmask \
> ttrace \
> use_default_colors \
> - waitpid \
> wresize \
>
> do :
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index d638b8f3c8e3..230c0be79c7c 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -1389,7 +1389,6 @@ AC_CHECK_FUNCS([ \
> sigsetmask \
> ttrace \
> use_default_colors \
> - waitpid \
> wresize \
> ])
>
> diff --git a/gdbserver/config.in b/gdbserver/config.in
> index 65f9ff6e6470..53aecf4afc4c 100644
> --- a/gdbserver/config.in
> +++ b/gdbserver/config.in
> @@ -393,6 +393,12 @@
> /* Define to 1 if you have the <vfork.h> header file. */
> #undef HAVE_VFORK_H
>
> +/* Define to 1 if you have the `wait' function. */
> +#undef HAVE_WAIT
> +
> +/* Define to 1 if you have the `waitpid' function. */
> +#undef HAVE_WAITPID
> +
> /* Define to 1 if you have the <wait.h> header file. */
> #undef HAVE_WAIT_H
>
> diff --git a/gdbserver/configure b/gdbserver/configure
> index 32980e5017ed..48cbaccc4eb4 100755
> --- a/gdbserver/configure
> +++ b/gdbserver/configure
> @@ -8772,6 +8772,8 @@ fi
> sigprocmask \
> sigtimedwait \
> socketpair \
> + wait \
> + waitpid \
>
> do :
> as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
> diff --git a/gdbsupport/common.m4 b/gdbsupport/common.m4
> index 54290cc85e78..4d4060322bfb 100644
> --- a/gdbsupport/common.m4
> +++ b/gdbsupport/common.m4
> @@ -82,6 +82,8 @@ AC_CHECK_HEADERS([ \
> sigprocmask \
> sigtimedwait \
> socketpair \
> + wait \
> + waitpid \
> ])
>
> # This is needed for RHEL 5 and uclibc-ng < 1.0.39.
> diff --git a/gdbsupport/config.in b/gdbsupport/config.in
> index 8467072752a6..0beacf22c057 100644
> --- a/gdbsupport/config.in
> +++ b/gdbsupport/config.in
> @@ -319,6 +319,12 @@
> /* Define to 1 if you have the <vfork.h> header file. */
> #undef HAVE_VFORK_H
>
> +/* Define to 1 if you have the `wait' function. */
> +#undef HAVE_WAIT
> +
> +/* Define to 1 if you have the `waitpid' function. */
> +#undef HAVE_WAITPID
> +
> /* Define to 1 if you have the <wait.h> header file. */
> #undef HAVE_WAIT_H
>
> diff --git a/gdbsupport/configure b/gdbsupport/configure
> index 87980f6870f3..0d994933b7d9 100755
> --- a/gdbsupport/configure
> +++ b/gdbsupport/configure
> @@ -11546,6 +11546,8 @@ fi
> sigprocmask \
> sigtimedwait \
> socketpair \
> + wait \
> + waitpid \
>
> do :
> as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
> diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
> index 3980e3f5ac1c..4cab8f9d48de 100644
> --- a/gdbsupport/eintr.h
> +++ b/gdbsupport/eintr.h
> @@ -71,11 +71,13 @@ handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
> return ret;
> }
>
> +#ifdef HAVE_WAITPID
> inline pid_t
> waitpid (pid_t pid, int *wstatus, int options)
> {
> return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
> }
> +#endif /* HAVE_WAITPID */
>
> inline int
> open (const char *pathname, int flags)
> @@ -83,11 +85,13 @@ open (const char *pathname, int flags)
> return gdb::handle_eintr (-1, ::open, pathname, flags);
> }
>
> +#ifdef HAVE_WAIT
> inline pid_t
> wait (int *wstatus)
> {
> return gdb::handle_eintr (-1, ::wait, wstatus);
> }
> +#endif /* HAVE_WAIT */
>
> inline int
> close (int fd)
>
> base-commit: 3eccfdce99e030be759ea20c66d6f78978b11a25
Tom> FWIW, I'm a bit surprised that it's necessary to add the configure
Tom> checks in gdbsupport, gdb and gdbserver, I would have expected that
Tom> adding it in gdbsupport would have been enough, but I guess things
Tom> work differently.
It probably should work already, since common-defs.h includes
gdbsupport/config.h, and so should pick up the results of
gdbsupport/configure.
Tom
@@ -71,23 +71,25 @@ handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
return ret;
}
+#ifndef USE_WIN32API
inline pid_t
waitpid (pid_t pid, int *wstatus, int options)
{
return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
}
-inline int
-open (const char *pathname, int flags)
-{
- return gdb::handle_eintr (-1, ::open, pathname, flags);
-}
-
inline pid_t
wait (int *wstatus)
{
return gdb::handle_eintr (-1, ::wait, wstatus);
}
+#endif
+
+inline int
+open (const char *pathname, int flags)
+{
+ return gdb::handle_eintr (-1, ::open, pathname, flags);
+}
inline int
close (int fd)