MinGW compilation warnings in libiberty's waitpid.c

MinGW compilation warnings in libiberty's waitpid.c

[Eli Zaretskii]

When compiling libiberty (as part of GDB) with MinGW on MS-Windows, I
see the following warning:

     gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS  -I. -I./../include   -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic  -D_GNU_SOURCE ./waitpid.c -o waitpid.o
     ./waitpid.c: In function 'waitpid':
     ./waitpid.c:31:18: warning: implicit declaration of function 'wait' [-Wimplicit-function-declaration]
	    int wpid = wait(stat_loc);
		       ^

The file waitpid.c should not be built on MinGW, as it is not needed
on Windows, and will not work if the function is called (because
there's no 'wait' function on MS-Windows).

Re: MinGW compilation warnings in libiberty's waitpid.c

[Pedro Alves]

On 05/08/2017 04:27 PM, Eli Zaretskii wrote:
> When compiling libiberty (as part of GDB) with MinGW on MS-Windows, I
> see the following warning:
> 
>      gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS  -I. -I./../include   -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic  -D_GNU_SOURCE ./waitpid.c -o waitpid.o
>      ./waitpid.c: In function 'waitpid':
>      ./waitpid.c:31:18: warning: implicit declaration of function 'wait' [-Wimplicit-function-declaration]
> 	    int wpid = wait(stat_loc);
> 		       ^
> 
> The file waitpid.c should not be built on MinGW, as it is not needed
> on Windows, and will not work if the function is called (because
> there's no 'wait' function on MS-Windows).
> 

Makes sense, but did you check whether there's an obvious place such
a change could be done in configure.ac?  

I wonder what code relies on this replacement, actually.  In liberty,
the only waitpid calls are in pex-unix.c, but those are all guarded
by HAVE_WAITPID.
Maybe it was used at some point when libiberty was a target library?

So I wonder whether we could just unconditionally remove the waitpid
replacement instead.

Thanks,
Pedro Alves

Re: MinGW compilation warnings in libiberty's waitpid.c

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 19 May 2017 16:36:46 +0100
> 
> So I wonder whether we could just unconditionally remove the waitpid
> replacement instead.

That's probably the best path forward.

Re: MinGW compilation warnings in libiberty's waitpid.c

[DJ Delorie]

Please try this patch, since my mingw environment is old:

Index: libiberty/ChangeLog
===================================================================
--- libiberty/ChangeLog (revision 248307)
+++ libiberty/ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2017-05-19  Eli Zaretskii <eliz@gnu.org>
+
+       * configure.ac (*-*-mingw*): Don't build waitpid.c.
+
 2017-05-02  Iain Buclaw  <ibuclaw@gdcproject.org>
 
        * d-demangle.c (dlang_hexdigit): New function.
Index: libiberty/configure.ac
===================================================================
--- libiberty/configure.ac      (revision 248307)
+++ libiberty/configure.ac      (working copy)
@@ -493,7 +493,6 @@
     AC_LIBOBJ([strnlen])
     AC_LIBOBJ([strverscmp])
     AC_LIBOBJ([vasprintf])
-    AC_LIBOBJ([waitpid])
 
     for f in $funcs; do
       case "$f" in

Re: MinGW compilation warnings in libiberty's waitpid.c

[Eli Zaretskii]

> From: DJ Delorie <dj@redhat.com>
> Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> Date: Fri, 19 May 2017 21:28:25 -0400
> 
> 
> Please try this patch, since my mingw environment is old:
> 
> Index: libiberty/ChangeLog
> ===================================================================
> --- libiberty/ChangeLog (revision 248307)
> +++ libiberty/ChangeLog (working copy)
> @@ -1,3 +1,7 @@
> +2017-05-19  Eli Zaretskii <eliz@gnu.org>
> +
> +       * configure.ac (*-*-mingw*): Don't build waitpid.c.
> +
>  2017-05-02  Iain Buclaw  <ibuclaw@gdcproject.org>
>  
>         * d-demangle.c (dlang_hexdigit): New function.
> Index: libiberty/configure.ac
> ===================================================================
> --- libiberty/configure.ac      (revision 248307)
> +++ libiberty/configure.ac      (working copy)
> @@ -493,7 +493,6 @@
>      AC_LIBOBJ([strnlen])
>      AC_LIBOBJ([strverscmp])
>      AC_LIBOBJ([vasprintf])
> -    AC_LIBOBJ([waitpid])
>  
>      for f in $funcs; do
>        case "$f" in
> 

Hmm... no, this doesn't solve the problem.  The expansion of AC_LIBOBJ
for waitpid is gone from the configure script, but the value of
LIBOBJS in libiberty/Makefile still includes waitpid.o.  What else is
related to this?

One caveat: I needed to hack config/override.m4 to allow me to run
autoconf 2.69 I have installed, because otherwise it insists on
autoconf 2.64 which I don't have.  I hope this isn't the reason for
the incomplete solution.

Re: MinGW compilation warnings in libiberty's waitpid.c

[DJ Delorie]

Eli Zaretskii <eliz@gnu.org> writes:
> Hmm... no, this doesn't solve the problem.  The expansion of AC_LIBOBJ
> for waitpid is gone from the configure script, but the value of
> LIBOBJS in libiberty/Makefile still includes waitpid.o.  What else is
> related to this?

After re-reading the sources a bit, I come to the following
conclusions...

* $funcs is a list of functions libiberty should provide if the host
  doesn't have them.

* We can override what the host *has* but not what it *shouldn't* have.

Since (or "if") nobody will (should) use waitpid() on mingw anyway, and
since libiberty really wants to include waitpid.o, how difficult would
it be to use some #ifdefs to have waitpid() just return an error on
mingw?  That at least gets past the mingw build problem.

> One caveat: I needed to hack config/override.m4 to allow me to run
> autoconf 2.69 I have installed, because otherwise it insists on
> autoconf 2.64 which I don't have.  I hope this isn't the reason for
> the incomplete solution.

I have many versions of autoconf installed, each in their own
directories, and add the right one to my $PATH on a per-project basis.
Autoconf works just fine that way, and there have been plenty of cases
of autoconf output changing in, er, "unexpected" ways across autoconf
releases.  If you regen configure and an "svn diff" (or git diff) shows
unexpected changes, check your autoconf :-)

Same for automake and autogen.

Re: MinGW compilation warnings in libiberty's waitpid.c

[Eli Zaretskii]

> From: DJ Delorie <dj@redhat.com>
> Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> Date: Mon, 22 May 2017 15:38:40 -0400
> 
> Since (or "if") nobody will (should) use waitpid() on mingw anyway, and
> since libiberty really wants to include waitpid.o, how difficult would
> it be to use some #ifdefs to have waitpid() just return an error on
> mingw?  That at least gets past the mingw build problem.

Instead of making waitpid an always-failing stub on MinGW, wouldn't it
be better to make it work on MinGW?  Like this:

--- libiberty/waitpid.c~0	2016-08-01 18:50:21.000000000 +0300
+++ libiberty/waitpid.c	2017-05-23 21:19:34.302415000 +0300
@@ -23,6 +23,11 @@ does the return value.  The third argume
 #include <sys/wait.h>
 #endif
 
+#ifdef __MINGW32__
+#include <process.h>
+#define wait(s)  _cwait(s,pid,_WAIT_CHILD)
+#endif
+
 pid_t
 waitpid (pid_t pid, int *stat_loc, int options ATTRIBUTE_UNUSED)
 {

Re: MinGW compilation warnings in libiberty's waitpid.c

[DJ Delorie]

Eli Zaretskii <eliz@gnu.org> writes:
> Instead of making waitpid an always-failing stub on MinGW, wouldn't it
> be better to make it work on MinGW?  Like this:

That's up to you, if it's target-specific.  What about mingw64?

> --- libiberty/waitpid.c~0	2016-08-01 18:50:21.000000000 +0300
> +++ libiberty/waitpid.c	2017-05-23 21:19:34.302415000 +0300
> @@ -23,6 +23,11 @@ does the return value.  The third argume
>  #include <sys/wait.h>
>  #endif
>  
> +#ifdef __MINGW32__
> +#include <process.h>
> +#define wait(s)  _cwait(s,pid,_WAIT_CHILD)
> +#endif
> +
>  pid_t
>  waitpid (pid_t pid, int *stat_loc, int options ATTRIBUTE_UNUSED)
>  {

Re: MinGW compilation warnings in libiberty's waitpid.c

[Eli Zaretskii]

> From: DJ Delorie <dj@redhat.com>
> Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> Date: Tue, 23 May 2017 15:37:32 -0400
> 
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Instead of making waitpid an always-failing stub on MinGW, wouldn't it
> > be better to make it work on MinGW?  Like this:
> 
> That's up to you, if it's target-specific.

Then I prefer this solution.

> What about mingw64?

It will be covered as well, because it defines __MINGW32__, and
because _cwait is in the MS runtime, so available to MinGW64, too.

Is there anything else I need to do about this part to get it solved
in the upstream repository?

Thanks.

Re: MinGW compilation warnings in libiberty's waitpid.c

[DJ Delorie]

Eli Zaretskii <eliz@gnu.org> writes:
>> What about mingw64?
>
> It will be covered as well, because it defines __MINGW32__,

Ah, OK.

> Is there anything else I need to do about this part to get it solved
> in the upstream repository?

A ChangeLog entry would be nice, so I don't have to write one ;-)

Re: MinGW compilation warnings in libiberty's waitpid.c

[Eli Zaretskii]

> From: DJ Delorie <dj@redhat.com>
> Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> Date: Tue, 23 May 2017 22:26:22 -0400
> 
> > Is there anything else I need to do about this part to get it solved
> > in the upstream repository?
> 
> A ChangeLog entry would be nice, so I don't have to write one ;-)

Sure:

2017-05-24  Eli Zaretskii  <eliz@gnu.org>

	* libiberty/waitpid.c (wait) [__MINGW32__]: Define as a macro
	that calls _cwait, so that this function works on MinGW.

Re: MinGW compilation warnings in libiberty's waitpid.c

[DJ Delorie]

Thanks.  Committed!

Re: MinGW compilation warnings in libiberty's waitpid.c

[Eli Zaretskii]

> From: DJ Delorie <dj@redhat.com>
> Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> Date: Wed, 24 May 2017 17:36:16 -0400
> 
> Thanks.  Committed!

Thanks.  Is the environ thing also fixed?

Joel/Pedro, how should I go about making sure these changes are in the
GDB copy of libiberty?

Re: MinGW compilation warnings in libiberty's waitpid.c

[Joel Brobecker]

> Thanks.  Is the environ thing also fixed?
> 
> Joel/Pedro, how should I go about making sure these changes are in the
> GDB copy of libiberty?

Normally, I'd expect someone pushing to GCC's libibert to also
update our repo accordingly. However, it's easy to forget so,
if you notice a change that was not propagated to us, we just
cherry-pick those changes so as to make sure our copy is up
to date with GCC's. We also see the occasional "resync libiberty"
commits which act as a failsafe, but I don't think we should wait
for one of those.

Should those ones be pushed to the gdb-8.0-branch as well?

-- 
Joel

Re: MinGW compilation warnings in libiberty's waitpid.c

[Eli Zaretskii]

> Date: Fri, 26 May 2017 09:30:53 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: DJ Delorie <dj@redhat.com>, gcc-patches@gcc.gnu.org,
> 	gdb-patches@sourceware.org
> 
> Normally, I'd expect someone pushing to GCC's libibert to also
> update our repo accordingly. However, it's easy to forget so,
> if you notice a change that was not propagated to us, we just
> cherry-pick those changes so as to make sure our copy is up
> to date with GCC's. We also see the occasional "resync libiberty"
> commits which act as a failsafe, but I don't think we should wait
> for one of those.

What can I do to expedite the process?  This currently holds the 8.0
release, and I'm uneasy to be the culprit.

> Should those ones be pushed to the gdb-8.0-branch as well?

Yes.

Re: MinGW compilation warnings in libiberty's waitpid.c

[Joel Brobecker]

> What can I do to expedite the process?  This currently holds the 8.0
> release, and I'm uneasy to be the culprit.

The way I usually do it is grab the commit in GCC, and apply it
as is in the GDB repository. To make it easier, I use the git
mirror of the GCC repository, because I can then just add the
GCC repository as one of my remotes, fetch from there, and then
cherry-pick. If you prefer to work from GCC's SVN repository,
just use SVN to get the diff, and re-apply it by hand in GDB's
git repository. The advantage with the cherry-pick is that git
does nearly all of the work (revision log, etc).

-- 
Joel

Re: MinGW compilation warnings in libiberty's waitpid.c

[DJ Delorie]

Joel Brobecker <brobecker@adacore.com> writes:
> Normally, I'd expect someone pushing to GCC's libibert to also
> update our repo accordingly.

Note that I used to auto-sync libiberty from gcc to binutils/gdb, but
when binutils/gdb switched to git, that script broke, and as I don't
like using git, I announced that I would no longer be auto-syncing.  So
it's up to the projects using copies of libiberty to make sure they stay
in sync with the gcc master copy.

Re: MinGW compilation warnings in libiberty's waitpid.c

[Iain Buclaw]

On 26 May 2017 at 18:30, Joel Brobecker <brobecker@adacore.com> wrote:
>> Thanks.  Is the environ thing also fixed?
>>
>> Joel/Pedro, how should I go about making sure these changes are in the
>> GDB copy of libiberty?
>
> Normally, I'd expect someone pushing to GCC's libibert to also
> update our repo accordingly. However, it's easy to forget so,
> if you notice a change that was not propagated to us, we just
> cherry-pick those changes so as to make sure our copy is up
> to date with GCC's. We also see the occasional "resync libiberty"
> commits which act as a failsafe, but I don't think we should wait
> for one of those.
>

This has been on my todo-list for a little while, as re-syncing is
something I normally do after pushing D language support updates into
libiberty.  However I decided to give it a wait until I got all
pending patches in, the last of which I'm just pushing in now.

Regards,
Iain.

Re: MinGW compilation warnings in libiberty's waitpid.c

[Joel Brobecker]

> This has been on my todo-list for a little while, as re-syncing is
> something I normally do after pushing D language support updates into
> libiberty.  However I decided to give it a wait until I got all
> pending patches in, the last of which I'm just pushing in now.

That's very kind of you to help us with that :). Thank you!

-- 
Joel

Re: MinGW compilation warnings in libiberty's waitpid.c

[Iain Buclaw]

On 30 May 2017 at 19:10, Joel Brobecker <brobecker@adacore.com> wrote:
>> This has been on my todo-list for a little while, as re-syncing is
>> something I normally do after pushing D language support updates into
>> libiberty.  However I decided to give it a wait until I got all
>> pending patches in, the last of which I'm just pushing in now.
>
> That's very kind of you to help us with that :). Thank you!
>

I ended up not having time before going on holiday.  If the resync
hasn't already been done, I'll do it now.

Iain.

Re: MinGW compilation warnings in libiberty's waitpid.c

[Joel Brobecker]

> I ended up not having time before going on holiday.  If the resync
> hasn't already been done, I'll do it now.

Thanks for doing that, Iain!

-- 
Joel