[PATCH] [vxworks] make wint_t and wchar_t the same distinct type

[PATCH] [vxworks] make wint_t and wchar_t the same distinct type

[Alexandre Oliva]

We used to define WINT_TYPE to WCHAR_TYPE, so that both wint_t and
wchar_t mapped to the same underlying type, but this caused a glitch
in Wstringop-overflow-6.C: on vxworks, wint_t is typedef'ed to
wchar_t, headers got included in the test that declared functions that
take wint_t parameters, and those conflicted with the builtin
declarations that had wint_t mapped to the underlying integral type.

The problem is that, in C++, wchar_t is a distinct type.  Having
wint_t be a typedef to wchar_t in the headers, but a typedef to
wchar_t's underlying integral type in builtins, makes for mismatches
between the declarations.

This patch defines WINT_TYPE to "wchar_t" for vxworks, and adjusts the
fallout, namely:

- since wchar_t may not have been defined yet when
  c_common_nodes_and_builtins runs, use the node already reserved for
  wchar_t for wint_t when WINT_TYPE is defined to wchar_t.

- for the same reason, when WINT_TYPE is wchar_t and we're not
  compiling C++ where wchar_t is a compiler built-in, define
  __WINT_TYPE__ to WCHAR_TYPE rather than WINT_TYPE, because wchar_t
  may not even be defined in the translation unit.

- recognize and handle wchar_type_node when type_suffix is called for
  wint_type_node.

Regstrapped on x86_64-linux-gnu.
Tested on arm-vxworks7 (gcc-12) and arm-eabi (trunk).  Ok to install?

for  gcc/ChangeLog

	* config/vx-common.h (WINT_TYPE): Alias to "wchar_t".

for  gcc/c-family/ChangeLog

	* c-common.cc (c_common_nodes_and_builtins): Take
	wchar_type_node for wint_type_node when aliased.
	(c_stddef_cpp_builtins): Define __WINT_TYPE__, when aliased to
	wchar_t, to the underlying type rather than wchar_t in
	non-C++.
	* c-cppbuiltin.cc (type_suffix): Handle wchar_type_node.
---
 gcc/c-family/c-common.cc     |   16 +++++++++++++---
 gcc/c-family/c-cppbuiltin.cc |    2 ++
 gcc/config/vx-common.h       |    2 +-
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index ae92cd5adaf5e..a92597c2f544f 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -4576,8 +4576,11 @@ c_common_nodes_and_builtins (void)
   char32_array_type_node
     = build_array_type (char32_type_node, array_domain_type);
 
-  wint_type_node =
-    TREE_TYPE (identifier_global_value (get_identifier (WINT_TYPE)));
+  if (strcmp (WINT_TYPE, "wchar_t") == 0)
+    wint_type_node = wchar_type_node;
+  else
+    wint_type_node =
+      TREE_TYPE (identifier_global_value (get_identifier (WINT_TYPE)));
 
   intmax_type_node =
     TREE_TYPE (identifier_global_value (get_identifier (INTMAX_TYPE)));
@@ -5359,7 +5362,14 @@ c_stddef_cpp_builtins(void)
   builtin_define_with_value ("__SIZE_TYPE__", SIZE_TYPE, 0);
   builtin_define_with_value ("__PTRDIFF_TYPE__", PTRDIFF_TYPE, 0);
   builtin_define_with_value ("__WCHAR_TYPE__", MODIFIED_WCHAR_TYPE, 0);
-  builtin_define_with_value ("__WINT_TYPE__", WINT_TYPE, 0);
+  /* C++ has wchar_t as a builtin type, C doesn't, so if WINT_TYPE
+     maps to wchar_t, define it to the underlying WCHAR_TYPE in C, and
+     to wchar_t in C++, so the desired type equivalence holds.  */
+  if (!c_dialect_cxx ()
+      && strcmp (WINT_TYPE, "wchar_t") == 0)
+    builtin_define_with_value ("__WINT_TYPE__", WCHAR_TYPE, 0);
+  else
+    builtin_define_with_value ("__WINT_TYPE__", WINT_TYPE, 0);
   builtin_define_with_value ("__INTMAX_TYPE__", INTMAX_TYPE, 0);
   builtin_define_with_value ("__UINTMAX_TYPE__", UINTMAX_TYPE, 0);
   if (flag_char8_t)
