public inbox for gdb@sourceware.org
 help / color / mirror / Atom feed
* _WIN32_WINNT redefined?
@ 2022-11-02  8:14 i.nixman
  2022-11-02  8:21 ` i.nixman
  0 siblings, 1 reply; 17+ messages in thread
From: i.nixman @ 2022-11-02  8:14 UTC (permalink / raw)
  To: gdb


hello!

host: x86_64-w64-mingw32
target: x86_64-w64-mingw32

I'm trying to build GDB using GCC that I built myself.
the GCC was patched and configured the way when std-threads implemented 
by libgcc using WINAPI directly, without winpthreads.
the compiler works, as far as I can test it.

but I have faced with a trouble with building GDB using the compiler.
the trouble looks like this:
```
In file included from 
mingw64/lib/gcc/x86_64-w64-mingw32/13.0.0/include/c++/mutex:45,
                  from 
../src/gdb-11.2/gdbsupport/../gdbsupport/thread-pool.h:27,
                  from ../src/gdb-11.2/gdbsupport/thread-pool.cc:24:
mingw64/lib/gcc/x86_64-w64-mingw32/13.0.0/include/c++/bits/std_mutex.h:163:5: 
error: '__gthread_cond_t' does not name a type; did you mean 
'__gthread_once_t'?
   163 |     __gthread_cond_t* native_handle() noexcept { return 
&_M_cond; }
       |     ^~~~~~~~~~~~~~~~
```

the problem is a consequence of the fact that `_WIN32_WINNT` is 
redefined somewhere to another value that is less than necessary.
but when I pass `-D_WIN32_WINNT=0x0601` for CFLAGS/CXXFLAGS - the 
trouble is gone.

the mingw-w64-api was configured with 
`--with-default-win32-winnt=0x0601` and I can confirm the `_WIN32_WINNT` 
is actually set to the correct value.
moreover, using the compiler I can successfully build a simple program 
that uses `std::thread` without the need to pass `-D_WIN32_WINNT=0x0601` 
for CFLAGS/CXXFLAGS.


so my question is: could it be so the `_WIN32_WINNT` macro is reassigned 
somewhere in GDB sources?




best!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02  8:14 _WIN32_WINNT redefined? i.nixman
@ 2022-11-02  8:21 ` i.nixman
  2022-11-02  8:44   ` i.nixman
  0 siblings, 1 reply; 17+ messages in thread
From: i.nixman @ 2022-11-02  8:21 UTC (permalink / raw)
  To: gdb

On 2022-11-02 08:14, niXman via Gdb wrote:


looks like it can be defined here:
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/common-defs.h;h=e4985332e3f4016ccec2b2502dfe28bab16e2c92;hb=HEAD#l81


as 0x0500 if it wasn't defined before...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02  8:21 ` i.nixman
@ 2022-11-02  8:44   ` i.nixman
  2022-11-02  9:46     ` i.nixman
  2022-11-02 12:55     ` Eli Zaretskii
  0 siblings, 2 replies; 17+ messages in thread
From: i.nixman @ 2022-11-02  8:44 UTC (permalink / raw)
  To: gdb

On 2022-11-02 08:21, niXman via Gdb wrote:
> On 2022-11-02 08:14, niXman via Gdb wrote:
> 
> 
> looks like it can be defined here:
> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/common-defs.h;h=e4985332e3f4016ccec2b2502dfe28bab16e2c92;hb=HEAD#l81
> 
> 
> as 0x0500 if it wasn't defined before...


right, because inclusion of windows.h here 
(https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/common-defs.h;h=e4985332e3f4016ccec2b2502dfe28bab16e2c92;hb=HEAD#l74) 
after line 74 solves the trouble.


but it introduced another trouble:
```
mkdir -p -- nat/.deps
   CXX    gdb.o
   CXX    ada-exp.o
ada-exp.c.tmp:557: warning: "IN" redefined
In file included from 
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/windef.h:9,
                  from 
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/windows.h:69,
                  from 
