diff mbox

MinGW build of GDB 8.2.90 (was: GDB 8.2.90 available for testing)

Message ID 837edjkbp4.fsf@gnu.org
State New
Headers show

Commit Message

Eli Zaretskii Feb. 28, 2019, 6:30 p.m. UTC 
> From: Joel Brobecker <brobecker@adacore.com>
> Date: Wed, 27 Feb 2019 09:51:12 +0400 (+04)
> 
> I have just finished creating the gdb-8.2.90 pre-release.
> It is available for download at the following location:
> 
>     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-8.2.90.tar.xz

Thanks, I've built this with mingw.org's MinGW and bumped into a few
problems.

First, the recent changes to support IPv6 caused compilation errors in
several files.  The problem is with several fragments such as this
one:

  #ifdef USE_WIN32API
  #include <winsock2.h>
  #include <wspiapi.h>
  #else
  ...

mingw.org's MinGW doesn't have wspiapi.h.  Moreover, the Microsoft
documentation indicates that to get prototypes of getaddrinfo,
freeaddrinfo, etc. one needs to include ws2tcpip.h (and not include
winsock2.h separately), see, for example,

  https://docs.microsoft.com/en-us/windows/desktop/api/ws2tcpip/nf-ws2tcpip-getaddrinfo

Sergio, can you tell why you used wspiapi.h instead?

Is it okay to push changes such as the one below, master and branch
(after filing a Bugzilla report)?


Note that one other side effect of the IPv6 support additions is that
on MS-Windows GDB will no longer run on versions older than XP, I
guess this is something that should be mentioned in NEWS?

The other two problems were minor warning, one in tui.c about an
unused variable 'cap' (it is not used on Windows) and another in
xml-syscall.c:

       CXX    xml-syscall.o
     xml-syscall.c: In function 'bool xml_list_syscalls_by_group(gdbarch*, const char*, std::vector<int>*)':
     xml-syscall.c:475:14: warning: types may not be defined in a for-range-declaration
	for (const struct syscall_desc *sysdesc : groupdesc->syscalls)
		   ^~~~~~

I solved the latter by removing "struct" from the declaration.  This
is with GCC 6.3.0; is that a GCC bug? is removing "struct" the right
solution?

Thanks.

Comments

Sergio Durigan Junior Feb. 28, 2019, 6:55 p.m. UTC | #1 
On Thursday, February 28 2019, Eli Zaretskii wrote:

>> From: Joel Brobecker <brobecker@adacore.com>
>> Date: Wed, 27 Feb 2019 09:51:12 +0400 (+04)
>> 
>> I have just finished creating the gdb-8.2.90 pre-release.
>> It is available for download at the following location:
>> 
>>     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-8.2.90.tar.xz
>
> Thanks, I've built this with mingw.org's MinGW and bumped into a few
> problems.
>
> First, the recent changes to support IPv6 caused compilation errors in
> several files.  The problem is with several fragments such as this
> one:
>
>   #ifdef USE_WIN32API
>   #include <winsock2.h>
>   #include <wspiapi.h>
>   #else
>   ...
>
> mingw.org's MinGW doesn't have wspiapi.h.  Moreover, the Microsoft
> documentation indicates that to get prototypes of getaddrinfo,
> freeaddrinfo, etc. one needs to include ws2tcpip.h (and not include
> winsock2.h separately), see, for example,
>
>   https://docs.microsoft.com/en-us/windows/desktop/api/ws2tcpip/nf-ws2tcpip-getaddrinfo
>
> Sergio, can you tell why you used wspiapi.h instead?

Thanks for the report, Eli.

To be honest, I don't remember where I found the information that
wspiapi.h needed to be used, but I do remember finding it somewhere on
the internet, and since I don't use Windows, I assumed it was correct.

However, and more importantly, I remember testing the whole patch by
compiling it using a mingw32 compiler on Fedora, and it was working
correctly.  In fact, we even have a mingw32 builder on our BuildBot
(running on Fedora), and it is still compiling GDB without problems:

  https://gdb-build.sergiodj.net/builders/Fedora-x86_64-w64-mingw32

So apparently this error is only triggered when you use mingw on
Windows...?  I don't know.

> Is it okay to push changes such as the one below, master and branch
> (after filing a Bugzilla report)?
>
> --- ./gdb/common/netstuff.c~0	2019-02-27 06:51:50.000000000 +0200
> +++ ./gdb/common/netstuff.c	2019-02-28 08:56:07.511568800 +0200
> @@ -21,8 +21,11 @@
>  #include <algorithm>
>  
>  #ifdef USE_WIN32API
> -#include <winsock2.h>
> -#include <wspiapi.h>
> +#if _WIN32_WINNT < 0x0501
> +# undef _WIN32_WINNT
> +# define _WIN32_WINNT 0x0501
> +#endif
> +#include <ws2tcpip.h>
>  #else
>  #include <netinet/in.h>
>  #include <arpa/inet.h>

