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

[Tom de Vries <tdevries@suse.de>]

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