../../../../src/gdb-11.2/gdb/../gdbsupport/common-defs.h:75,
                  from ../../../../src/gdb-11.2/gdb/defs.h:28,
                  from ada-exp.y:38:
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/minwindef.h:57: 
note: this is the location of the previous definition
    57 | #define IN
       |
ada-exp.c.tmp:482:11: error: 'INT' redeclared as different kind of 
entity
In file included from 
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/minwindef.h:163:
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/winnt.h:299:15: 
note: previous declaration 'typedef int INT'
   299 |   typedef int INT;
       |               ^~~
ada-exp.c.tmp:485:13: error: 'FLOAT' redeclared as different kind of 
entity
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/minwindef.h:142:17: 
note: previous declaration 'typedef float FLOAT'
   142 |   typedef float FLOAT;
       |                 ^~~~~
```

=)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02  8:44   ` i.nixman
@ 2022-11-02  9:46     ` i.nixman
  2022-11-02 10:15       ` i.nixman
  2022-11-02 12:50       ` Eli Zaretskii
  2022-11-02 12:55     ` Eli Zaretskii
  1 sibling, 2 replies; 17+ messages in thread
From: i.nixman @ 2022-11-02  9:46 UTC (permalink / raw)
  To: gdb

On 2022-11-02 08:44, niXman via Gdb wrote:
> On 2022-11-02 08:21, niXman via Gdb wrote:

ok, I did a little research.

the `_WIN32_WINNT` is defined in `gdbsupport/common-defs.h` only, but 
referenced in `gnulib/import/gettimeofdeay.c` and 
`gnulib/import/stat-w32.c` only, but not directly because those files 
include `windows,h` and `sdkdkver.h`. thus that `_WIN32_WINNT` define is 
never used!

so I think the best way is to remove that define completely.


ideas?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02  9:46     ` i.nixman
@ 2022-11-02 10:15       ` i.nixman
  2022-11-02 12:59         ` Eli Zaretskii
  2022-11-02 12:50       ` Eli Zaretskii
  1 sibling, 1 reply; 17+ messages in thread
From: i.nixman @ 2022-11-02 10:15 UTC (permalink / raw)
  To: gdb

On 2022-11-02 09:46, niXman via Gdb wrote:
> On 2022-11-02 08:44, niXman via Gdb wrote:
>> On 2022-11-02 08:21, niXman via Gdb wrote:
> 
> ok, I did a little research.
> 
> the `_WIN32_WINNT` is defined in `gdbsupport/common-defs.h` only, but
> referenced in `gnulib/import/gettimeofdeay.c` and
> `gnulib/import/stat-w32.c` only, but not directly because those files
> include `windows,h` and `sdkdkver.h`. thus that `_WIN32_WINNT` define
> is never used!
> 
> so I think the best way is to remove that define completely.
> 
> 
> ideas?


right, with commented out the entire PP block the build was successful!



best!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02  9:46     ` i.nixman
  2022-11-02 10:15       ` i.nixman
@ 2022-11-02 12:50       ` Eli Zaretskii
  2022-11-02 14:13         ` i.nixman
  1 sibling, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2022-11-02 12:50 UTC (permalink / raw)
  To: i.nixman; +Cc: gdb

> Date: Wed, 02 Nov 2022 09:46:20 +0000
> From: niXman via Gdb <gdb@sourceware.org>
> 
> On 2022-11-02 08:44, niXman via Gdb wrote:
> > On 2022-11-02 08:21, niXman via Gdb wrote:
> 
> ok, I did a little research.
> 
> the `_WIN32_WINNT` is defined in `gdbsupport/common-defs.h` only, but 
> referenced in `gnulib/import/gettimeofdeay.c` and 
> `gnulib/import/stat-w32.c` only, but not directly because those files 
> include `windows,h` and `sdkdkver.h`. thus that `_WIN32_WINNT` define is 
> never used!

No, _WIN32_WINNT is referenced in a lot of w32api header files, which
use it to decide whether to expose function prototypes, data type
declarations, macros, etc.  In effect, the value of _WIN32_WINNT
determines the oldest version of MS-Windows which the program needs to
support, by hiding APIs that are not available on later versions --
this ensures the program doesn't use those later APIs unintentionally.

