[Bug libstdc++/113450] New: std/format/functions/format.cc FAILs

[Bug libstdc++/113450] New: std/format/functions/format.cc FAILs

[ro at gcc dot gnu.org]

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

            Bug ID: 113450
           Summary: std/format/functions/format.cc FAILs
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ro at gcc dot gnu.org
                CC: redi at gcc dot gnu.org
  Target Milestone: ---
            Target: *-*-solaris2.11

The new std/format/functions/format.cc test FAILs on Solaris/SPARC and x86,
both
32 and 64-bit:

+FAIL: std/format/functions/format.cc  -std=gnu++20 execution test
+FAIL: std/format/functions/format.cc  -std=gnu++26 execution test

It shows an assertion failure

/vol/gcc/src/hg/master/local/libstdc++-v3/testsuite/std/format/functions/format.cc:360:
test_minmax()::<lambda(T, U)> [with T = char; U = unsigned char]: Assertion 's
== "-1" + zeros' failed.

#3  0x00027554 in operator()<char> (__closure=<optimized out>)
    at
/vol/gcc/src/hg/master/local/libstdc++-v3/testsuite/std/format/functions/format.cc:360
360         VERIFY( s == "-1" + zeros );
(gdb) p s
$1 = {
  _M_dataplus = {<std::allocator<char>> = {<std::__new_allocator<char>> = {<No
data fields>}, <No data fields>}, _M_p = 0xffbfe808 "10000000"}, 
  _M_string_length = 8, {
    _M_local_buf = "10000000\000\000\000\035\377\277\350\030", 
    _M_allocated_capacity = 825241648}}

[Bug libstdc++/113450] std/format/functions/format.cc FAILs

[ro at gcc dot gnu.org]

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

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-01-17

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I assume that int8_t is char on Solaris, rather than signed char?

Formatting a char behaves differently from signed char, and other integral
types.

I think this will fix it:

--- a/libstdc++-v3/testsuite/std/format/functions/format.cc
+++ b/libstdc++-v3/testsuite/std/format/functions/format.cc
@@ -365,7 +365,7 @@ test_minmax()
     s = std::format("{:b}" , std::numeric_limits<U>::max());
     VERIFY( s == '1' + ones );
   };
-  check(std::int8_t(0));
+  check(std::make_signed_t<std::int8_t>(0));
   check(std::int16_t(0));
   check(std::int32_t(0));
   check(std::int64_t(0));


That causes the lambda to use signed char instead of char, and that is
formatted as an integer not a character.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #2 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> I assume that int8_t is char on Solaris, rather than signed char?

Indeed.  AFAIK char being signed goes back to SysVr4 at least (and has
bitten us several times already, especially over in LLVM).

> Formatting a char behaves differently from signed char, and other integral
> types.
>
> I think this will fix it:
[...]
> That causes the lambda to use signed char instead of char, and that is
> formatted as an integer not a character.

It does indeed.  Tested on both i386-pc-solaris2.11 and
sparc-sun-solaris2.11 for good measure.  Thanks!

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:db42a0a98916340af33338c08e6a7d328121b958

commit r14-8224-gdb42a0a98916340af33338c08e6a7d328121b958
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jan 17 21:40:25 2024 +0000

    libstdc++: Fix std::format test for Solaris [PR113450]

    When int8_t is a typedef for char (rather than signed char) this test
    fails because it tries to format a char, which is treated differently
    from formatting other integral types (including signed char).

    Use signed char explicitly so the result doesn't depend on the
    non-portable definition of int8_t.

    libstdc++-v3/ChangeLog:

            PR libstdc++/113450
            * testsuite/std/format/functions/format.cc: Use signed char
            instead of int8_t.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I pushed a slightly different change, but it should be equivalent. Please
reopen if I messed it up :-)

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:5240df78a07303a37b1f0b83165624d2a648089e

