public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
@ 2024-03-06 13:42 FX Coudert
  2024-03-06 13:54 ` Sam James
  0 siblings, 1 reply; 12+ messages in thread
From: FX Coudert @ 2024-03-06 13:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: Iain Sandoe, dimitry

I would like to patch this patch from September 2023:
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631611.html

This bug is now hitting macOS in the latest version of Xcode (it was originally seen on freebsd).
I confirm that the patch is restoring bootstrap on x86_64-apple-darwin23

OK to push?
FX





> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
> 
> When building gcc's C++ sources against recent libc++, the poisoning of
> the ctype macros due to including safe-ctype.h before including C++
> standard headers such as <list>, <map>, etc, causes many compilation
> errors, similar to:
> 
> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
> In file included from /usr/include/c++/v1/vector:321:
> In file included from
> /usr/include/c++/v1/__format/formatter_bool.h:20:
> In file included from
> /usr/include/c++/v1/__format/formatter_integral.h:32:
> In file included from /usr/include/c++/v1/locale:202:
> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
> only applies to structs, variables, functions, and namespaces
> 546 | _LIBCPP_INLINE_VISIBILITY
> | ^
> /usr/include/c++/v1/__config:813:37: note: expanded from macro
> '_LIBCPP_INLINE_VISIBILITY'
> 813 | # define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
> | ^
> /usr/include/c++/v1/__config:792:26: note: expanded from macro
> '_LIBCPP_HIDE_FROM_ABI'
> 792 |
> __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
> _LIBCPP_VERSIONED_IDENTIFIER))))
> | ^
> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
> In file included from /usr/include/c++/v1/vector:321:
> In file included from
> /usr/include/c++/v1/__format/formatter_bool.h:20:
> In file included from
> /usr/include/c++/v1/__format/formatter_integral.h:32:
> In file included from /usr/include/c++/v1/locale:202:
> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
> declaration list
> 547 | char_type toupper(char_type __c) const
> | ^
> /usr/include/c++/v1/__locale:553:48: error: too many arguments
> provided to function-like macro invocation
> 553 | const char_type* toupper(char_type* __low, const
> char_type* __high) const
> | ^
> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
> macro 'toupper' defined here
> 146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
> | ^
> 
> This is because libc++ uses different transitive includes than
> libstdc++, and some of those transitive includes pull in various ctype
> declarations (typically via <locale>).
> 
> There was already a special case for including <string> before
> safe-ctype.h, so move the rest of the C++ standard header includes to
> the same location, to fix the problem.
> 

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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-03-06 13:42 [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning FX Coudert
@ 2024-03-06 13:54 ` Sam James
  2024-03-06 14:35   ` Iain Sandoe
  0 siblings, 1 reply; 12+ messages in thread
From: Sam James @ 2024-03-06 13:54 UTC (permalink / raw)
  To: FX Coudert; +Cc: gcc-patches, Iain Sandoe, dimitry

[-- Attachment #1: Type: text/plain, Size: 3109 bytes --]

FX Coudert <fxcoudert@gmail.com> writes:

> I would like to patch this patch from September 2023:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631611.html
>
> This bug is now hitting macOS in the latest version of Xcode (it was originally seen on freebsd).
> I confirm that the patch is restoring bootstrap on x86_64-apple-darwin23

Iain hit an issue with it and I never got a chance to look into how to
fix it.

>
> OK to push?
> FX
>
>
>
>
>
>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>> 
>> When building gcc's C++ sources against recent libc++, the poisoning of
>> the ctype macros due to including safe-ctype.h before including C++
>> standard headers such as <list>, <map>, etc, causes many compilation
>> errors, similar to:
>> 
>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>> In file included from /usr/include/c++/v1/vector:321:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>> In file included from /usr/include/c++/v1/locale:202:
>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>> only applies to structs, variables, functions, and namespaces
>> 546 | _LIBCPP_INLINE_VISIBILITY
>> | ^
>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>> '_LIBCPP_INLINE_VISIBILITY'
>> 813 | # define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>> | ^
>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>> '_LIBCPP_HIDE_FROM_ABI'
>> 792 |
>> __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>> _LIBCPP_VERSIONED_IDENTIFIER))))
>> | ^
>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>> In file included from /usr/include/c++/v1/vector:321:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>> In file included from /usr/include/c++/v1/locale:202:
>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>> declaration list
>> 547 | char_type toupper(char_type __c) const
>> | ^
>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>> provided to function-like macro invocation
>> 553 | const char_type* toupper(char_type* __low, const
>> char_type* __high) const
>> | ^
>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>> macro 'toupper' defined here
>> 146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>> | ^
>> 
>> This is because libc++ uses different transitive includes than
>> libstdc++, and some of those transitive includes pull in various ctype
>> declarations (typically via <locale>).
>> 
>> There was already a special case for including <string> before
>> safe-ctype.h, so move the rest of the C++ standard header includes to
>> the same location, to fix the problem.
>> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-03-06 13:54 ` Sam James
@ 2024-03-06 14:35   ` Iain Sandoe
  2024-03-06 14:57     ` FX Coudert
  0 siblings, 1 reply; 12+ messages in thread