> so I think the best way is to remove that define completely.

No, that's wrong, because if you remove that, you will basically force
everyone who builds GDB for Windows to use the default value of
_WIN32_WINNT defined by the w32api headers.  And that default is not
necessarily what we want.

> ideas?

Find out why the current definition causes trouble in your build, then
we could discuss the correct solution that works for everyone.

In the original report you said:

> In file included from 
> mingw64/lib/gcc/x86_64-w64-mingw32/13.0.0/include/c++/mutex:45,
>                   from 
> ../src/gdb-11.2/gdbsupport/../gdbsupport/thread-pool.h:27,
>                   from ../src/gdb-11.2/gdbsupport/thread-pool.cc:24:
> mingw64/lib/gcc/x86_64-w64-mingw32/13.0.0/include/c++/bits/std_mutex.h:163:5: 
> error: '__gthread_cond_t' does not name a type; did you mean 
> '__gthread_once_t'?
>    163 |     __gthread_cond_t* native_handle() noexcept { return 
> &_M_cond; }
>        |     ^~~~~~~~~~~~~~~~
> ```
> 
> the problem is a consequence of the fact that `_WIN32_WINNT` is 
> redefined somewhere to another value that is less than necessary.
> but when I pass `-D_WIN32_WINNT=0x0601` for CFLAGS/CXXFLAGS - the 
> trouble is gone.

This doesn't explain why -D_WIN32_WINNT=0x0601 solves the problem.
Can you explain that in enough detail, and point to the relevant parts
of the headers?

In addition, you point to this place in common-defs.h:

  /* We don't support Windows versions before XP, so we define
     _WIN32_WINNT correspondingly to ensure the Windows API headers
     expose the required symbols.  */
  #if defined (__MINGW32__) || defined (__CYGWIN__)
  # ifdef _WIN32_WINNT
  #  if _WIN32_WINNT < 0x0501
  #   undef _WIN32_WINNT
  #   define _WIN32_WINNT 0x0501
  #  endif
  # else
  #  define _WIN32_WINNT 0x0501
  # endif
  #endif	/* __MINGW32__ || __CYGWIN__ */

This defines _WIN32_WINNT only if it wasn't already defined, or if it
was defined to a value smaller than 0x0501.  So why isn't _WIN32_WINNT
defined in your w32api headers?  It should be defined to some default
value, which was determined when the w32api headers were produced, and
I'm guessing that this default value should be 0x0601, in which case
the above preprocessor stuff should not redefine it.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02  8:44   ` i.nixman
  2022-11-02  9:46     ` i.nixman
@ 2022-11-02 12:55     ` Eli Zaretskii
  1 sibling, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2022-11-02 12:55 UTC (permalink / raw)
  To: i.nixman; +Cc: gdb

