[Bug libstdc++/110077] New: [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[Bug libstdc++/110077] New: [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[ro at gcc dot gnu.org]

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

            Bug ID: 110077
           Summary: [14 regression] libstdc++-abi/abi_check FAILs on
                    Solaris
           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

Between 20230530 (a899401404186843f38462c8fc9de733f19ce864) and 20230531
(070d651c6db37c3658be0a5274f44265045428e6),
libstdc++-abi/abi_check started to FAIL on Solaris (both sparc and x86, 32 and
64-bit):

+FAIL: libstdc++-abi/abi_check

The log file shows

1 incompatible symbols
0
_ZSt10from_charsPKcS0_RDF128_St12chars_format
std::from_chars(char const*, char const*, _Float128&, std::chars_format)
version status: incompatible
GLIBCXX_3.4.31
type: function
status: added

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[ro at gcc dot gnu.org]

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

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

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

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
   Last reconfirmed|                            |2023-06-01
      Known to fail|                            |14.0
           Keywords|                            |ABI
             Status|UNCONFIRMED                 |ASSIGNED
           Priority|P3                          |P1
     Ever confirmed|0                           |1
      Known to work|                            |13.1.1

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|*-*-solaris2.11             |*-*-solaris2.11
                   |                            |aarch4-unknown-linux-gnu

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This affects aarch64 too:
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/620335.html
And probably other targets where long double uses binary128 rep.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I'm testing this fix:

--- a/libstdc++-v3/src/c++17/floating_from_chars.cc
+++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
@@ -1325,7 +1325,8 @@ _ZSt10from_charsPKcS0_RDF128_St12chars_format(const char*
first,
                                              __ieee128& value,
                                              chars_format fmt) noexcept
 __attribute__((alias ("_ZSt10from_charsPKcS0_Ru9__ieee128St12chars_format")));
-#elif defined(__FLT128_MANT_DIG__)
+#elif __FLT128_MANT_DIG__ == 113 && __LDBL_MANT_DIG__ != 113
+// Overload for _Float128 is not defined inline in <charconv>, define it here.
 from_chars_result
 from_chars(const char* first, const char* last, _Float128& value,
           chars_format fmt) noexcept

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS 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:00da6bcfccbc5ab13821d8dd7334dd48c22d5702

commit r14-1651-g00da6bcfccbc5ab13821d8dd7334dd48c22d5702
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 9 11:08:03 2023 +0100

    libstdc++: Remove duplicate definition of _Float128 std::from_chars
[PR110077]

    When long double uses IEEE binary128 representation we define the
    _Float128 overload of std::from_chars inline in <charconv>. My changes
    in r14-1431-g7037e7b6e4ac41 cause it to also be defined non-inline in
    the library, leading to an abi-check failure for (at least) sparc and
    aarch64.

    Suppress the definition in the library if long double and _Float128 have
    are both IEEE binary128.

    libstdc++-v3/ChangeLog:

            PR libstdc++/110077
            * src/c++17/floating_from_chars.cc (from_chars) <_Float128>:
            Only define if _Float128 and long double have different
            representations.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

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

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This should be fixed now

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[ro at gcc dot gnu.org]

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

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

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

--- Comment #5 from Rainer Orth <ro at gcc dot gnu.org> ---
It is on sparc, but x86 still FAILs like before.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This is going to be hard for me to figure out without access to a Solaris x86
system.

Could you please attach the output of this command using GCC trunk on solaris
x86?

g++ -std=c++23 -include charconv -x c++ /dev/null -E -dD

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[ro at gcc dot gnu.org]

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

--- Comment #7 from Rainer Orth <ro at gcc dot gnu.org> ---
Created attachment 55426
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55426&action=edit
32-bit i386-pc-solaris2.11 charconv.ii

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[ro at gcc dot gnu.org]

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

--- Comment #8 from Rainer Orth <ro at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #6)
> This is going to be hard for me to figure out without access to a Solaris
> x86 system.

There's hope that at least one, maybe two, Solaris 11.4/x86 systems can be
added to the cfarm.  As always, the primary problem is politics...

> Could you please attach the output of this command using GCC trunk on
> solaris x86?
> 
> g++ -std=c++23 -include charconv -x c++ /dev/null -E -dD

Sure, done.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Thanks for the quick response!

For x86 both these conditions are false:

#if defined(__STDCPP_FLOAT128_T__) &&
defined(_GLIBCXX_LDOUBLE_IS_IEEE_BINARY128)
...
#elif defined(__STDCPP_FLOAT128_T__) && defined(_GLIBCXX_HAVE_FLOAT128_MATH)
...
#endif

and that means we the header doesn't declare from_chars for _Float128.

With my changes for PR 109921 (r14-1431) that from_chars overload gets defined
in the library, causing the abi-check error. But if it's not present in the
header, it's useless to define it in the library, it can't be called.

One solution would be to just add the declaration to the header, and adjust the
exports so this new symbol is exported at GLIBCXX_3.4.32 not GLIBCXX_3.4.31

Alternatively, I need to find the right set of conditions to suppress the
definition in the library.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #9)
> One solution would be to just add the declaration to the header, and adjust
> the exports so this new symbol is exported at GLIBCXX_3.4.32 not
> GLIBCXX_3.4.31

N.B. this is what we do for glibc-based linux targets. The symbol is present in
the library even when glibc doesn't provide strtof128. This means that we don't
have a different set of exported symbols when built on old or new glibc.

If Solaris is ever going to get support for strtof128 and other _Float128
support then that is probably what we should do here as well.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #1)
> This affects aarch64 too:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-June/620335.html
> And probably other targets where long double uses binary128 rep.

For the record, this is fixed for aarch64 (by the same commit that fixed it for
solaris sparc):
https://gcc.gnu.org/pipermail/libstdc++/2023-June/056246.html

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #10)
> (In reply to Jonathan Wakely from comment #9)
> > One solution would be to just add the declaration to the header, and adjust
> > the exports so this new symbol is exported at GLIBCXX_3.4.32 not
> > GLIBCXX_3.4.31
> 
> N.B. this is what we do for glibc-based linux targets. The symbol is present
> in the library even when glibc doesn't provide strtof128. This means that we
> don't have a different set of exported symbols when built on old or new
> glibc.

Actually that's not true, we always define it in the library but for old glibc
we *don't* declare it in the header. So to make Solaris x86 consistent we'd
just want to fix up the symbol version.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #13 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> (In reply to Jonathan Wakely from comment #9)
>> One solution would be to just add the declaration to the header, and adjust
>> the exports so this new symbol is exported at GLIBCXX_3.4.32 not
>> GLIBCXX_3.4.31
>
> N.B. this is what we do for glibc-based linux targets. The symbol is present in
> the library even when glibc doesn't provide strtof128. This means that we don't
> have a different set of exported symbols when built on old or new glibc.
>
> If Solaris is ever going to get support for strtof128 and other _Float128
> support then that is probably what we should do here as well.

That would mean an implementation of C23 Annex H, right?  IIUC,
implementing that is optional.

However, I can reach out to determine if there are any plans for that.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #13)
> That would mean an implementation of C23 Annex H, right?  IIUC,
> implementing that is optional.

Right (on both counts).

> However, I can reach out to determine if there are any plans for that.

Thanks, that would be useful.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I now think the right fix is:

--- a/libstdc++-v3/src/c++17/floating_from_chars.cc
+++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
@@ -1325,24 +1325,14 @@ _ZSt10from_charsPKcS0_RDF128_St12chars_format(const
char* first,
                                              __ieee128& value,
                                              chars_format fmt) noexcept
 __attribute__((alias ("_ZSt10from_charsPKcS0_Ru9__ieee128St12chars_format")));
-#elif __FLT128_MANT_DIG__ == 113 && __LDBL_MANT_DIG__ != 113
+#elif defined(USE_STRTOF128_FOR_FROM_CHARS)
 // Overload for _Float128 is not defined inline in <charconv>, define it here.
 from_chars_result
 from_chars(const char* first, const char* last, _Float128& value,
           chars_format fmt) noexcept
 {
-#ifdef USE_STRTOF128_FOR_FROM_CHARS
   // fast_float doesn't support IEEE binary128 format, but we can use strtold.
   return from_chars_strtod(first, last, value, fmt);
-#else
-  // Read a long double. This might give an incorrect result (e.g. values
-  // out of range of long double give an error, even if they fit in
_Float128).
-  long double ldbl_val;
-  auto res = std::from_chars(first, last, ldbl_val, fmt);
-  if (res.ec == errc{})
-    value = ldbl_val;
-  return res;
-#endif
 }
 #endif


I think r14-1431-g7037e7b6e4ac41 was wrong to try to define that overload
unconditionally.  Not all targets need it, so defining the lossy fallback using
long double is not useful (and caused this ABI change on Solaris x86).

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[cvs-commit at gcc dot gnu.org]

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

--- Comment #16 from CVS 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:0867d30a68de68f4c809757348447bef94ef1491

commit r14-2661-g0867d30a68de68f4c809757348447bef94ef1491
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jul 19 21:15:17 2023 +0100

    libstdc++: Do not define inaccurate from_chars for _Float128 [PR110077]

    I think r14-1431-g7037e7b6e4ac41 was wrong to try to define the
    _Float128 overload unconditionally. Not all targets need it, so defining
    the lossy fallback using long double is not useful (and caused an ABI
    change on Solaris x86).

    Making the definition depend on USE_STRTOF128_FOR_FROM_CHARS again
    partially reverts the change for PR 109921, however that should still be
    fixed because the changes to make USE_STRTOF128_FOR_FROM_CHARS depend on
    USE_STRTOD_FOR_FROM_CHARS are not reverted.

    libstdc++-v3/ChangeLog:

            PR libstdc++/110077
            * src/c++17/floating_from_chars.cc (from_chars): Only define
            _Float128 overload when using __strfromf128.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

--- Comment #17 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I hope this is fixed now.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #18 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #17 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> I hope this is fixed now.

It is indeed.  Thanks a lot.

[Bug libstdc++/110077] [14 regression] libstdc++-abi/abi_check FAILs on Solaris

[redi at gcc dot gnu.org]

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

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

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

--- Comment #19 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Great, thanks for confirming it!