commit r13-8238-g5240df78a07303a37b1f0b83165624d2a648089e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jan 17 21:40:25 2024 +0000

    libstdc++: Fix std::format test for Solaris [PR113450]

    When int8_t is a typedef for char (rather than signed char) this test
    fails because it tries to format a char, which is treated differently
    from formatting other integral types (including signed char).

    Use signed char explicitly so the result doesn't depend on the
    non-portable definition of int8_t.

    libstdc++-v3/ChangeLog:

            PR libstdc++/113450
            * testsuite/std/format/functions/format.cc: Use signed char
            instead of int8_t.

    (cherry picked from commit db42a0a98916340af33338c08e6a7d328121b958)

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #6 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> I pushed a slightly different change, but it should be equivalent. Please
> reopen if I messed it up :-)

The variant worked just fine.  Thanks.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #1)
> I assume that int8_t is char on Solaris, rather than signed char?

This actually violates the C and C++ standards, which require that intN_t is a
signed integer type, and C 6.2.5 says "There are five standard signed integer
types, designated as signed char, short int, int, long int, and long long int."
So char isn't allowed, it should be signed char.

I don't know if it's possible to fix that on Solaris.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #7)
> (In reply to Jonathan Wakely from comment #1)
> > I assume that int8_t is char on Solaris, rather than signed char?
> 
> This actually violates the C and C++ standards, which require that intN_t is
> a signed integer type, and C 6.2.5 says "There are five standard signed
> integer types, designated as signed char, short int, int, long int, and long
> long int." So char isn't allowed, it should be signed char.
> 
> I don't know if it's possible to fix that on Solaris.

fixinc?

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It's technically an ABI break, since void f(int8_t) would mangle differently.
It probably wouldn't affect much in practice, but would still be a break.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Or convince Oracle to change it (again, an ABI break).

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #11 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> (In reply to Jonathan Wakely from comment #1)
>> I assume that int8_t is char on Solaris, rather than signed char?
>
> This actually violates the C and C++ standards, which require that intN_t is a
> signed integer type, and C 6.2.5 says "There are five standard signed integer
> types, designated as signed char, short int, int, long int, and long long int."
> So char isn't allowed, it should be signed char.

I've done some digging now: <inttypes.h>/<sys/int_types.h> were
introduced into Solaris 2.6 (the file dates from Jul 16 1997), way
before the C99 standard was finalized.

I've looked around the WG14 archive, and some drafts after that date
(N737 (1997-06-26), N788 (1997-11-17), N851 (1998-09-22)) do have the
`signed integer type' wording.  Unfortunately, all of the previous ones
(which would have been the basis of the Solaris implementation) are no
longer available.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #12 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> It's technically an ABI break, since void f(int8_t) would mangle differently.
> It probably wouldn't affect much in practice, but would still be a break.

Still, given how serious Sun/Oracle is about ABI compatibility (and
always has been), I'm doubtful they would be amenable to the idea.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #13 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Or convince Oracle to change it (again, an ABI break).

I can try, but don't hold your breath.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #14 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #11 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot
> Uni-Bielefeld.DE> ---
>> --- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
>> (In reply to Jonathan Wakely from comment #1)
>>> I assume that int8_t is char on Solaris, rather than signed char?
>>
>> This actually violates the C and C++ standards, which require that intN_t is a
>> signed integer type, and C 6.2.5 says "There are five standard signed integer
>> types, designated as signed char, short int, int, long int, and long long int."
>> So char isn't allowed, it should be signed char.
>
> I've done some digging now: <inttypes.h>/<sys/int_types.h> were
> introduced into Solaris 2.6 (the file dates from Jul 16 1997), way
> before the C99 standard was finalized.

Further research has uncovered some interesting facts:

* Neither the SysV SPARC psABI (3rd Edition, 1996) nor the original i386
  psABI (4th Edition, 1997) specify int8_t at all.  Actually no wonder
  because both predate C99.

  However, even the current Intel386 psABI 1.1 (2015) has nothing here,
  nor does the AMD64 psABI 1.0 (2024).

  To my astonishment however, the SPARC Compliance Definition 2.4.1
  (1999), defacto if not in name the SPARC V9 psABI, lists in Figure
  6-140: <inttypes.h>, p.6P-13:

  typedef signed char int8_t;

  So it seems at least Solaris/SPARCV9 violates its own ABI ;-(

* When checking clang, there were more surprises:

#define __INT8_TYPE__ signed char

  With very few exceptions, clang uses the default definitions of the
  intN_t types everywhere.

  However, the question remains how much that counts: given clang/llvm
  has seen years of neglect on Solaris, I wonder how much actual code is
  really built with it on Solaris.

* The Oracle Studio 12.6 cc has no definition of __INT8_TYPE__ at all
  AFAICT.  If that's true, one could change the type's definition simply
  by adjusting <sys/int_types.h>, which would be nice and easy.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[redi at gcc dot gnu.org]

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

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #14)
> Further research has uncovered some interesting facts:
> 
> * Neither the SysV SPARC psABI (3rd Edition, 1996) nor the original i386
>   psABI (4th Edition, 1997) specify int8_t at all.  Actually no wonder
>   because both predate C99.
> 
>   However, even the current Intel386 psABI 1.1 (2015) has nothing here,
>   nor does the AMD64 psABI 1.0 (2024).
> 
>   To my astonishment however, the SPARC Compliance Definition 2.4.1
>   (1999), defacto if not in name the SPARC V9 psABI, lists in Figure
>   6-140: <inttypes.h>, p.6P-13:
> 
>   typedef signed char int8_t;
> 
>   So it seems at least Solaris/SPARCV9 violates its own ABI ;-(

Ouch!

> * When checking clang, there were more surprises:
> 
> #define __INT8_TYPE__ signed char
> 
>   With very few exceptions, clang uses the default definitions of the
>   intN_t types everywhere.

That's interesting. I think GCC defines those __INTn_TYPE__ macros after
inspecting the target headers (assuming the target provides <stdint.h> or
<inttypes.h>) to ensure they agree. I wonder if Clang intentionally deviated
from the Solaris headers to "fix" this issue, or if they just define
__INT8_TYPE__ to signed char for all 8-bit targets because "obviously" that's
correct for all targets (even though it isn't actually correct for Solaris).

That means GCC and Clang will disagree about the mangling of a C++ function
like
void foo(int8_t);

>   However, the question remains how much that counts: given clang/llvm
>   has seen years of neglect on Solaris, I wonder how much actual code is
>   really built with it on Solaris.
> 
> * The Oracle Studio 12.6 cc has no definition of __INT8_TYPE__ at all
>   AFAICT.  If that's true, one could change the type's definition simply
>   by adjusting <sys/int_types.h>, which would be nice and easy.

I think those macros are a GCC thing not required by any standard. Oracle
Studio can just rely on <stdint.h> being present, because they know that's true
for Solaris. GCC can't (or couldn't historically) rely on <stdint.h> being
present for all targets so needed some other way to make those types available.

Thanks for digging into this!

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #16 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> (In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #14)
>> * When checking clang, there were more surprises:
>> 
>> #define __INT8_TYPE__ signed char
>> 
>>   With very few exceptions, clang uses the default definitions of the
>>   intN_t types everywhere.
>
> That's interesting. I think GCC defines those __INTn_TYPE__ macros after
> inspecting the target headers (assuming the target provides <stdint.h> or
> <inttypes.h>) to ensure they agree. I wonder if Clang intentionally deviated

Unfortunately not: gcc hardcodes all those types all over gcc/config
(e.g. sol2.h) as INT8_TYPE.

> from the Solaris headers to "fix" this issue, or if they just define
> __INT8_TYPE__ to signed char for all 8-bit targets because "obviously" that's
> correct for all targets (even though it isn't actually correct for Solaris).

I guess no one cared enough about Solaris in Clang to notice this.
There isn't even a test to check if Clang's internal idea of e.g. int8_t
and that of the system headers match (but neither does gcc).

> That means GCC and Clang will disagree about the mangling of a C++ function
> like
> void foo(int8_t);

As I found, they won't: while their internal definitions of
__INT8_TYPE__ differ, when one wants to use int8_t, one needs to include
a corresponding header (<stdint.h>/<inttypes.h>/<cstdint>), thus will
always get what the header defines.

Only when using __INT8_TYPE__ directly will you get the mangled form of
the compiler's internal idea of int8_t.

To check what happens, I've re-run last night bootstraps (sparc and x86)
with <sys/int_types.h> changed to define int8_t and friends as signed
char, at the same time modifying gcc/config/sol2.h to match.

The bootstrap (i386 so far, sparc still running, though I don't expect
any difference) went without issues: the only regression seen is

+UNRESOLVED: gdc.test/runnable_cxx/stdint.d   compilation failed to produce
executable
+UNRESOLVED: gdc.test/runnable_cxx/stdint.d -shared-libphobos   compilation
failed to produce executable

where the executable fails to link:

Undefined                       first referenced
 symbol                             in file
_Z15testCppI8Manglechchch           /var/tmp//ccJLlOBa.o

This test has D (stdint.d) and C++ (extra-files/stdint.cpp) components,
checking the gdc and g++'s ideas of various types match.  Unlike C/C++,
where the definition of int8_t is from <sys/int_types.h>, D has it's
equivalent in libphobos/libdruntime/core/stdc/stdint.d which will have
to be adjusted, too.

>>   However, the question remains how much that counts: given clang/llvm
>>   has seen years of neglect on Solaris, I wonder how much actual code is
>>   really built with it on Solaris.
>> 
>> * The Oracle Studio 12.6 cc has no definition of __INT8_TYPE__ at all
>>   AFAICT.  If that's true, one could change the type's definition simply
>>   by adjusting <sys/int_types.h>, which would be nice and easy.
>
> I think those macros are a GCC thing not required by any standard. Oracle
> Studio can just rely on <stdint.h> being present, because they know that's true
> for Solaris. GCC can't (or couldn't historically) rely on <stdint.h> being
> present for all targets so needed some other way to make those types available.

I guess so: I just meant to find out if cc/CC has it's own idea of the
types apart from the system headers.  However, going forward, this can
certainly learned from Oracle.

I'll think given the information so far, I'll take the idea of changing
int8_t for C99+ conformance to them and see what they think.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[jsm28 at gcc dot gnu.org]

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

--- Comment #17 from Joseph S. Myers <jsm28 at gcc dot gnu.org> ---
The tests that GCC's internal notion of the types agrees with the headers are
in gcc.dg/c99-stdint-5.c and gcc.dg/c99-stdint-6.c.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #18 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #17 from Joseph S. Myers <jsm28 at gcc dot gnu.org> ---
> The tests that GCC's internal notion of the types agrees with the headers are
> in gcc.dg/c99-stdint-5.c and gcc.dg/c99-stdint-6.c.

Ah, now I remember: they had to be adjusted back in the day for various
omissions on Solaris and Tru64 UNIX.

During my recent test bootstrap, they didn't show up because I had
changed <sys/int_types.h> and sol2.h's INT8_TYPE etc. in lockstep, so no
inconsistencies occured.

[Bug libstdc++/113450] [14 Regression] std/format/functions/format.cc FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #19 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
I'm talking with Oracle Solaris Engineering and they're amenable to
making the int8_t change from char to signed char.

To assess the possible impact, the plan is to compare the public symbols
of C++ libraries delivered with Solaris now and after a rebuild with
<sys/int_types.h> changed.

Are there other important issues to consider with such a change?