> Date: Wed, 02 Nov 2022 08:44:09 +0000
> From: niXman via Gdb <gdb@sourceware.org>
> 
> > looks like it can be defined here:
> > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/common-defs.h;h=e4985332e3f4016ccec2b2502dfe28bab16e2c92;hb=HEAD#l81
> > 
> > 
> > as 0x0500 if it wasn't defined before...
> 
> 
> right, because inclusion of windows.h here 
> (https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/common-defs.h;h=e4985332e3f4016ccec2b2502dfe28bab16e2c92;hb=HEAD#l74) 
> after line 74 solves the trouble.

Do you mean that the w32api headers you use _override_ any external
definition of _WIN32_WINNT?  They shouldn't: this macro exists to
control what w32api headers expose and what they don't, so including
windows.h should not change its value.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 10:15       ` i.nixman
@ 2022-11-02 12:59         ` Eli Zaretskii
  2022-11-02 13:10           ` Jeffrey Walton
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2022-11-02 12:59 UTC (permalink / raw)
  To: i.nixman; +Cc: gdb

> Date: Wed, 02 Nov 2022 10:15:01 +0000
> X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00, DKIM_SIGNED,
>  DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_PASS, SPF_PASS,
>  TXREP autolearn=ham autolearn_force=no version=3.4.6
> From: niXman via Gdb <gdb@sourceware.org>
> 
> On 2022-11-02 09:46, niXman via Gdb wrote:
> > On 2022-11-02 08:44, niXman via Gdb wrote:
> >> On 2022-11-02 08:21, niXman via Gdb wrote:
> > 
> > ok, I did a little research.
> > 
> > the `_WIN32_WINNT` is defined in `gdbsupport/common-defs.h` only, but
> > referenced in `gnulib/import/gettimeofdeay.c` and
> > `gnulib/import/stat-w32.c` only, but not directly because those files
> > include `windows,h` and `sdkdkver.h`. thus that `_WIN32_WINNT` define
> > is never used!
> > 
> > so I think the best way is to remove that define completely.
> > 
> > 
> > ideas?
> 
> 
> right, with commented out the entire PP block the build was successful!

This is the wrong solution, IMO.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 12:59         ` Eli Zaretskii
@ 2022-11-02 13:10           ` Jeffrey Walton
  2022-11-02 13:26             ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Jeffrey Walton @ 2022-11-02 13:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: i.nixman, gdb

On Wed, Nov 2, 2022 at 9:02 AM Eli Zaretskii via Gdb <gdb@sourceware.org> wrote:
>
> > Date: Wed, 02 Nov 2022 10:15:01 +0000
> > X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00, DKIM_SIGNED,
> >  DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_PASS, SPF_PASS,
> >  TXREP autolearn=ham autolearn_force=no version=3.4.6
> > From: niXman via Gdb <gdb@sourceware.org>
> >
> > On 2022-11-02 09:46, niXman via Gdb wrote:
> > > On 2022-11-02 08:44, niXman via Gdb wrote:
> > >> On 2022-11-02 08:21, niXman via Gdb wrote:
> > >
> > > ok, I did a little research.
> > >
> > > the `_WIN32_WINNT` is defined in `gdbsupport/common-defs.h` only, but
> > > referenced in `gnulib/import/gettimeofdeay.c` and
> > > `gnulib/import/stat-w32.c` only, but not directly because those files
> > > include `windows,h` and `sdkdkver.h`. thus that `_WIN32_WINNT` define
> > > is never used!
> > >
> > > so I think the best way is to remove that define completely.
> > >
> > >
> > > ideas?
> >
> > right, with commented out the entire PP block the build was successful!
>
> This is the wrong solution, IMO.

Here's what Microsoft has to say about it:
https://learn.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt
"

    To modify the macros, in a header file (for example, in targetver.h,
    which is included by some project templates that target Windows),
    add the following lines.

    #define WINVER 0x0A00
    #define _WIN32_WINNT 0x0A00

If I am reading that correctly, there should be a common header file
which defines WINVER and _WIN32_WINNT. In my old MFC days, we would
set it in a file like <stdafx.h> . In a non-MFC project, we would set
it under the Visual Studio preprocessor macros, which is just
CPPFLAGS.

Maybe there needs to be a configure option to set the values.
Configure would then create the header file with the values. If the
configure option is not supplied, then use a sane default value and
create the header file.

Jeff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 13:10           ` Jeffrey Walton
@ 2022-11-02 13:26             ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2022-11-02 13:26 UTC (permalink / raw)
  To: noloader; +Cc: i.nixman, gdb

> From: Jeffrey Walton <noloader@gmail.com>
> Date: Wed, 2 Nov 2022 09:10:21 -0400
> Cc: i.nixman@autistici.org, gdb@sourceware.org
> 
> > > right, with commented out the entire PP block the build was successful!
> >
> > This is the wrong solution, IMO.
> 
> Here's what Microsoft has to say about it:
> https://learn.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt
> "
> 
>     To modify the macros, in a header file (for example, in targetver.h,
>     which is included by some project templates that target Windows),
>     add the following lines.
> 
>     #define WINVER 0x0A00
>     #define _WIN32_WINNT 0x0A00
> 
> If I am reading that correctly, there should be a common header file
> which defines WINVER and _WIN32_WINNT. In my old MFC days, we would
> set it in a file like <stdafx.h> . In a non-MFC project, we would set
> it under the Visual Studio preprocessor macros, which is just
> CPPFLAGS.

I believe we use common-defs.h as that common header.

> Maybe there needs to be a configure option to set the values.

What for?  I see no reason to expose this complexity to people who
configure GDB.

The underlying problem here is that one of the two flavors of MinGW
has its headers (and _WIN32_WINNT in particular) set for Widows 9X,
and GDB no longer supports those old versions.  So we force the w32api
headers of that MinGW flavor to expose the parts that are supported on
Windows XP and later.  If the w32api headers are set for a higher
value of _WIN32_WINNT (as the other MinGW flavor already does), then
the offending part of common-defs.h should be a no-op.

Why this didn't work for the OP is unclear.  Until it is clear, IMO we
are not ready to have a rational discussion of the solutions for the
issue.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 12:50       ` Eli Zaretskii
@ 2022-11-02 14:13         ` i.nixman
  2022-11-02 15:18           ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: i.nixman @ 2022-11-02 14:13 UTC (permalink / raw)
  To: Eli Zaretskii, gdb

On 2022-11-02 12:50, Eli Zaretskii wrote:

Hello Eli!

I will try to explain step by step.


the root of the issue: GDB wan't build using MinGW-W64 toolchain uses 
this patch: 
https://gcc.gnu.org/pipermail/libstdc++/2022-October/054895.html
(for short, the patch provides the ability to libstdc++ to enable the 
support for std-threads and so on but without the need to use 
libwinpthreads: 
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-libraries/winpthreads/)

next, as I wrote earlier, I have faced with a trouble I can't build GDB 
using that path:
```
In file included from 
mingw64/lib/gcc/x86_64-w64-mingw32/13.0.0/include/c++/mutex:45,
                  from 
../src/gdb-11.2/gdbsupport/../gdbsupport/thread-pool.h:27,
                  from ../src/gdb-11.2/gdbsupport/thread-pool.cc:24:
mingw64/lib/gcc/x86_64-w64-mingw32/13.0.0/include/c++/bits/std_mutex.h:163:5: 
error: '__gthread_cond_t' does not name a type; did you mean 
'__gthread_once_t'?
   163 |     __gthread_cond_t* native_handle() noexcept { return 
&_M_cond; }
       |     ^~~~~~~~~~~~~~~~
```

because the patch requires the _WIN32_WINNT will be set to 0x0600 or 
greater.

next, GDB's building process trying to build `gdbsupport/thread-pool.cc` 
which includes `common-defs.h` first.
then, `gdbsupport/thread-pool.cc` includes `gdbsupport/thread-pool.h` 
which includes other standard headers like 
condition_variable/mutex/thread, etc.
at this point the inclusion chain looks like this: `mutex`[1] -> 
`bits/std_mutex.h`[2] -> `bits/gthr.h`[3] -> `gthr-default.h`[4]

[1] 
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/std/mutex;h=b310c15687d323c1750f717e630fbe0666515139;hb=HEAD
[2] 
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/bits/std_mutex.h;h=b22e0e12793700291c65c1695313dbeae4561da2;hb=HEAD
[3] 
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libgcc/gthr.h;h=7943c94f6d16b7aed4269aa08db746ae131430b1;hb=HEAD
(note for this link: in reality that file located inside gcc sources as 
`libgcc/gthr.h`, but on target toolchain it is `bits/gthr.h` without any 
changes)
[4] https://pastebin.com/LbE0qbwu
(note for this link: I paste that file on pastebin because I want to 
show you how looks that file after applying the patch)

now look at the 4 link at line 65: `#if _WIN32_WINNT >= 0x0600`
that is, in this case the definition from within the GDB sources 
"infects" the GCC sources. do you agree that this should not be the 
case?

well, back to the root.
please look at `gdbsupport/common-defs.h`: 
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/common-defs.h;h=e4985332e3f4016ccec2b2502dfe28bab16e2c92;hb=HEAD

you may not believe me, but none of the inclusions above that 
preprocessor block I'm talking about include neither `windows.h` nor 
`winver.h` nothing that could tell to that preprocessor block the value 
of `_WIN32_WINNT` :)

