Message ID | 835zrbe36c.fsf@gnu.org |
---|---|
State | New, archived |
Headers |
Received: (qmail 86715 invoked by alias); 18 Apr 2019 15:36:38 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 86691 invoked by uid 89); 18 Apr 2019 15:36:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: eggs.gnu.org Received: from eggs.gnu.org (HELO eggs.gnu.org) (209.51.188.92) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 18 Apr 2019 15:36:33 +0000 Received: from fencepost.gnu.org ([2001:470:142:3::e]:33863) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <eliz@gnu.org>) id 1hH95T-0007ib-Oi for gdb-patches@sourceware.org; Thu, 18 Apr 2019 11:36:31 -0400 Received: from [176.228.60.248] (port=3123 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from <eliz@gnu.org>) id 1hH95S-0002Wu-Ld for gdb-patches@sourceware.org; Thu, 18 Apr 2019 11:36:31 -0400 Date: Thu, 18 Apr 2019 18:36:11 +0300 Message-Id: <835zrbe36c.fsf@gnu.org> From: Eli Zaretskii <eliz@gnu.org> To: gdb-patches@sourceware.org Subject: Fix compilation using mingw.org's MinGW X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-IsSubscribed: yes |
Commit Message
Eli Zaretskii
April 18, 2019, 3:36 p.m. UTC
windows-nat.c uses CONSOLE_FONT_INFO unconditionally, but the corresponding declaration in the Windows API headers is conditional, because the APIs using CONSOLE_FONT_INFO are only available since Windows XP. When that condition doesn't hold, windows-nat.c won't compile: windows-nat.c:114:10: error: 'CONSOLE_FONT_INFO' has not been declared CONSOLE_FONT_INFO *); ^~~~~~~~~~~~~~~~~ windows-nat.c: In function 'void windows_set_console_info(STARTUPINFOA*, DWORD*)': windows-nat.c:2174:7: error: 'CONSOLE_FONT_INFO' was not declared in this scope CONSOLE_FONT_INFO cfi; ^~~~~~~~~~~~~~~~~ windows-nat.c:2174:7: note: suggested alternative: 'CONSOLE_CURSOR_INFO' CONSOLE_FONT_INFO cfi; ^~~~~~~~~~~~~~~~~ CONSOLE_CURSOR_INFO windows-nat.c:2176:48: error: 'cfi' was not declared in this scope GetCurrentConsoleFont (hconsole, FALSE, &cfi); ^~~ windows-nat.c: At global scope: windows-nat.c:3302:55: error: 'CONSOLE_FONT_INFO' has not been declared bad_GetCurrentConsoleFont (HANDLE w, BOOL bMaxWindow, CONSOLE_FONT_INFO *f) ^~~~~~~~~~~~~~~~~ windows-nat.c: In function 'BOOL bad_GetCurrentConsoleFont(HANDLE, BOOL, int*)': windows-nat.c:3304:6: error: request for member 'nFont' in '* f', which is of non-class type 'int' f->nFont = 0; ^~~~~ MinGW64 no longer supports versions of Windows older than XP, but mingw.org's MinGW does. Since we load GetCurrentConsoleFont dynamically at run time, and have code for when the load fails, it makes sense to not assume XP at compile time. The patch below does that. OK to push (with the pertinent ChangeLog entry)?
Comments
On Thu, 18 Apr 2019 18:36:11 +0300 Eli Zaretskii <eliz@gnu.org> wrote: > --- gdb/windows-nat.c~0 2019-03-27 00:52:05.000000000 +0200 > +++ gdb/windows-nat.c 2019-04-18 11:37:02.061945600 +0300 > @@ -81,6 +81,16 @@ > #define GetConsoleFontSize dyn_GetConsoleFontSize > #define GetCurrentConsoleFont dyn_GetCurrentConsoleFont > > +#if _WIN32_WINNT < 0x0501 > + > +typedef > +struct _CONSOLE_FONT_INFO > +{ DWORD nFont; > + COORD dwFontSize; > +} CONSOLE_FONT_INFO, *PCONSOLE_FONT_INFO; > + > +#endif > + > typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL, > PTOKEN_PRIVILEGES, > DWORD, PTOKEN_PRIVILEGES, I had expected to see a typedef for struct CONSOLE_FONT_INFO, but instead see one for _CONSOLE_FONT_INFO (where the difference is the leading underscore.) I'm guessing that there's some magic in Windows specific header files which makes this work, but I just want to check to make sure that this wasn't a typo on your part... Kevin
On 4/18/19 4:36 PM, Eli Zaretskii wrote: > MinGW64 no longer supports versions of Windows older than XP, but > mingw.org's MinGW does. Since we load GetCurrentConsoleFont > dynamically at run time, and have code for when the load fails, it > makes sense to not assume XP at compile time. The patch below does > that. OK to push (with the pertinent ChangeLog entry)? Didn't we drop support for < XP some time ago? Thanks, Pedro Alves
On 4/18/19 6:20 PM, Pedro Alves wrote: > On 4/18/19 4:36 PM, Eli Zaretskii wrote: > >> MinGW64 no longer supports versions of Windows older than XP, but >> mingw.org's MinGW does. Since we load GetCurrentConsoleFont >> dynamically at run time, and have code for when the load fails, it >> makes sense to not assume XP at compile time. The patch below does >> that. OK to push (with the pertinent ChangeLog entry)? > > Didn't we drop support for < XP some time ago? > From gdb/NEWS: ~~~~~~~~~~~~ * Removed targets GDB no longer supports native debugging on versions of MS-Windows before Windows XP. ~~~~~~~~~~~~ So shouldn't we instead be setting _WIN32_WINNT to some appropriate number? Thanks, Pedro Alves
> Date: Thu, 18 Apr 2019 10:00:58 -0700 > From: Kevin Buettner <kevinb@redhat.com> > Cc: gdb-patches@sourceware.org > > > +#if _WIN32_WINNT < 0x0501 > > + > > +typedef > > +struct _CONSOLE_FONT_INFO > > +{ DWORD nFont; > > + COORD dwFontSize; > > +} CONSOLE_FONT_INFO, *PCONSOLE_FONT_INFO; > > + > > +#endif > > + > > typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL, > > PTOKEN_PRIVILEGES, > > DWORD, PTOKEN_PRIVILEGES, > > I had expected to see a typedef for struct CONSOLE_FONT_INFO, but > instead see one for _CONSOLE_FONT_INFO (where the difference is > the leading underscore.) The struct is with the leading underscore, the typedef isn't. > I'm guessing that there's some magic in Windows specific header files > which makes this work, but I just want to check to make sure that this > wasn't a typo on your part... It wasn't a typo, and there is no magic ;-)
> From: Pedro Alves <palves@redhat.com> > Date: Thu, 18 Apr 2019 18:22:14 +0100 > > * Removed targets > > GDB no longer supports native debugging on versions of MS-Windows > before Windows XP. I was talking about compile time, not run time. In any case, if we don't support systems older than XP, then why do we load those functions dynamically at run time and call them via a function pointer? > So shouldn't we instead be setting _WIN32_WINNT to some > appropriate number? I don't mind, but where? And also: should we make such changes on the 8.3 branch at this time?
On Thu, 18 Apr 2019 21:54:05 +0300 Eli Zaretskii <eliz@gnu.org> wrote: > > Date: Thu, 18 Apr 2019 10:00:58 -0700 > > From: Kevin Buettner <kevinb@redhat.com> > > Cc: gdb-patches@sourceware.org > > > > > +#if _WIN32_WINNT < 0x0501 > > > + > > > +typedef > > > +struct _CONSOLE_FONT_INFO > > > +{ DWORD nFont; > > > + COORD dwFontSize; > > > +} CONSOLE_FONT_INFO, *PCONSOLE_FONT_INFO; > > > + > > > +#endif > > > + > > > typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL, > > > PTOKEN_PRIVILEGES, > > > DWORD, PTOKEN_PRIVILEGES, > > > > I had expected to see a typedef for struct CONSOLE_FONT_INFO, but > > instead see one for _CONSOLE_FONT_INFO (where the difference is > > the leading underscore.) > > The struct is with the leading underscore, the typedef isn't. > > > I'm guessing that there's some magic in Windows specific header files > > which makes this work, but I just want to check to make sure that this > > wasn't a typo on your part... > > It wasn't a typo, and there is no magic ;-) Apologies - I clearly wasn't awake enough when I looked at your patch. So... no objections from me regarding this patch. Kevin
> Date: Thu, 18 Apr 2019 13:38:46 -0700 > From: Kevin Buettner <kevinb@redhat.com> > Cc: gdb-patches@sourceware.org > > > It wasn't a typo, and there is no magic ;-) > > Apologies - I clearly wasn't awake enough when I looked at your patch. No sweat. Thanks for reviewing the patch.
On 4/18/19 7:56 PM, Eli Zaretskii wrote: >> From: Pedro Alves <palves@redhat.com> >> Date: Thu, 18 Apr 2019 18:22:14 +0100 >> >> * Removed targets >> >> GDB no longer supports native debugging on versions of MS-Windows >> before Windows XP. > > I was talking about compile time, not run time. Not sure it makes any practical difference. From the text above, you could claim that we wanted to support hosting GDB on older versions of Windows, but not support native debugging, say, so you could still remote debug from such older Windows boxes. But was that the intention? I'd say keep it simple -- if we don't support native debugging, then we don't support building at all either. The subject for the commit that added that NEWS entry sound like that was the intention: commit 742a7df5f4a149f0818aaccfc432c4c0c9a6e26d Author: Eli Zaretskii <eliz@gnu.org> AuthorDate: Sat Mar 2 15:18:32 2019 +0200 GDB no longer supports Windows before XP. But surprisingly, I can't find the discussion behind this commit in the archives to see the context and what was decided. > > In any case, if we don't support systems older than XP, then why do we > load those functions dynamically at run time and call them via a > function pointer? Because that code is older than the decision to stop supporting older Windows versions, surely? : ChangeLog-2010: * windows-nat.c (GetConsoleFontSize, GetCurrentConsoleFont): ChangeLog-2010: (bad_GetCurrentConsoleFont, bad_GetConsoleFontSize): New functions. ChangeLog-2010: GetCurrentConsoleFont. ChangeLog-2015: (GetCurrentConsoleFont_ftype, GetModuleInformation_ftype) > >> So shouldn't we instead be setting _WIN32_WINNT to some >> appropriate number? > > I don't mind, but where? I'd do it in common/common-defs.h, before any #include, where we define other macros that must be defined before any include, like __STDC_LIMIT_MACROS, _FORTIFY_SOURCE, etc. > And also: should we make such changes on the > 8.3 branch at this time? Not sure. Off hand I'd think it's pretty safe, but maybe for 8.3 your patch is safer. Thanks, Pedro Alves
> Cc: gdb-patches@sourceware.org > From: Pedro Alves <palves@redhat.com> > Date: Fri, 19 Apr 2019 11:51:46 +0100 > > building at all either. The subject for the commit that added that NEWS entry > sound like that was the intention: > > commit 742a7df5f4a149f0818aaccfc432c4c0c9a6e26d > Author: Eli Zaretskii <eliz@gnu.org> > AuthorDate: Sat Mar 2 15:18:32 2019 +0200 > > GDB no longer supports Windows before XP. > > But surprisingly, I can't find the discussion behind this commit > in the archives to see the context and what was decided. The discussion which led to that change is here: https://www.sourceware.org/ml/gdb-patches/2019-02/msg00574.html > >> So shouldn't we instead be setting _WIN32_WINNT to some > >> appropriate number? > > > > I don't mind, but where? > > I'd do it in common/common-defs.h, before any #include, where we define > other macros that must be defined before any include, like > __STDC_LIMIT_MACROS, _FORTIFY_SOURCE, etc. > > > And also: should we make such changes on the > > 8.3 branch at this time? > > Not sure. Off hand I'd think it's pretty safe, but maybe for 8.3 > your patch is safer. I will try doing this in common-defs.h, but for the branch, we could set _WIN32_WINNT only in windows-nat.c, as that's the only file that currently cares, which should be safer. WDYT?
On 4/19/19 12:23 PM, Eli Zaretskii wrote: >> Cc: gdb-patches@sourceware.org >> From: Pedro Alves <palves@redhat.com> >> Date: Fri, 19 Apr 2019 11:51:46 +0100 >> >> building at all either. The subject for the commit that added that NEWS entry >> sound like that was the intention: >> >> commit 742a7df5f4a149f0818aaccfc432c4c0c9a6e26d >> Author: Eli Zaretskii <eliz@gnu.org> >> AuthorDate: Sat Mar 2 15:18:32 2019 +0200 >> >> GDB no longer supports Windows before XP. >> >> But surprisingly, I can't find the discussion behind this commit >> in the archives to see the context and what was decided. > > The discussion which led to that change is here: > > https://www.sourceware.org/ml/gdb-patches/2019-02/msg00574.html > So we're already assuming XP at build time in common code, in common/netstuff.c. >>>> So shouldn't we instead be setting _WIN32_WINNT to some >>>> appropriate number? >>> >>> I don't mind, but where? >> >> I'd do it in common/common-defs.h, before any #include, where we define >> other macros that must be defined before any include, like >> __STDC_LIMIT_MACROS, _FORTIFY_SOURCE, etc. >> >>> And also: should we make such changes on the >>> 8.3 branch at this time? >> >> Not sure. Off hand I'd think it's pretty safe, but maybe for 8.3 >> your patch is safer. > > I will try doing this in common-defs.h, but for the branch, we could > set _WIN32_WINNT only in windows-nat.c, as that's the only file that > currently cares, which should be safer. WDYT? Sounds good to me. Thanks, Pedro Alves
--- gdb/windows-nat.c~0 2019-03-27 00:52:05.000000000 +0200 +++ gdb/windows-nat.c 2019-04-18 11:37:02.061945600 +0300 @@ -81,6 +81,16 @@ #define GetConsoleFontSize dyn_GetConsoleFontSize #define GetCurrentConsoleFont dyn_GetCurrentConsoleFont +#if _WIN32_WINNT < 0x0501 + +typedef +struct _CONSOLE_FONT_INFO +{ DWORD nFont; + COORD dwFontSize; +} CONSOLE_FONT_INFO, *PCONSOLE_FONT_INFO; + +#endif + typedef BOOL WINAPI (AdjustTokenPrivileges_ftype) (HANDLE, BOOL, PTOKEN_PRIVILEGES, DWORD, PTOKEN_PRIVILEGES,