As I said, I don't use Windows and don't understand the system, but if
these changes fix the problem for you, I'd say they're justified and
should be pushed (even though I don't understand the "if
_WIN32_WINNT..." part).

> Note that one other side effect of the IPv6 support additions is that
> on MS-Windows GDB will no longer run on versions older than XP, I
> guess this is something that should be mentioned in NEWS?

I confess I did not know that.  If that's the case, then we should
indeed notify the users via the NEWS file, IMO.

> The other two problems were minor warning, one in tui.c about an
> unused variable 'cap' (it is not used on Windows) and another in
> xml-syscall.c:
>
>        CXX    xml-syscall.o
>      xml-syscall.c: In function 'bool xml_list_syscalls_by_group(gdbarch*, const char*, std::vector<int>*)':
>      xml-syscall.c:475:14: warning: types may not be defined in a for-range-declaration
> 	for (const struct syscall_desc *sysdesc : groupdesc->syscalls)
> 		   ^~~~~~
>
> I solved the latter by removing "struct" from the declaration.  This
> is with GCC 6.3.0; is that a GCC bug? is removing "struct" the right
> solution?

Yeah, this is the right thing to do.  I remember having to do this a few
times, and seeing other patches doing the same.

Thanks,
LRN Feb. 28, 2019, 7:06 p.m. UTC | #2 
On 28.02.2019 21:55, Sergio Durigan Junior wrote:
> On Thursday, February 28 2019, Eli Zaretskii wrote:
>> On Wed, 27 Feb 2019 09:51:12 +0400 (+04) Joel Brobecker wrote:
>>>
>>> I have just finished creating the gdb-8.2.90 pre-release.
>>> It is available for download at the following location:
>>>
>>>     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-8.2.90.tar.xz
>>
>> Thanks, I've built this with mingw.org's MinGW and bumped into a few
>> problems.
>>
>> First, the recent changes to support IPv6 caused compilation errors in
>> several files.  The problem is with several fragments such as this
>> one:
>>
>>   #ifdef USE_WIN32API
>>   #include <winsock2.h>
>>   #include <wspiapi.h>
>>   #else
>>   ...
>>
>> mingw.org's MinGW doesn't have wspiapi.h.  Moreover, the Microsoft
>> documentation indicates that to get prototypes of getaddrinfo,
>> freeaddrinfo, etc. one needs to include ws2tcpip.h (and not include
>> winsock2.h separately)
> However, and more importantly, I remember testing the whole patch by
> compiling it using a mingw32 compiler on Fedora, and it was working
> correctly.  In fact, we even have a mingw32 builder on our BuildBot
> (running on Fedora), and it is still compiling GDB without problems:
> 
>   https://gdb-build.sergiodj.net/builders/Fedora-x86_64-w64-mingw32
> 
> So apparently this error is only triggered when you use mingw on
> Windows...?  I don't know.
> 

Obviously, Fedora cross-compiler is mingw-w64, not mingw.org. Big difference,
feature-wise.
Eli Zaretskii Feb. 28, 2019, 7:44 p.m. UTC | #3 
> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Cc: Joel Brobecker <brobecker@adacore.com>,  gdb-patches@sourceware.org
> Date: Thu, 28 Feb 2019 13:55:27 -0500
> 
> However, and more importantly, I remember testing the whole patch by
> compiling it using a mingw32 compiler on Fedora, and it was working
> correctly.  In fact, we even have a mingw32 builder on our BuildBot
> (running on Fedora), and it is still compiling GDB without problems:
> 
>   https://gdb-build.sergiodj.net/builders/Fedora-x86_64-w64-mingw32
> 
> So apparently this error is only triggered when you use mingw on
> Windows...?  I don't know.

No, the problem is that there are two flavors of MinGW, and I used the
other one.

> As I said, I don't use Windows and don't understand the system, but if
> these changes fix the problem for you, I'd say they're justified and
> should be pushed (even though I don't understand the "if
> _WIN32_WINNT..." part).

For the record, the _WIN32_WINNT part is because mingw.org's MinGW by
default defines _WIN32_WINNT to target older versions of Windows,
which don't support getaddrinfo, and the Windows API headers then mask
the prototypes of those functions.

> > Note that one other side effect of the IPv6 support additions is that
> > on MS-Windows GDB will no longer run on versions older than XP, I
> > guess this is something that should be mentioned in NEWS?
> 
> I confess I did not know that.  If that's the case, then we should
> indeed notify the users via the NEWS file, IMO.

OK, will do.

> >        CXX    xml-syscall.o
> >      xml-syscall.c: In function 'bool xml_list_syscalls_by_group(gdbarch*, const char*, std::vector<int>*)':
> >      xml-syscall.c:475:14: warning: types may not be defined in a for-range-declaration
> > 	for (const struct syscall_desc *sysdesc : groupdesc->syscalls)
> > 		   ^~~~~~
> >
> > I solved the latter by removing "struct" from the declaration.  This
> > is with GCC 6.3.0; is that a GCC bug? is removing "struct" the right
> > solution?
> 
> Yeah, this is the right thing to do.  I remember having to do this a few
> times, and seeing other patches doing the same.

OK, will do that as well.

Thanks for the feedback.
Sergio Durigan Junior Feb. 28, 2019, 8:17 p.m. UTC | #4 
On Thursday, February 28 2019, Eli Zaretskii wrote:

>> From: Sergio Durigan Junior <sergiodj@redhat.com>
>> Cc: Joel Brobecker <brobecker@adacore.com>,  gdb-patches@sourceware.org
>> Date: Thu, 28 Feb 2019 13:55:27 -0500
>> 
>> However, and more importantly, I remember testing the whole patch by
>> compiling it using a mingw32 compiler on Fedora, and it was working
>> correctly.  In fact, we even have a mingw32 builder on our BuildBot
>> (running on Fedora), and it is still compiling GDB without problems:
>> 
>>   https://gdb-build.sergiodj.net/builders/Fedora-x86_64-w64-mingw32
>> 
>> So apparently this error is only triggered when you use mingw on
>> Windows...?  I don't know.
>
> No, the problem is that there are two flavors of MinGW, and I used the
> other one.

Understood.

>> As I said, I don't use Windows and don't understand the system, but if
>> these changes fix the problem for you, I'd say they're justified and
>> should be pushed (even though I don't understand the "if
>> _WIN32_WINNT..." part).
>
> For the record, the _WIN32_WINNT part is because mingw.org's MinGW by
> default defines _WIN32_WINNT to target older versions of Windows,
> which don't support getaddrinfo, and the Windows API headers then mask
> the prototypes of those functions.

Thanks for the explanation.

>> > Note that one other side effect of the IPv6 support additions is that
>> > on MS-Windows GDB will no longer run on versions older than XP, I
>> > guess this is something that should be mentioned in NEWS?
>> 
>> I confess I did not know that.  If that's the case, then we should
>> indeed notify the users via the NEWS file, IMO.
>
> OK, will do.
>
>> >        CXX    xml-syscall.o
>> >      xml-syscall.c: In function 'bool xml_list_syscalls_by_group(gdbarch*, const char*, std::vector<int>*)':
>> >      xml-syscall.c:475:14: warning: types may not be defined in a for-range-declaration
>> > 	for (const struct syscall_desc *sysdesc : groupdesc->syscalls)
>> > 		   ^~~~~~
>> >
>> > I solved the latter by removing "struct" from the declaration.  This
>> > is with GCC 6.3.0; is that a GCC bug? is removing "struct" the right
>> > solution?
>> 
>> Yeah, this is the right thing to do.  I remember having to do this a few
>> times, and seeing other patches doing the same.
>
> OK, will do that as well.
>
> Thanks for the feedback.

Thank you for taking care of it.

Just as a reminder, these changes need to be pushed to origin/master as
well.

Thanks,
Eli Zaretskii Feb. 28, 2019, 8:29 p.m. UTC | #5 
> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Cc: brobecker@adacore.com,  gdb-patches@sourceware.org
> Date: Thu, 28 Feb 2019 15:17:02 -0500
> 
> Just as a reminder, these changes need to be pushed to origin/master as
> well.

Yes, I usually push to master and then cherry-pick to the branch.  I
believe this is the procedure?
Sergio Durigan Junior Feb. 28, 2019, 8:37 p.m. UTC | #6 
On Thursday, February 28 2019, Eli Zaretskii wrote:

>> From: Sergio Durigan Junior <sergiodj@redhat.com>
>> Cc: brobecker@adacore.com,  gdb-patches@sourceware.org
>> Date: Thu, 28 Feb 2019 15:17:02 -0500
>> 
>> Just as a reminder, these changes need to be pushed to origin/master as
>> well.
>
> Yes, I usually push to master and then cherry-pick to the branch.  I
> believe this is the procedure?

Sure, that works.

Thanks,
diff mbox

Patch

--- ./gdb/common/netstuff.c~0	2019-02-27 06:51:50.000000000 +0200
+++ ./gdb/common/netstuff.c	2019-02-28 08:56:07.511568800 +0200
@@ -21,8 +21,11 @@ 
 #include <algorithm>
 
 #ifdef USE_WIN32API
-#include <winsock2.h>
-#include <wspiapi.h>
+#if _WIN32_WINNT < 0x0501
+# undef _WIN32_WINNT
+# define _WIN32_WINNT 0x0501
+#endif
+#include <ws2tcpip.h>
 #else
 #include <netinet/in.h>
 #include <arpa/inet.h>