From: Iain Sandoe @ 2024-03-06 14:35 UTC (permalink / raw)
  To: Sam James; +Cc: FX Coudert, GCC Patches, dimitry



> On 6 Mar 2024, at 13:54, Sam James <sam@gentoo.org> wrote:
> 
> FX Coudert <fxcoudert@gmail.com> writes:
> 
>> I would like to patch this patch from September 2023:
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631611.html
>> 
>> This bug is now hitting macOS in the latest version of Xcode (it was originally seen on freebsd).
>> I confirm that the patch is restoring bootstrap on x86_64-apple-darwin23
> 
> Iain hit an issue with it and I never got a chance to look into how to
> fix it.

Hmm I recall trying it and finding a problem - was there some different fix applied
in the end?

Iain

> 
>> 
>> OK to push?
>> FX
>> 
>> 
>> 
>> 
>> 
>>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>>> 
>>> When building gcc's C++ sources against recent libc++, the poisoning of
>>> the ctype macros due to including safe-ctype.h before including C++
>>> standard headers such as <list>, <map>, etc, causes many compilation
>>> errors, similar to:
>>> 
>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>> In file included from /usr/include/c++/v1/vector:321:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>> In file included from /usr/include/c++/v1/locale:202:
>>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>>> only applies to structs, variables, functions, and namespaces
>>> 546 | _LIBCPP_INLINE_VISIBILITY
>>> | ^
>>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>>> '_LIBCPP_INLINE_VISIBILITY'
>>> 813 | # define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>> | ^
>>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>>> '_LIBCPP_HIDE_FROM_ABI'
>>> 792 |
>>> __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>>> _LIBCPP_VERSIONED_IDENTIFIER))))
>>> | ^
>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>> In file included from /usr/include/c++/v1/vector:321:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>> In file included from /usr/include/c++/v1/locale:202:
>>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>>> declaration list
>>> 547 | char_type toupper(char_type __c) const
>>> | ^
>>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>>> provided to function-like macro invocation
>>> 553 | const char_type* toupper(char_type* __low, const
>>> char_type* __high) const
>>> | ^
>>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>>> macro 'toupper' defined here
>>> 146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>> | ^
>>> 
>>> This is because libc++ uses different transitive includes than
>>> libstdc++, and some of those transitive includes pull in various ctype
>>> declarations (typically via <locale>).
>>> 
>>> There was already a special case for including <string> before
>>> safe-ctype.h, so move the rest of the C++ standard header includes to
>>> the same location, to fix the problem.
>>> 


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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-03-06 14:35   ` Iain Sandoe
@ 2024-03-06 14:57     ` FX Coudert
  2024-03-06 23:02       ` Dimitry Andric
  0 siblings, 1 reply; 12+ messages in thread
From: FX Coudert @ 2024-03-06 14:57 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: Sam James, GCC Patches, dimitry

> Hmm I recall trying it and finding a problem - was there some different fix applied
> in the end?

The bug is still open, I don’t think a patch was applied, and I don’t find any email to the list stating what the problem could be.

FX

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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-03-06 14:57     ` FX Coudert
@ 2024-03-06 23:02       ` Dimitry Andric
  2024-03-07  7:11         ` Iain Sandoe
  0 siblings, 1 reply; 12+ messages in thread