thus, it turns out that that preprocessor block of code does not check 
and does not correct the value of `_WIN32_WINNT` but simply always 
imposes its value.


as solution I see two ways:
1) to remove that PP block completely, because as I wrote earlier - no 
one in the GDB sources refers to it.
2) to hide that PP block so that it cannot "infect" other code by by 
getting into public headers.





best!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 14:13         ` i.nixman
@ 2022-11-02 15:18           ` Eli Zaretskii
  2022-11-02 15:51             ` i.nixman
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2022-11-02 15:18 UTC (permalink / raw)
  To: i.nixman; +Cc: gdb

> Date: Wed, 02 Nov 2022 14:13:13 +0000
> From: i.nixman@autistici.org
> 
> the root of the issue: GDB wan't build using MinGW-W64 toolchain uses 
> this patch: 
> https://gcc.gnu.org/pipermail/libstdc++/2022-October/054895.html
> (for short, the patch provides the ability to libstdc++ to enable the 
> support for std-threads and so on but without the need to use 
> libwinpthreads: 
> https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-libraries/winpthreads/)
> 
> next, as I wrote earlier, I have faced with a trouble I can't build GDB 
> using that path:
> ```
> In file included from 
> mingw64/lib/gcc/x86_64-w64-mingw32/13.0.0/include/c++/mutex:45,
>                   from 
> ../src/gdb-11.2/gdbsupport/../gdbsupport/thread-pool.h:27,
>                   from ../src/gdb-11.2/gdbsupport/thread-pool.cc:24:
> mingw64/lib/gcc/x86_64-w64-mingw32/13.0.0/include/c++/bits/std_mutex.h:163:5: 
> error: '__gthread_cond_t' does not name a type; did you mean 
> '__gthread_once_t'?
>    163 |     __gthread_cond_t* native_handle() noexcept { return 
> &_M_cond; }
>        |     ^~~~~~~~~~~~~~~~
> ```
> 
> because the patch requires the _WIN32_WINNT will be set to 0x0600 or 
> greater.

Then the stuff in common-defs.h should be augmented to define
_WIN32_WINNT to the value 0x0600 or greater, if it isn't already high
enough, but only if the patch for gthreads is being used.  How exactly
to write the cpp conditional for that, I don't know, but hopefully you
will be able to figure that out.

Or maybe you should do that in gdbsupport/thread-pool.cc instead.
Whatever is easier.

One thing is certain: GDB builds on Windows that don't use the gthread
patch should not be broken by increasing the minimum value of
_WIN32_WINNT with which GDB can be built on Windows.

> as solution I see two ways:
> 1) to remove that PP block completely, because as I wrote earlier - no 
> one in the GDB sources refers to it.
> 2) to hide that PP block so that it cannot "infect" other code by by 
> getting into public headers.

I think the right solution is:

3) amend the cpp block to define _WIN32_WINNT to 0x0600 if the threads
patch is being used.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 15:18           ` Eli Zaretskii
@ 2022-11-02 15:51             ` i.nixman
  2022-11-02 16:02               ` i.nixman
  2022-11-02 16:51               ` Eli Zaretskii
  0 siblings, 2 replies; 17+ messages in thread
From: i.nixman @ 2022-11-02 15:51 UTC (permalink / raw)
  To: Eli Zaretskii, gdb

On 2022-11-02 15:18, Eli Zaretskii wrote:

> Then the stuff in common-defs.h should be augmented to define
> _WIN32_WINNT to the value 0x0600 or greater, if it isn't already high
> enough, but only if the patch for gthreads is being used.  How exactly
> to write the cpp conditional for that, I don't know, but hopefully you
> will be able to figure that out.

no.
just for eyes:
74 #if defined (__MINGW32__) || defined (__CYGWIN__)
75 # ifdef _WIN32_WINNT
76 #  if _WIN32_WINNT < 0x0501
77 #   undef _WIN32_WINNT
78 #   define _WIN32_WINNT 0x0501
79 #  endif
80 # else
81 #  define _WIN32_WINNT 0x0501
82 # endif
83 #endif  /* __MINGW32__ || __CYGWIN__ */

the condition on line 75 is always false, because none of the inclusions 
above include neither `windows.h` nor `winver.h`.
it can be solved by inclusion `windows.h` after line 74.
but I did it and faced with another error:
```
   CXX    gdb.o
   CXX    ada-exp.o