diff --git a/gcc/c-family/c-cppbuiltin.cc b/gcc/c-family/c-cppbuiltin.cc
index b333f97fd3237..98f5aef2af95d 100644
--- a/gcc/c-family/c-cppbuiltin.cc
+++ b/gcc/c-family/c-cppbuiltin.cc
@@ -1903,6 +1903,8 @@ type_suffix (tree type)
 	      systems use it anyway.  */
 	   || type == char_type_node)
     is_long = 0;
+  else if (type == wchar_type_node)
+    return type_suffix (underlying_wchar_type_node);
   else
     gcc_unreachable ();
 
diff --git a/gcc/config/vx-common.h b/gcc/config/vx-common.h
index 83580d0dec288..9733c90fe4c6f 100644
--- a/gcc/config/vx-common.h
+++ b/gcc/config/vx-common.h
@@ -69,7 +69,7 @@ along with GCC; see the file COPYING3.  If not see
 #undef WINT_TYPE_SIZE
 #define WINT_TYPE_SIZE WCHAR_TYPE_SIZE
 #undef WINT_TYPE
-#define WINT_TYPE WCHAR_TYPE
+#define WINT_TYPE "wchar_t"
 
 /* ---------------------- Debug and unwind info formats ------------------  */
 

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] [vxworks] make wint_t and wchar_t the same distinct type

[Jason Merrill]

On 2/17/23 23:04, Alexandre Oliva wrote:
> 
> We used to define WINT_TYPE to WCHAR_TYPE, so that both wint_t and
> wchar_t mapped to the same underlying type, but this caused a glitch
> in Wstringop-overflow-6.C: on vxworks, wint_t is typedef'ed to
> wchar_t

And fixing that isn't an option?  Do the integer builtins work properly 
if we force them to use wchar_t instead of an integer type?