From: Dimitry Andric @ 2024-03-06 23:02 UTC (permalink / raw)
  To: FX Coudert; +Cc: Iain Sandoe, Sam James, GCC Patches

On 6 Mar 2024, at 15:57, FX Coudert <fxcoudert@gmail.com> wrote:
> 
>> Hmm I recall trying it and finding a problem - was there some different fix applied
>> in the end?
> 
> The bug is still open, I don’t think a patch was applied, and I don’t find any email to the list stating what the problem could be.

The original patch (https://gcc.gnu.org/bugzilla/attachment.cgi?id=56010) still applies to the master branch. It turned out there is also a related problem in libcc1plugin.cc and libcp1plugin.cc <http://libcp1plugin.cc/>, which is fixed by https://gcc.gnu.org/bugzilla/attachment.cgi?id=57639 :

commit 49222b98ac8e30a4a042ada0ece3d7df93f049d2
Author: Dimitry Andric <dimitry@andric.com>
Date:   2024-03-06T23:46:27+01:00

    Fix libcc1plugin and libc1plugin to use INCLUDE_VECTOR before including
    system.h, instead of directly including <vector>, to avoid running into
    poisoned identifiers.

diff --git a/libcc1/libcc1plugin.cc b/libcc1/libcc1plugin.cc
index 72d17c3b81c..e64847466f4 100644
--- a/libcc1/libcc1plugin.cc
+++ b/libcc1/libcc1plugin.cc
@@ -32,6 +32,7 @@
 #undef PACKAGE_VERSION
 
 #define INCLUDE_MEMORY
+#define INCLUDE_VECTOR
 #include "gcc-plugin.h"
 #include "system.h"
 #include "coretypes.h"
@@ -69,8 +70,6 @@
 #include "gcc-c-interface.h"
 #include "context.hh"
 
-#include <vector>
-
 using namespace cc1_plugin;
 
 \f
diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
index 0eff7c68d29..da68c5d0ac1 100644
--- a/libcc1/libcp1plugin.cc
+++ b/libcc1/libcp1plugin.cc
@@ -33,6 +33,7 @@
 #undef PACKAGE_VERSION
 
 #define INCLUDE_MEMORY
+#define INCLUDE_VECTOR
 #include "gcc-plugin.h"
 #include "system.h"
 #include "coretypes.h"
@@ -71,8 +72,6 @@
 #include "rpc.hh"
 #include "context.hh"
 
-#include <vector>
-
 using namespace cc1_plugin;
 
 \f


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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-03-06 23:02       ` Dimitry Andric
@ 2024-03-07  7:11         ` Iain Sandoe
  2024-03-07  9:51           ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Iain Sandoe @ 2024-03-07  7:11 UTC (permalink / raw)
  To: Dimitry Andric; +Cc: FX Coudert, Sam James, Richard Biener, GCC Patches

Hi Dimitry, folks,

> On 6 Mar 2024, at 23:02, Dimitry Andric <dimitry@andric.com> wrote:
> 
> On 6 Mar 2024, at 15:57, FX Coudert <fxcoudert@gmail.com> wrote:
>> 
>>> Hmm I recall trying it and finding a problem - was there some different fix applied
>>> in the end?
>> 
>> The bug is still open, I don’t think a patch was applied, and I don’t find any email to the list stating what the problem could be.
> 
> The original patch (https://gcc.gnu.org/bugzilla/attachment.cgi?id=56010) still applies to the master branch.

I have retested this on various Darwin versions and confirm that it fixes the bootstrap fail on x86_64-darwin23 and works OK on other versions (including 32b hosts). 

+1 for applying this soon.

====

the second patch really needs to be posted separately to make review easier (if we were not in stage 4, I’d say it’s ‘obvious’ anyway).

thanks
Iain


> It turned out there is also a related problem in libcc1plugin.cc and libcp1plugin.cc <http://libcp1plugin.cc/>, which is fixed by https://gcc.gnu.org/bugzilla/attachment.cgi?id=57639 :
> 
> commit 49222b98ac8e30a4a042ada0ece3d7df93f049d2
> Author: Dimitry Andric <dimitry@andric.com>
> Date:   2024-03-06T23:46:27+01:00
> 
>    Fix libcc1plugin and libc1plugin to use INCLUDE_VECTOR before including
>    system.h, instead of directly including <vector>, to avoid running into
>    poisoned identifiers.
> 
> diff --git a/libcc1/libcc1plugin.cc b/libcc1/libcc1plugin.cc
> index 72d17c3b81c..e64847466f4 100644
> --- a/libcc1/libcc1plugin.cc
> +++ b/libcc1/libcc1plugin.cc
> @@ -32,6 +32,7 @@
> #undef PACKAGE_VERSION
> 
> #define INCLUDE_MEMORY
> +#define INCLUDE_VECTOR
> #include "gcc-plugin.h"
> #include "system.h"
> #include "coretypes.h"
> @@ -69,8 +70,6 @@
> #include "gcc-c-interface.h"
> #include "context.hh"
> 
> -#include <vector>
> -
> using namespace cc1_plugin;
> 
> 
> diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
> index 0eff7c68d29..da68c5d0ac1 100644
> --- a/libcc1/libcp1plugin.cc
> +++ b/libcc1/libcp1plugin.cc
> @@ -33,6 +33,7 @@
> #undef PACKAGE_VERSION
> 
> #define INCLUDE_MEMORY
> +#define INCLUDE_VECTOR
> #include "gcc-plugin.h"
> #include "system.h"
> #include "coretypes.h"
> @@ -71,8 +72,6 @@
> #include "rpc.hh"
> #include "context.hh"
> 
> -#include <vector>
> -
> using namespace cc1_plugin;
> 
> 
> 


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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-03-07  7:11         ` Iain Sandoe
@ 2024-03-07  9:51           ` Richard Biener
  2024-03-07 13:40             ` FX Coudert
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2024-03-07  9:51 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: Dimitry Andric, FX Coudert, Sam James, GCC Patches

On Thu, 7 Mar 2024, Iain Sandoe wrote:

> Hi Dimitry, folks,
> 
> > On 6 Mar 2024, at 23:02, Dimitry Andric <dimitry@andric.com> wrote:
> > 
> > On 6 Mar 2024, at 15:57, FX Coudert <fxcoudert@gmail.com> wrote:
> >> 
> >>> Hmm I recall trying it and finding a problem - was there some different fix applied
> >>> in the end?
> >> 
> >> The bug is still open, I don?t think a patch was applied, and I don?t find any email to the list stating what the problem could be.
> > 
> > The original patch (https://gcc.gnu.org/bugzilla/attachment.cgi?id=56010) still applies to the master branch.
> 
> I have retested this on various Darwin versions and confirm that it fixes the bootstrap fail on x86_64-darwin23 and works OK on other versions (including 32b hosts). 
> 
> +1 for applying this soon.

I think it's an obvious change ...

Richard.

> ====
> 
> the second patch really needs to be posted separately to make review easier (if we were not in stage 4, I?d say it?s ?obvious? anyway).
> 
> thanks
> Iain
> 
> 
> > It turned out there is also a related problem in libcc1plugin.cc and libcp1plugin.cc <http://libcp1plugin.cc/>, which is fixed by https://gcc.gnu.org/bugzilla/attachment.cgi?id=57639 :
> > 
> > commit 49222b98ac8e30a4a042ada0ece3d7df93f049d2
> > Author: Dimitry Andric <dimitry@andric.com>
> > Date:   2024-03-06T23:46:27+01:00
> > 
> >    Fix libcc1plugin and libc1plugin to use INCLUDE_VECTOR before including
> >    system.h, instead of directly including <vector>, to avoid running into
> >    poisoned identifiers.
> > 
> > diff --git a/libcc1/libcc1plugin.cc b/libcc1/libcc1plugin.cc
> > index 72d17c3b81c..e64847466f4 100644
> > --- a/libcc1/libcc1plugin.cc
> > +++ b/libcc1/libcc1plugin.cc
> > @@ -32,6 +32,7 @@
> > #undef PACKAGE_VERSION
> > 
> > #define INCLUDE_MEMORY
> > +#define INCLUDE_VECTOR
> > #include "gcc-plugin.h"
> > #include "system.h"
> > #include "coretypes.h"
> > @@ -69,8 +70,6 @@
> > #include "gcc-c-interface.h"
> > #include "context.hh"
> > 
> > -#include <vector>
> > -
> > using namespace cc1_plugin;
> > 
> > 
> > diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
> > index 0eff7c68d29..da68c5d0ac1 100644
> > --- a/libcc1/libcp1plugin.cc
> > +++ b/libcc1/libcp1plugin.cc
> > @@ -33,6 +33,7 @@
> > #undef PACKAGE_VERSION
> > 
> > #define INCLUDE_MEMORY
> > +#define INCLUDE_VECTOR
> > #include "gcc-plugin.h"
> > #include "system.h"
> > #include "coretypes.h"
> > @@ -71,8 +72,6 @@
> > #include "rpc.hh"
> > #include "context.hh"
> > 
> > -#include <vector>
> > -
> > using namespace cc1_plugin;
> > 
> > 
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-03-07  9:51           ` Richard Biener
@ 2024-03-07 13:40             ` FX Coudert
  2024-04-03  9:01               ` Iain Sandoe
  0 siblings, 1 reply; 12+ messages in thread
From: FX Coudert @ 2024-03-07 13:40 UTC (permalink / raw)
  To: Richard Biener; +Cc: Iain Sandoe, Dimitry Andric, Sam James, GCC Patches

> I think it's an obvious change ...

Thanks, pushed.

Dimitry, I suggest you post the second patch for review.

FX

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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-03-07 13:40             ` FX Coudert
@ 2024-04-03  9:01               ` Iain Sandoe
  2024-04-03  9:38                 ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Iain Sandoe @ 2024-04-03  9:01 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches, Gerald Pfeifer

Hi Richard,

> On 7 Mar 2024, at 13:40, FX Coudert <fxcoudert@gmail.com> wrote:
> 
>> I think it's an obvious change ...
> 
> Thanks, pushed.
> 
> Dimitry, I suggest you post the second patch for review.

Given that the two patches here (for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632) were considered obvious - and are needed on release branches.

OK for backporting?

(Gerald has volunteered to do the earlier ones, I have already made/tested the gcc-13 case)

thanks
Iain


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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-04-03  9:01               ` Iain Sandoe
@ 2024-04-03  9:38                 ` Richard Biener
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Biener @ 2024-04-03  9:38 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches, Gerald Pfeifer

On Wed, 3 Apr 2024, Iain Sandoe wrote:

> Hi Richard,
> 
> > On 7 Mar 2024, at 13:40, FX Coudert <fxcoudert@gmail.com> wrote:
> > 
> >> I think it's an obvious change ...
> > 
> > Thanks, pushed.
> > 
> > Dimitry, I suggest you post the second patch for review.
> 
> Given that the two patches here (for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632) were considered obvious - and are needed on release branches.
> 
> OK for backporting?

OK.

> (Gerald has volunteered to do the earlier ones, I have already made/tested the gcc-13 case)
> 
> thanks
> Iain
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

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

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2023-09-28 16:37 Dimitry Andric
@ 2024-01-30 16:19 ` Jonathan Wakely
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Wakely @ 2024-01-30 16:19 UTC (permalink / raw)
  To: Dimitry Andric; +Cc: gcc-patches, Gerald Pfeifer

On 28/09/23 18:37 +0200, Dimitry Andric wrote:
>Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>
>When building gcc's C++ sources against recent libc++, the poisoning of
>the ctype macros due to including safe-ctype.h before including C++
>standard headers such as <list>, <map>, etc, causes many compilation
>errors, similar to:
>
> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
> In file included from /usr/include/c++/v1/vector:321:
> In file included from
> /usr/include/c++/v1/__format/formatter_bool.h:20:
> In file included from
> /usr/include/c++/v1/__format/formatter_integral.h:32:
> In file included from /usr/include/c++/v1/locale:202:
> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
> only applies to structs, variables, functions, and namespaces
>   546 |     _LIBCPP_INLINE_VISIBILITY
>       |     ^
> /usr/include/c++/v1/__config:813:37: note: expanded from macro
> '_LIBCPP_INLINE_VISIBILITY'
>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>       |                                     ^
> /usr/include/c++/v1/__config:792:26: note: expanded from macro
> '_LIBCPP_HIDE_FROM_ABI'
>   792 |
>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
> _LIBCPP_VERSIONED_IDENTIFIER))))
>       |                          ^
> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
> In file included from /usr/include/c++/v1/vector:321:
> In file included from
> /usr/include/c++/v1/__format/formatter_bool.h:20:
> In file included from
> /usr/include/c++/v1/__format/formatter_integral.h:32:
> In file included from /usr/include/c++/v1/locale:202:
> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
> declaration list
>   547 |     char_type toupper(char_type __c) const
>       |                                     ^
> /usr/include/c++/v1/__locale:553:48: error: too many arguments
> provided to function-like macro invocation
>   553 |     const char_type* toupper(char_type* __low, const
>   char_type* __high) const
>       |                                                ^
> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
> macro 'toupper' defined here
>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>       |         ^
>
>This is because libc++ uses different transitive includes than
>libstdc++, and some of those transitive includes pull in various ctype
>declarations (typically via <locale>).
>
>There was already a special case for including <string> before
>safe-ctype.h, so move the rest of the C++ standard header includes to
>the same location, to fix the problem.

I can't approve this either, but FWIW it looks good to me.


>Signed-off-by: Dimitry Andric <dimitry@andric.com>
>---
>gcc/system.h | 39 ++++++++++++++++++---------------------
>1 file changed, 18 insertions(+), 21 deletions(-)
>
>diff --git a/gcc/system.h b/gcc/system.h
>index e924152ad4c..7a516b11438 100644
>--- a/gcc/system.h
>+++ b/gcc/system.h
>@@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
>#undef fread_unlocked
>#undef fwrite_unlocked
>
>-/* Include <string> before "safe-ctype.h" to avoid GCC poisoning
>-   the ctype macros through safe-ctype.h */
>-
>-#ifdef __cplusplus
>-#ifdef INCLUDE_STRING
>-# include <string>
>-#endif
>-#endif
>-
>-/* There are an extraordinary number of issues with <ctype.h>.
>-   The last straw is that it varies with the locale.  Use libiberty's
>-   replacement instead.  */
>-#include "safe-ctype.h"
>-
>-#include <sys/types.h>
>-
>-#include <errno.h>
>-
>-#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>-extern int errno;
>-#endif
>+/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
>+   poisoning the ctype macros through safe-ctype.h */
>
>#ifdef __cplusplus
>#if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
>@@ -229,6 +210,9 @@ extern int errno;
>#ifdef INCLUDE_SET
># include <set>
>#endif
>+#ifdef INCLUDE_STRING
>+# include <string>
>+#endif
>#ifdef INCLUDE_VECTOR
># include <vector>
>#endif
>@@ -245,6 +229,19 @@ extern int errno;
># include <type_traits>
>#endif
>
>+/* There are an extraordinary number of issues with <ctype.h>.
>+   The last straw is that it varies with the locale.  Use libiberty's
>+   replacement instead.  */
>+#include "safe-ctype.h"
>+
>+#include <sys/types.h>
>+
>+#include <errno.h>
>+
>+#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>+extern int errno;
>+#endif
>+
>/* Some of glibc's string inlines cause warnings.  Plus we'd rather
>   rely on (and therefore test) GCC's string builtins.  */
>#define __NO_STRING_INLINES


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

* [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
@ 2023-09-28 16:37 Dimitry Andric
  2024-01-30 16:19 ` Jonathan Wakely
  0 siblings, 1 reply; 12+ messages in thread
From: Dimitry Andric @ 2023-09-28 16:37 UTC (permalink / raw)
  To: gcc-patches; +Cc: Gerald Pfeifer

Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632

When building gcc's C++ sources against recent libc++, the poisoning of
the ctype macros due to including safe-ctype.h before including C++
standard headers such as <list>, <map>, etc, causes many compilation
errors, similar to:

 In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
 In file included from /home/dim/src/gcc/master/gcc/system.h:233:
 In file included from /usr/include/c++/v1/vector:321:
 In file included from
 /usr/include/c++/v1/__format/formatter_bool.h:20:
 In file included from
 /usr/include/c++/v1/__format/formatter_integral.h:32:
 In file included from /usr/include/c++/v1/locale:202:
 /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
 only applies to structs, variables, functions, and namespaces
   546 |     _LIBCPP_INLINE_VISIBILITY
       |     ^
 /usr/include/c++/v1/__config:813:37: note: expanded from macro
 '_LIBCPP_INLINE_VISIBILITY'
   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
       |                                     ^
 /usr/include/c++/v1/__config:792:26: note: expanded from macro
 '_LIBCPP_HIDE_FROM_ABI'
   792 |
   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
 _LIBCPP_VERSIONED_IDENTIFIER))))
       |                          ^
 In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
 In file included from /home/dim/src/gcc/master/gcc/system.h:233:
 In file included from /usr/include/c++/v1/vector:321:
 In file included from
 /usr/include/c++/v1/__format/formatter_bool.h:20:
 In file included from
 /usr/include/c++/v1/__format/formatter_integral.h:32:
 In file included from /usr/include/c++/v1/locale:202:
 /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
 declaration list
   547 |     char_type toupper(char_type __c) const
       |                                     ^
 /usr/include/c++/v1/__locale:553:48: error: too many arguments
 provided to function-like macro invocation
   553 |     const char_type* toupper(char_type* __low, const
   char_type* __high) const
       |                                                ^
 /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
 macro 'toupper' defined here
   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
       |         ^