ada-exp.c.tmp:557: warning: "IN" redefined
In file included from 
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/windef.h:9,
                  from 
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/windows.h:69,
                  from 
../../../../src/gdb-11.2/gdb/../gdbsupport/common-defs.h:75,
                  from ../../../../src/gdb-11.2/gdb/defs.h:28,
                  from ada-exp.y:38:
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/minwindef.h:57: 
note: this is the location of the previous definition
    57 | #define IN
       |
ada-exp.c.tmp:482:11: error: 'INT' redeclared as different kind of 
entity
In file included from 
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/minwindef.h:163:
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/winnt.h:299:15: 
note: previous declaration 'typedef int INT'
   299 |   typedef int INT;
       |               ^~~
ada-exp.c.tmp:485:13: error: 'FLOAT' redeclared as different kind of 
entity
C:/msys64/home/Sysuser/mingw-gcc-trunk/x86_64-trunk-win32-seh-rt_v10-rev0/mingw64/x86_64-w64-mingw32/include/minwindef.h:142:17: 
note: previous declaration 'typedef float FLOAT'
   142 |   typedef float FLOAT;
       |                 ^~~~~
```

in that case I couldn't figure out why the `ada-exp.c.tmp` file is 
mentioned in the error message but I don't see anything that looks like 
an error in the `ada-exp.c` file.
I think `ada-exp.c.tmp` is some kind of generated file... I don't know 
what and how should I fix in that case %)


> Or maybe you should do that in gdbsupport/thread-pool.cc instead.
> Whatever is easier.

I will think on it...



> One thing is certain: GDB builds on Windows that don't use the gthread
> patch should not be broken by increasing the minimum value of
> _WIN32_WINNT with which GDB can be built on Windows.

I understand.



I have another option. will try and report back.





best!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 15:51             ` i.nixman
@ 2022-11-02 16:02               ` i.nixman
  2022-11-02 16:51               ` Eli Zaretskii
  1 sibling, 0 replies; 17+ messages in thread