> headers got included in the test that declared functions that
> take wint_t parameters, and those conflicted with the builtin
> declarations that had wint_t mapped to the underlying integral type.
> 
> The problem is that, in C++, wchar_t is a distinct type.  Having
> wint_t be a typedef to wchar_t in the headers, but a typedef to
> wchar_t's underlying integral type in builtins, makes for mismatches
> between the declarations.
> 
> This patch defines WINT_TYPE to "wchar_t" for vxworks, and adjusts the
> fallout, namely:
> 
> - since wchar_t may not have been defined yet when
>    c_common_nodes_and_builtins runs, use the node already reserved for
>    wchar_t for wint_t when WINT_TYPE is defined to wchar_t.
> 
> - for the same reason, when WINT_TYPE is wchar_t and we're not
>    compiling C++ where wchar_t is a compiler built-in, define
>    __WINT_TYPE__ to WCHAR_TYPE rather than WINT_TYPE, because wchar_t
>    may not even be defined in the translation unit.
> 
> - recognize and handle wchar_type_node when type_suffix is called for
>    wint_type_node.
> 
> Regstrapped on x86_64-linux-gnu.
> Tested on arm-vxworks7 (gcc-12) and arm-eabi (trunk).  Ok to install?
> 
> for  gcc/ChangeLog
> 
> 	* config/vx-common.h (WINT_TYPE): Alias to "wchar_t".
> 
> for  gcc/c-family/ChangeLog
> 
> 	* c-common.cc (c_common_nodes_and_builtins): Take
> 	wchar_type_node for wint_type_node when aliased.
> 	(c_stddef_cpp_builtins): Define __WINT_TYPE__, when aliased to
> 	wchar_t, to the underlying type rather than wchar_t in
> 	non-C++.
> 	* c-cppbuiltin.cc (type_suffix): Handle wchar_type_node.
> ---
>   gcc/c-family/c-common.cc     |   16 +++++++++++++---
>   gcc/c-family/c-cppbuiltin.cc |    2 ++
>   gcc/config/vx-common.h       |    2 +-
>   3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
> index ae92cd5adaf5e..a92597c2f544f 100644
> --- a/gcc/c-family/c-common.cc
> +++ b/gcc/c-family/c-common.cc
> @@ -4576,8 +4576,11 @@ c_common_nodes_and_builtins (void)
>     char32_array_type_node
>       = build_array_type (char32_type_node, array_domain_type);
>   
> -  wint_type_node =
> -    TREE_TYPE (identifier_global_value (get_identifier (WINT_TYPE)));
> +  if (strcmp (WINT_TYPE, "wchar_t") == 0)
> +    wint_type_node = wchar_type_node;
> +  else
> +    wint_type_node =
> +      TREE_TYPE (identifier_global_value (get_identifier (WINT_TYPE)));
>   
>     intmax_type_node =
>       TREE_TYPE (identifier_global_value (get_identifier (INTMAX_TYPE)));
> @@ -5359,7 +5362,14 @@ c_stddef_cpp_builtins(void)
>     builtin_define_with_value ("__SIZE_TYPE__", SIZE_TYPE, 0);
>     builtin_define_with_value ("__PTRDIFF_TYPE__", PTRDIFF_TYPE, 0);
>     builtin_define_with_value ("__WCHAR_TYPE__", MODIFIED_WCHAR_TYPE, 0);
> -  builtin_define_with_value ("__WINT_TYPE__", WINT_TYPE, 0);
> +  /* C++ has wchar_t as a builtin type, C doesn't, so if WINT_TYPE
> +     maps to wchar_t, define it to the underlying WCHAR_TYPE in C, and
> +     to wchar_t in C++, so the desired type equivalence holds.  */
> +  if (!c_dialect_cxx ()
> +      && strcmp (WINT_TYPE, "wchar_t") == 0)
> +    builtin_define_with_value ("__WINT_TYPE__", WCHAR_TYPE, 0);
> +  else
> +    builtin_define_with_value ("__WINT_TYPE__", WINT_TYPE, 0);
>     builtin_define_with_value ("__INTMAX_TYPE__", INTMAX_TYPE, 0);
>     builtin_define_with_value ("__UINTMAX_TYPE__", UINTMAX_TYPE, 0);
>     if (flag_char8_t)
> diff --git a/gcc/c-family/c-cppbuiltin.cc b/gcc/c-family/c-cppbuiltin.cc
> index b333f97fd3237..98f5aef2af95d 100644
> --- a/gcc/c-family/c-cppbuiltin.cc
> +++ b/gcc/c-family/c-cppbuiltin.cc
> @@ -1903,6 +1903,8 @@ type_suffix (tree type)
>   	      systems use it anyway.  */
>   	   || type == char_type_node)
>       is_long = 0;
> +  else if (type == wchar_type_node)
> +    return type_suffix (underlying_wchar_type_node);
>     else
>       gcc_unreachable ();
>   
> diff --git a/gcc/config/vx-common.h b/gcc/config/vx-common.h
> index 83580d0dec288..9733c90fe4c6f 100644
> --- a/gcc/config/vx-common.h
> +++ b/gcc/config/vx-common.h
> @@ -69,7 +69,7 @@ along with GCC; see the file COPYING3.  If not see
>   #undef WINT_TYPE_SIZE
>   #define WINT_TYPE_SIZE WCHAR_TYPE_SIZE
>   #undef WINT_TYPE
> -#define WINT_TYPE WCHAR_TYPE
> +#define WINT_TYPE "wchar_t"
>   
>   /* ---------------------- Debug and unwind info formats ------------------  */
>   
> 