This is because libc++ uses different transitive includes than
libstdc++, and some of those transitive includes pull in various ctype
declarations (typically via <locale>).

There was already a special case for including <string> before
safe-ctype.h, so move the rest of the C++ standard header includes to
the same location, to fix the problem.

Signed-off-by: Dimitry Andric <dimitry@andric.com>
---
gcc/system.h | 39 ++++++++++++++++++---------------------
1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/gcc/system.h b/gcc/system.h
index e924152ad4c..7a516b11438 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
#undef fread_unlocked
#undef fwrite_unlocked

-/* Include <string> before "safe-ctype.h" to avoid GCC poisoning
-   the ctype macros through safe-ctype.h */
-
-#ifdef __cplusplus
-#ifdef INCLUDE_STRING
-# include <string>
-#endif
-#endif
-
-/* There are an extraordinary number of issues with <ctype.h>.
-   The last straw is that it varies with the locale.  Use libiberty's
-   replacement instead.  */
-#include "safe-ctype.h"
-
-#include <sys/types.h>
-
-#include <errno.h>
-
-#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
-extern int errno;
-#endif
+/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
+   poisoning the ctype macros through safe-ctype.h */

#ifdef __cplusplus
#if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
@@ -229,6 +210,9 @@ extern int errno;
#ifdef INCLUDE_SET
# include <set>
#endif
+#ifdef INCLUDE_STRING
+# include <string>
+#endif
#ifdef INCLUDE_VECTOR
# include <vector>
#endif
@@ -245,6 +229,19 @@ extern int errno;
# include <type_traits>
#endif

+/* There are an extraordinary number of issues with <ctype.h>.
+   The last straw is that it varies with the locale.  Use libiberty's
+   replacement instead.  */
+#include "safe-ctype.h"
+
+#include <sys/types.h>
+
+#include <errno.h>
+
+#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
+extern int errno;
+#endif
+
/* Some of glibc's string inlines cause warnings.  Plus we'd rather
   rely on (and therefore test) GCC's string builtins.  */
#define __NO_STRING_INLINES
-- 
2.42.0


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

end of thread, other threads:[~2024-04-03  9:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-06 13:42 [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning FX Coudert
2024-03-06 13:54 ` Sam James
2024-03-06 14:35   ` Iain Sandoe
2024-03-06 14:57     ` FX Coudert
2024-03-06 23:02       ` Dimitry Andric
2024-03-07  7:11         ` Iain Sandoe
2024-03-07  9:51           ` Richard Biener
2024-03-07 13:40             ` FX Coudert
2024-04-03  9:01               ` Iain Sandoe
2024-04-03  9:38                 ` Richard Biener
  -- strict thread matches above, loose matches on Subject: below --
2023-09-28 16:37 Dimitry Andric
2024-01-30 16:19 ` Jonathan Wakely

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).