From: i.nixman @ 2022-11-02 16:02 UTC (permalink / raw)
  To: Eli Zaretskii, gdb

On 2022-11-02 15:51, niXman via Gdb wrote:

> I have another option. will try and report back.

by the way.
for now I solved the issue by adding `-D_WIN32_WINNT=0x0601` to the 
CFLAGS/CXXFLAGS.
this once again confirms my opinion that the comparison in line 75 is 
always false.




best!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 15:51             ` i.nixman
  2022-11-02 16:02               ` i.nixman
@ 2022-11-02 16:51               ` Eli Zaretskii
  2022-11-02 17:20                 ` i.nixman
  1 sibling, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2022-11-02 16:51 UTC (permalink / raw)
  To: i.nixman; +Cc: gdb

> Date: Wed, 02 Nov 2022 15:51:05 +0000
> From: i.nixman@autistici.org
> 
> On 2022-11-02 15:18, Eli Zaretskii wrote:
> 
> > Then the stuff in common-defs.h should be augmented to define
> > _WIN32_WINNT to the value 0x0600 or greater, if it isn't already high
> > enough, but only if the patch for gthreads is being used.  How exactly
> > to write the cpp conditional for that, I don't know, but hopefully you
> > will be able to figure that out.
> 
> no.
> just for eyes:
> 74 #if defined (__MINGW32__) || defined (__CYGWIN__)
> 75 # ifdef _WIN32_WINNT
> 76 #  if _WIN32_WINNT < 0x0501
> 77 #   undef _WIN32_WINNT
> 78 #   define _WIN32_WINNT 0x0501
> 79 #  endif
> 80 # else
> 81 #  define _WIN32_WINNT 0x0501
> 82 # endif
> 83 #endif  /* __MINGW32__ || __CYGWIN__ */
> 
> the condition on line 75 is always false, because none of the inclusions 
> above include neither `windows.h` nor `winver.h`.