Re: [PATCH] [vxworks] make wint_t and wchar_t the same distinct type

[Alexandre Oliva]

Hello, Jason,

On Feb 17, 2023, Jason Merrill <jason@redhat.com> wrote:

> On 2/17/23 23:04, Alexandre Oliva wrote:
>> 
>> We used to define WINT_TYPE to WCHAR_TYPE, so that both wint_t and
>> wchar_t mapped to the same underlying type, but this caused a glitch
>> in Wstringop-overflow-6.C: on vxworks, wint_t is typedef'ed to
>> wchar_t

> And fixing that isn't an option?

Erhm, why do you say "fixing"?  That implies it's broken, but I don't
see anything in the C++ standards, or in the relevant bits that it
imports from the C standards, that rules out using wint_t = wchar_t in
C++.  wint_t is imported from the C standard as an integral type that
meets certain requirements, and AFAICT in C++ wchar_t is an integral
type that meets those requirements.  Am I missing something?


Now, could it be changed so that wint_t is wchar_t's underlying type
rather than wchar_t?  If the equivalence is a compliance error, we could
file a bug report with WRS and request them to fix it, but modifying
their system headers would require a copyright license they don't grant,
so we avoid doing that.  I imagine that breaking this equivalence would
have ABI implications, and even break legitimate (though unportable)
programs because of overload, specializations and whatnot, so there
would have to be very strong reasons to support a request for such a
change.

> Do the integer builtins work properly if we force them to use wchar_t
> instead of an integer type?

I haven't observed any regressions, I don't see any builtin functions
with wint in their signature that we even expand as builtins, and my
imagination is failing me on why an integral type such as wchar_t would
fail for wint_t, where other integral types, including wchar_t's
underlying type, would work.  Did you have any specific risks in mind
about what could go wrong?

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] [vxworks] make wint_t and wchar_t the same distinct type

[Jason Merrill]

On 2/22/23 10:23, Alexandre Oliva wrote:
> Hello, Jason,
> 
> On Feb 17, 2023, Jason Merrill <jason@redhat.com> wrote:
> 
>> On 2/17/23 23:04, Alexandre Oliva wrote:
>>>
>>> We used to define WINT_TYPE to WCHAR_TYPE, so that both wint_t and
>>> wchar_t mapped to the same underlying type, but this caused a glitch
>>> in Wstringop-overflow-6.C: on vxworks, wint_t is typedef'ed to
>>> wchar_t
> 
>> And fixing that isn't an option?
> 
> Erhm, why do you say "fixing"?  That implies it's broken, but I don't
> see anything in the C++ standards, or in the relevant bits that it
> imports from the C standards, that rules out using wint_t = wchar_t in
> C++.  wint_t is imported from the C standard as an integral type that
> meets certain requirements, and AFAICT in C++ wchar_t is an integral
> type that meets those requirements.  Am I missing something?
> 
> 
> Now, could it be changed so that wint_t is wchar_t's underlying type
> rather than wchar_t?  If the equivalence is a compliance error, we could
> file a bug report with WRS and request them to fix it, but modifying
> their system headers would require a copyright license they don't grant,
> so we avoid doing that.  I imagine that breaking this equivalence would
> have ABI implications, and even break legitimate (though unportable)
> programs because of overload, specializations and whatnot, so there
> would have to be very strong reasons to support a request for such a
> change.
> 
>> Do the integer builtins work properly if we force them to use wchar_t
>> instead of an integer type?
> 
> I haven't observed any regressions, I don't see any builtin functions
> with wint in their signature that we even expand as builtins, and my
> imagination is failing me on why an integral type such as wchar_t would
> fail for wint_t, where other integral types, including wchar_t's
> underlying type, would work.  Did you have any specific risks in mind
> about what could go wrong?

Sorry, I think I was mostly confusing myself about how distinct wchar_t 
really is.

The patch is OK.

Jason