[1/3,gdbsupport] Add gdb::{waitpid,read,write,close}

  We have gdb::handle_eintr, which allows us to rewrite:
...
  ssize_t ret;
    do
      {
        errno = 0;
        ret = ::write (pipe[1], "+", 1);
      }
    while (ret == -1 && errno == EINTR);
...
into:
...
  ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
...

However, the call to write got a bit mangled, requiring effort to decode it
back to its original form.

Instead, add a new function gdb::write that allows us to write:
...
  ssize_t ret = gdb::write (pipe[1], "+", 1);
...

Likewise for waitpid, read and close.

Tested on x86_64-linux.
---
 gdb/cli/cli-cmds.c      |  2 +-
 gdb/nat/linux-waitpid.c |  2 +-
 gdbserver/netbsd-low.cc | 10 ++++----
 gdbsupport/Makefile.am  |  1 +
 gdbsupport/Makefile.in  |  1 +
 gdbsupport/eintr.cc     | 56 +++++++++++++++++++++++++++++++++++++++++
 gdbsupport/eintr.h      |  7 ++++++
 7 files changed, 72 insertions(+), 7 deletions(-)
 create mode 100644 gdbsupport/eintr.cc


base-commit: 77f6ff446173ac7790f35d43de7d196a768c38b1
  

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> +namespace gdb {
Tom> +
Tom> +extern "C" {

I didn't even know this was valid ... but is it really needed?
It seems wrong.

Tom> +  pid_t
Tom> +  waitpid (pid_t pid, int *wstatus, int options)
Tom> +  {
Tom> +    return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
Tom> +  }

Also it seems like these could all be inline functions in the header
anyway.

Tom
  

On 11/1/24 18:35, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> +namespace gdb {
> Tom> +
> Tom> +extern "C" {
> 
> I didn't even know this was valid ... but is it really needed?
> It seems wrong.
> 

It seemed to work:
...
$ cat test.c
namespace gdb {

   extern "C" {
     int foo (void) { return 1; };
   }
}

$ cat test2.c
namespace gdb {
   extern "C" {
     int foo (void);
   }
}

int
main (void)
{
   return gdb::foo ();
}
$ g++ test.c test2.c
...
but the linkage symbol is plain foo:
...
$ nm ./a.out  | grep foo
00000000004004c7 T foo
...

So, you're right, this is wrong.

I seem to have ended up with gdb_waitpid instead of gdb::waitpid in v2, 
similar to gdb_select, but thinking about it now I slightly prefer 
gdb::waitpid.  I suppose it doesn't matter that much.

> Tom> +  pid_t
> Tom> +  waitpid (pid_t pid, int *wstatus, int options)
> Tom> +  {
> Tom> +    return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
> Tom> +  }
> 
> Also it seems like these could all be inline functions in the header
> anyway.

Done in v2.  In the case of fnctl, I ended up with a template to handle 
the variable number of arguments.

Submitted here ( 
https://sourceware.org/pipermail/gdb-patches/2024-November/212941.html ).

Thanks,
- Tom
  

On 11/5/24 13:07, Tom de Vries wrote:
> On 11/1/24 18:35, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> +namespace gdb {
>> Tom> +
>> Tom> +extern "C" {
>>
>> I didn't even know this was valid ... but is it really needed?
>> It seems wrong.
>>
> 
> It seemed to work:
> ...
> $ cat test.c
> namespace gdb {
> 
>    extern "C" {
>      int foo (void) { return 1; };
>    }
> }
> 
> $ cat test2.c
> namespace gdb {
>    extern "C" {
>      int foo (void);
>    }
> }
> 
> int
> main (void)
> {
>    return gdb::foo ();
> }
> $ g++ test.c test2.c
> ...
> but the linkage symbol is plain foo:
> ...
> $ nm ./a.out  | grep foo
> 00000000004004c7 T foo
> ...
> 
> So, you're right, this is wrong.
> 
> I seem to have ended up with gdb_waitpid instead of gdb::waitpid in v2, 
> similar to gdb_select, but thinking about it now I slightly prefer 
> gdb::waitpid.  I suppose it doesn't matter that much.
> 

When looking at this again today, it bugged me enough rewrite v2 in the 
gdb::waitpid style.  Committed after retesting.

Thanks,
- Tom

>> Tom> +  pid_t
>> Tom> +  waitpid (pid_t pid, int *wstatus, int options)
>> Tom> +  {
>> Tom> +    return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, 
>> options);
>> Tom> +  }
>>
>> Also it seems like these could all be inline functions in the header
>> anyway.
> 
> Done in v2.  In the case of fnctl, I ended up with a template to handle 
> the variable number of arguments.
> 
> Submitted here ( https://sourceware.org/pipermail/gdb-patches/2024- 
> November/212941.html ).
> 
> Thanks,
> - Tom
>
  

On 2024-11-22 11:46, Tom de Vries wrote:
> On 11/5/24 13:07, Tom de Vries wrote:
>> On 11/1/24 18:35, Tom Tromey wrote:
>>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>>
>>> Tom> +namespace gdb {
>>> Tom> +
>>> Tom> +extern "C" {
>>>
>>> I didn't even know this was valid ... but is it really needed?
>>> It seems wrong.
>>>
>>
>> It seemed to work:
>> ...
>> $ cat test.c
>> namespace gdb {
>>
>>    extern "C" {
>>      int foo (void) { return 1; };
>>    }
>> }
>>
>> $ cat test2.c
>> namespace gdb {
>>    extern "C" {
>>      int foo (void);
>>    }
>> }
>>
>> int
>> main (void)
>> {
>>    return gdb::foo ();
>> }
>> $ g++ test.c test2.c
>> ...
>> but the linkage symbol is plain foo:
>> ...
>> $ nm ./a.out  | grep foo
>> 00000000004004c7 T foo
>> ...
>>
>> So, you're right, this is wrong.
>>
>> I seem to have ended up with gdb_waitpid instead of gdb::waitpid in v2, similar to gdb_select, but thinking about it now I slightly prefer gdb::waitpid.  I suppose it doesn't matter that much.
>>
> 
> When looking at this again today, it bugged me enough rewrite v2 in the gdb::waitpid style.  Committed after retesting.

When building for mingw-w64, I see this, which seems related:

      CXX    cli/cli-cmds.o
    In file included from /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:58:
    /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
    /home/simark/src/binutils-gdb/gdb/../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
    /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
       75 | waitpid (pid_t pid, int *wstatus, int options)
          | ^~~~~~~

Note that to get to it, you have to step past this other build error in
gdbserver (by doing `make all-gdb` for instance):

  https://inbox.sourceware.org/gdb-patches/75926446-4c8a-40ee-8ed8-ab55f38e1520@simark.ca/T/#m194744b18419e37dfb4ec82c8e0ce73fb017ba17

Simon
  

On 11/29/24 17:15, Simon Marchi wrote:
> 
> 
> On 2024-11-22 11:46, Tom de Vries wrote:
>> On 11/5/24 13:07, Tom de Vries wrote:
>>> On 11/1/24 18:35, Tom Tromey wrote:
>>>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>>>
>>>> Tom> +namespace gdb {
>>>> Tom> +
>>>> Tom> +extern "C" {
>>>>
>>>> I didn't even know this was valid ... but is it really needed?
>>>> It seems wrong.
>>>>
>>>
>>> It seemed to work:
>>> ...
>>> $ cat test.c
>>> namespace gdb {
>>>
>>>     extern "C" {
>>>       int foo (void) { return 1; };
>>>     }
>>> }
>>>
>>> $ cat test2.c
>>> namespace gdb {
>>>     extern "C" {
>>>       int foo (void);
>>>     }
>>> }
>>>
>>> int
>>> main (void)
>>> {
>>>     return gdb::foo ();
>>> }
>>> $ g++ test.c test2.c
>>> ...
>>> but the linkage symbol is plain foo:
>>> ...
>>> $ nm ./a.out  | grep foo
>>> 00000000004004c7 T foo
>>> ...
>>>
>>> So, you're right, this is wrong.
>>>
>>> I seem to have ended up with gdb_waitpid instead of gdb::waitpid in v2, similar to gdb_select, but thinking about it now I slightly prefer gdb::waitpid.  I suppose it doesn't matter that much.
>>>
>>
>> When looking at this again today, it bugged me enough rewrite v2 in the gdb::waitpid style.  Committed after retesting.
> 
> When building for mingw-w64, I see this, which seems related:
> 
>        CXX    cli/cli-cmds.o
>      In file included from /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:58:
>      /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
>      /home/simark/src/binutils-gdb/gdb/../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
>      /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
>         75 | waitpid (pid_t pid, int *wstatus, int options)
>            | ^~~~~~~
> 
> Note that to get to it, you have to step past this other build error in
> gdbserver (by doing `make all-gdb` for instance):
> 
>    https://inbox.sourceware.org/gdb-patches/75926446-4c8a-40ee-8ed8-ab55f38e1520@simark.ca/T/#m194744b18419e37dfb4ec82c8e0ce73fb017ba17

Hi Simon,

thanks for reporting this.

I don't have a windows setup unfortunately, so I have no way of 
reproducing this, and at this point I have no idea why waitpid is not 
declared.

Thanks,
- Tom
  

On 2024-11-29 11:23, Tom de Vries wrote:
> On 11/29/24 17:15, Simon Marchi wrote:
>>
>>
>> On 2024-11-22 11:46, Tom de Vries wrote:
>>> On 11/5/24 13:07, Tom de Vries wrote:
>>>> On 11/1/24 18:35, Tom Tromey wrote:
>>>>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>>>>
>>>>> Tom> +namespace gdb {
>>>>> Tom> +
>>>>> Tom> +extern "C" {
>>>>>
>>>>> I didn't even know this was valid ... but is it really needed?
>>>>> It seems wrong.
>>>>>
>>>>
>>>> It seemed to work:
>>>> ...
>>>> $ cat test.c
>>>> namespace gdb {
>>>>
>>>>     extern "C" {
>>>>       int foo (void) { return 1; };
>>>>     }
>>>> }
>>>>
>>>> $ cat test2.c
>>>> namespace gdb {
>>>>     extern "C" {
>>>>       int foo (void);
>>>>     }
>>>> }
>>>>
>>>> int
>>>> main (void)
>>>> {
>>>>     return gdb::foo ();
>>>> }
>>>> $ g++ test.c test2.c
>>>> ...
>>>> but the linkage symbol is plain foo:
>>>> ...
>>>> $ nm ./a.out  | grep foo
>>>> 00000000004004c7 T foo
>>>> ...
>>>>
>>>> So, you're right, this is wrong.
>>>>
>>>> I seem to have ended up with gdb_waitpid instead of gdb::waitpid in v2, similar to gdb_select, but thinking about it now I slightly prefer gdb::waitpid.  I suppose it doesn't matter that much.
>>>>
>>>
>>> When looking at this again today, it bugged me enough rewrite v2 in the gdb::waitpid style.  Committed after retesting.
>>
>> When building for mingw-w64, I see this, which seems related:
>>
>>        CXX    cli/cli-cmds.o
>>      In file included from /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:58:
>>      /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
>>      /home/simark/src/binutils-gdb/gdb/../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
>>      /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
>>         75 | waitpid (pid_t pid, int *wstatus, int options)
>>            | ^~~~~~~
>>
>> Note that to get to it, you have to step past this other build error in
>> gdbserver (by doing `make all-gdb` for instance):
>>
>>    https://inbox.sourceware.org/gdb-patches/75926446-4c8a-40ee-8ed8-ab55f38e1520@simark.ca/T/#m194744b18419e37dfb4ec82c8e0ce73fb017ba17
> 
> Hi Simon,
> 
> thanks for reporting this.
> 
> I don't have a windows setup unfortunately, so I have no way of reproducing this, and at this point I have no idea why waitpid is not declared.

For just build testing, I cross-compile with a mingw-w64 cross compiler,
which is available on most distros, looks like it is on Suse:

https://en.opensuse.org/MinGW

The problem is that you might have to cross build the packages gdb
requires.  If you're lucky, your distro also packages them for
mingw-w64.  For instance, it looks like Suse packages libgmp:

https://software.opensuse.org/package/mingw64-gmp?locale=ro

These are the configure flags I currently use for this build:

   '--host=x86_64-w64-mingw32' \
   'CFLAGS=-g3 -O0' \
   'CC=ccache x86_64-w64-mingw32-gcc' \
   'CXXFLAGS=-g3 -O0 -fmax-errors=1' \
   'CXX=ccache x86_64-w64-mingw32-g++' \
   '--disable-source-highlight' \
   '--disable-gold' \
   '--disable-ld' \
   '--disable-binutils' \
   '--disable-gprof' \
   '--disable-gas' \
   '--without-python' \
   '--disable-nls'

I'll try to get it to build inside a Suse container.

Simon
  

On 11/29/24 17:27, Simon Marchi wrote:
> 
> 
> On 2024-11-29 11:23, Tom de Vries wrote:
>> On 11/29/24 17:15, Simon Marchi wrote:
>>>
>>>
>>> On 2024-11-22 11:46, Tom de Vries wrote:
>>>> On 11/5/24 13:07, Tom de Vries wrote:
>>>>> On 11/1/24 18:35, Tom Tromey wrote:
>>>>>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>>>>>
>>>>>> Tom> +namespace gdb {
>>>>>> Tom> +
>>>>>> Tom> +extern "C" {
>>>>>>
>>>>>> I didn't even know this was valid ... but is it really needed?
>>>>>> It seems wrong.
>>>>>>
>>>>>
>>>>> It seemed to work:
>>>>> ...
>>>>> $ cat test.c
>>>>> namespace gdb {
>>>>>
>>>>>      extern "C" {
>>>>>        int foo (void) { return 1; };
>>>>>      }
>>>>> }
>>>>>
>>>>> $ cat test2.c
>>>>> namespace gdb {
>>>>>      extern "C" {
>>>>>        int foo (void);
>>>>>      }
>>>>> }
>>>>>
>>>>> int
>>>>> main (void)
>>>>> {
>>>>>      return gdb::foo ();
>>>>> }
>>>>> $ g++ test.c test2.c
>>>>> ...
>>>>> but the linkage symbol is plain foo:
>>>>> ...
>>>>> $ nm ./a.out  | grep foo
>>>>> 00000000004004c7 T foo
>>>>> ...
>>>>>
>>>>> So, you're right, this is wrong.
>>>>>
>>>>> I seem to have ended up with gdb_waitpid instead of gdb::waitpid in v2, similar to gdb_select, but thinking about it now I slightly prefer gdb::waitpid.  I suppose it doesn't matter that much.
>>>>>
>>>>
>>>> When looking at this again today, it bugged me enough rewrite v2 in the gdb::waitpid style.  Committed after retesting.
>>>
>>> When building for mingw-w64, I see this, which seems related:
>>>
>>>         CXX    cli/cli-cmds.o
>>>       In file included from /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:58:
>>>       /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
>>>       /home/simark/src/binutils-gdb/gdb/../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
>>>       /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
>>>          75 | waitpid (pid_t pid, int *wstatus, int options)
>>>             | ^~~~~~~
>>>
>>> Note that to get to it, you have to step past this other build error in
>>> gdbserver (by doing `make all-gdb` for instance):
>>>
>>>     https://inbox.sourceware.org/gdb-patches/75926446-4c8a-40ee-8ed8-ab55f38e1520@simark.ca/T/#m194744b18419e37dfb4ec82c8e0ce73fb017ba17
>>
>> Hi Simon,
>>
>> thanks for reporting this.
>>
>> I don't have a windows setup unfortunately, so I have no way of reproducing this, and at this point I have no idea why waitpid is not declared.
> 
> For just build testing, I cross-compile with a mingw-w64 cross compiler,
> which is available on most distros, looks like it is on Suse:
> 
> https://en.opensuse.org/MinGW
> 
> The problem is that you might have to cross build the packages gdb
> requires.  If you're lucky, your distro also packages them for
> mingw-w64.  For instance, it looks like Suse packages libgmp:
> 
> https://software.opensuse.org/package/mingw64-gmp?locale=ro
> 
> These are the configure flags I currently use for this build:
> 
>     '--host=x86_64-w64-mingw32' \
>     'CFLAGS=-g3 -O0' \
>     'CC=ccache x86_64-w64-mingw32-gcc' \
>     'CXXFLAGS=-g3 -O0 -fmax-errors=1' \
>     'CXX=ccache x86_64-w64-mingw32-g++' \
>     '--disable-source-highlight' \
>     '--disable-gold' \
>     '--disable-ld' \
>     '--disable-binutils' \
>     '--disable-gprof' \
>     '--disable-gas' \
>     '--without-python' \
>     '--disable-nls'
> 
> I'll try to get it to build inside a Suse container.
> 

Oh, I see, that's a good idea, thanks, I'll try that.

- Tom
  

>> The problem is that you might have to cross build the packages gdb
>> requires.  If you're lucky, your distro also packages them for
>> mingw-w64.  For instance, it looks like Suse packages libgmp:
>>
>> https://software.opensuse.org/package/mingw64-gmp?locale=ro
>>
>> These are the configure flags I currently use for this build:
>>
>>     '--host=x86_64-w64-mingw32' \
>>     'CFLAGS=-g3 -O0' \
>>     'CC=ccache x86_64-w64-mingw32-gcc' \
>>     'CXXFLAGS=-g3 -O0 -fmax-errors=1' \
>>     'CXX=ccache x86_64-w64-mingw32-g++' \
>>     '--disable-source-highlight' \
>>     '--disable-gold' \
>>     '--disable-ld' \
>>     '--disable-binutils' \
>>     '--disable-gprof' \
>>     '--disable-gas' \
>>     '--without-python' \
>>     '--disable-nls'
>>
>> I'll try to get it to build inside a Suse container.
>>
> 
> Oh, I see, that's a good idea, thanks, I'll try that.
> 
> - Tom

Ok, I think I got it to work.  See attached Dockerfile.

Use with (fixup the path to the source tree in the `-v` flag):

$ docker build -t opensuse-gdb-mingw-w64 .
$ docker run -v /home/simark/src/binutils-gdb:/src -it opensuse-gdb-mingw-w64:latest
# ./build.sh

Of course, since you likely have a tumbleweed system already, you can
simply install the required packages on that machine.

Simon
FROM opensuse/tumbleweed:latest

RUN mkdir /build
WORKDIR /build

RUN zypper install -y \
    mingw64-cross-gcc \
    mingw64-cross-gcc-c++ \
    mingw64-gmp-devel \
    mingw64-mpfr-devel \
    make \
    gcc \
    makeinfo \
    flex \
    bison

RUN echo "/src/configure '--host=x86_64-w64-mingw32'" > /build/build.sh
RUN echo "make -j $(nproc)" >> /build/build.sh
RUN chmod +x /build/build.sh
  

On 11/29/24 18:23, Simon Marchi wrote:
> 
>>> The problem is that you might have to cross build the packages gdb
>>> requires.  If you're lucky, your distro also packages them for
>>> mingw-w64.  For instance, it looks like Suse packages libgmp:
>>>
>>> https://software.opensuse.org/package/mingw64-gmp?locale=ro
>>>
>>> These are the configure flags I currently use for this build:
>>>
>>>      '--host=x86_64-w64-mingw32' \
>>>      'CFLAGS=-g3 -O0' \
>>>      'CC=ccache x86_64-w64-mingw32-gcc' \
>>>      'CXXFLAGS=-g3 -O0 -fmax-errors=1' \
>>>      'CXX=ccache x86_64-w64-mingw32-g++' \
>>>      '--disable-source-highlight' \
>>>      '--disable-gold' \
>>>      '--disable-ld' \
>>>      '--disable-binutils' \
>>>      '--disable-gprof' \
>>>      '--disable-gas' \
>>>      '--without-python' \
>>>      '--disable-nls'
>>>
>>> I'll try to get it to build inside a Suse container.
>>>
>>
>> Oh, I see, that's a good idea, thanks, I'll try that.
>>
>> - Tom
> 
> Ok, I think I got it to work.  See attached Dockerfile.
> 
> Use with (fixup the path to the source tree in the `-v` flag):
> 
> $ docker build -t opensuse-gdb-mingw-w64 .
> $ docker run -v /home/simark/src/binutils-gdb:/src -it opensuse-gdb-mingw-w64:latest
> # ./build.sh
> 
> Of course, since you likely have a tumbleweed system already, you can
> simply install the required packages on that machine.
> 

Hi Simon,

thanks for the help.

I've reproduced the build error now (on my Leap 15.5 system), I'll take 
a look.

Thanks,
- Tom
  

Tom de Vries <tdevries@suse.de> writes:

>> When building for mingw-w64, I see this, which seems related:
>>        CXX    cli/cli-cmds.o
>>      In file included from /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:58:
>>      /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
>>      /home/simark/src/binutils-gdb/gdb/../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
>>      /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
>>         75 | waitpid (pid_t pid, int *wstatus, int options)
>>            | ^~~~~~~
>> Note that to get to it, you have to step past this other build error in
>> gdbserver (by doing `make all-gdb` for instance):
>>    https://inbox.sourceware.org/gdb-patches/75926446-4c8a-40ee-8ed8-ab55f38e1520@simark.ca/T/#m194744b18419e37dfb4ec82c8e0ce73fb017ba17
>
> Hi Simon,
>
> thanks for reporting this.
>
> I don't have a windows setup unfortunately, so I have no way of reproducing this, and at this point I have no idea why waitpid is not declared.

FWIW (not much: not very Windows-savvy), I fixed my test build with the
following patch.  I was also getting a "'::wait' has not been declared"
error, hence the extra #ifdef (found no HAVE_ macro for 'wait', so went
with HAVE_SYS_WAIT_H after glancing the 3posix manpage).

(Figured I'd share since that let me build & test native Windows, though
emphatically not suggesting this as a "proper fix": not sure "wait ⇔
HAVE_SYS_WAIT_H" is a correct implication, haven't checked if gnulib
might help, …)


diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
index 3980e3f5ac1..2bd17108f6e 100644
--- a/gdbsupport/eintr.h
+++ b/gdbsupport/eintr.h
@@ -22,7 +22,9 @@
 
 #include <cerrno>
 #include <sys/types.h>
+#ifdef HAVE_SYS_WAIT_H
 #include <sys/wait.h>
+#endif
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <unistd.h>
@@ -71,11 +73,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
 
 inline int
 open (const char *pathname, int flags)
@@ -83,11 +87,13 @@ open (const char *pathname, int flags)
   return gdb::handle_eintr (-1, ::open, pathname, flags);
 }
 
+#ifdef HAVE_SYS_WAIT_H
 inline pid_t
 wait (int *wstatus)
 {
   return gdb::handle_eintr (-1, ::wait, wstatus);
 }
+#endif
 
 inline int
 close (int fd)
  

On 12/2/24 10:38, Kévin Le Gouguec wrote:
> Tom de Vries <tdevries@suse.de> writes:
> 
>>> When building for mingw-w64, I see this, which seems related:
>>>         CXX    cli/cli-cmds.o
>>>       In file included from /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:58:
>>>       /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h: In function ‘pid_t gdb::waitpid(pid_t, int*, int)’:
>>>       /home/simark/src/binutils-gdb/gdb/../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
>>>       /home/simark/src/binutils-gdb/gdb/../gdbsupport/eintr.h:75:1: note: ‘gdb::waitpid’ declared here
>>>          75 | waitpid (pid_t pid, int *wstatus, int options)
>>>             | ^~~~~~~
>>> Note that to get to it, you have to step past this other build error in
>>> gdbserver (by doing `make all-gdb` for instance):
>>>     https://inbox.sourceware.org/gdb-patches/75926446-4c8a-40ee-8ed8-ab55f38e1520@simark.ca/T/#m194744b18419e37dfb4ec82c8e0ce73fb017ba17
>>
>> Hi Simon,
>>
>> thanks for reporting this.
>>
>> I don't have a windows setup unfortunately, so I have no way of reproducing this, and at this point I have no idea why waitpid is not declared.
> 
> FWIW (not much: not very Windows-savvy), 

Hi Kevin,

same here.

> I fixed my test build with the
> following patch.  I was also getting a "'::wait' has not been declared"
> error, hence the extra #ifdef (found no HAVE_ macro for 'wait', so went
> with HAVE_SYS_WAIT_H after glancing the 3posix manpage).
> 
> (Figured I'd share since that let me build & test native Windows, though
> emphatically not suggesting this as a "proper fix": not sure "wait ⇔
> HAVE_SYS_WAIT_H" is a correct implication, haven't checked if gnulib
> might help, …)
> 

Thanks for sharing this.

I've been looking into this, and am currently testing a patch that 
simply uses "#ifndef USE_WIN32API".

The HAVE_WAITPID is available in gdb but not gdbserver, so it could be 
used but would need moving to gdbsupport or duplicating to gdbserver.
I suppose it would make sense to add a HAVE_WAIT alongside while doing that.

I've left the '#include <sys/wait.h>' unmodified, since it causes no 
compilation problems, because there's a gnulib fallback.

The situation around sys/wait.h and HAVE_SYS_WAIT_H is a bit confusing, 
for me at least.  The platform doesn't have a sys/wait.h, but gnulib 
provides a fallback.  While configuring, we ignore the gnulib header, 
and so HAVE_SYS_WAIT_H is undefined.  But when compiling the gnulib 
header is not ignored, so we can use '#include <sys/wait.h>'.  AFAIU in 
the current situation, the only thing that guarding '#include 
<sys/wait.h>' with HAVE_SYS_WAIT_H achieves is preventing the gnulib 
fallback
from being included.  I suspect that this is a configuration bug, but 
I'm not sure.

BTW, the canonical way (according to ARI) of including <sys/wait.h> 
seems to be including gdb_wait.h (which does use HAVE_SYS_WAIT_H).

Thanks,
- Tom

> diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
> index 3980e3f5ac1..2bd17108f6e 100644
> --- a/gdbsupport/eintr.h
> +++ b/gdbsupport/eintr.h
> @@ -22,7 +22,9 @@
>   
>   #include <cerrno>
>   #include <sys/types.h>
> +#ifdef HAVE_SYS_WAIT_H
>   #include <sys/wait.h>
> +#endif
>   #include <sys/stat.h>
>   #include <fcntl.h>
>   #include <unistd.h>
> @@ -71,11 +73,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
>   
>   inline int
>   open (const char *pathname, int flags)
> @@ -83,11 +87,13 @@ open (const char *pathname, int flags)
>     return gdb::handle_eintr (-1, ::open, pathname, flags);
>   }
>   
> +#ifdef HAVE_SYS_WAIT_H
>   inline pid_t
>   wait (int *wstatus)
>   {
>     return gdb::handle_eintr (-1, ::wait, wstatus);
>   }
> +#endif
>   
>   inline int
>   close (int fd)
>