That's on your system, with your MinGW w32api headers.  But that's not
the only game in town.

> it can be solved by inclusion `windows.h` after line 74.
> but I did it and faced with another error:

That's not what I suggested.  I suggested that you augment the above
cpp conditionals such that when the gthread patch is in use,
_WIN32_WINNT is defined to 0x0600, and otherwise to 0x0501, as before.

The question is: what could be a preprocessor conditional to determine
whether the gthread patch is used, so that it could be used to augment
the above.  This is something I cannot answer, but I hope you can.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 16:51               ` Eli Zaretskii
@ 2022-11-02 17:20                 ` i.nixman
  2022-11-02 18:13                   ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: i.nixman @ 2022-11-02 17:20 UTC (permalink / raw)
  To: Eli Zaretskii, gdb

On 2022-11-02 16:51, Eli Zaretskii wrote:

> That's on your system, with your MinGW w32api headers.  But that's not
> the only game in town.

I'm sure there is no WINAPI implementation that does not define 
_WIN32_WINNT. it's just impossible.
but in line 75 the condition is exactly for that impossible case.



> That's not what I suggested.  I suggested that you augment the above
> cpp conditionals such that when the gthread patch is in use,
> _WIN32_WINNT is defined to 0x0600, and otherwise to 0x0501, as before.
> 
> The question is: what could be a preprocessor conditional to determine
> whether the gthread patch is used, so that it could be used to augment
> the above.  This is something I cannot answer, but I hope you can.

I solved my problem.
What I'm doing now is trying to tell you that it's illogical to check 
the value of _WIN32_WINNT BEFORE the corresponding header file in which 
this value was originally defined, has been included.





best!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: _WIN32_WINNT redefined?
  2022-11-02 17:20                 ` i.nixman
@ 2022-11-02 18:13                   ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2022-11-02 18:13 UTC (permalink / raw)
  To: i.nixman; +Cc: gdb

> Date: Wed, 02 Nov 2022 17:20:50 +0000
> From: i.nixman@autistici.org
> 
> On 2022-11-02 16:51, Eli Zaretskii wrote:
> 
> > That's on your system, with your MinGW w32api headers.  But that's not
> > the only game in town.
> 
> I'm sure there is no WINAPI implementation that does not define 
> _WIN32_WINNT. it's just impossible.
> but in line 75 the condition is exactly for that impossible case.

It is not impossible.

I use mingw.org's MinGW, where the header which defines _WIN32_WINNT
is included by every header file.  So as soon as you #include
anything, _WIN32_WINNT is already defined.

Maybe it isn't so in the headers you are using, but that doesn't mean
it is so for everyone.

And I don't see how is that relevant: even if _WIN32_WINNT is
undefined before that place, my suggestion is to change this:

 # else
 #  define _WIN32_WINNT 0x0501

into something that defines _WIN32_WINNT to 0x0600 under the condition
that the gthread patch is being used, and to 0x0501 otherwise.  This
should solve everyone's problem, right?

> What I'm doing now is trying to tell you that it's illogical to check 
> the value of _WIN32_WINNT BEFORE the corresponding header file in which 
> this value was originally defined, has been included.

See above: that is not necessarily what happens, not in all the cases
anyway.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-11-02 18:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-02  8:14 _WIN32_WINNT redefined? i.nixman
2022-11-02  8:21 ` i.nixman
2022-11-02  8:44   ` i.nixman
2022-11-02  9:46     ` i.nixman
2022-11-02 10:15       ` i.nixman
2022-11-02 12:59         ` Eli Zaretskii
2022-11-02 13:10           ` Jeffrey Walton
2022-11-02 13:26             ` Eli Zaretskii
2022-11-02 12:50       ` Eli Zaretskii
2022-11-02 14:13         ` i.nixman
2022-11-02 15:18           ` Eli Zaretskii
2022-11-02 15:51             ` i.nixman
2022-11-02 16:02               ` i.nixman
2022-11-02 16:51               ` Eli Zaretskii
2022-11-02 17:20                 ` i.nixman
2022-11-02 18:13                   ` Eli Zaretskii
2022-11-02 12:55     ` Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).