| Message ID | CAFnZKr1EPETS4CNu0rHwkhWSGRDt_e0TJpjUPTfSeDywDXyVRg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | libstdcxx: Update ctype_base.h from NetBSD upstream | expand |
Hi Matthew, On Mon, 4 Feb 2019, Matthew Bauer wrote: > The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They > have changed their ctype.h definition. It was updated in their intree > libstdc++-v3 but not in the GCC one. My understanding is this is a > straightforward rewrite. I've attached my own patch, but the file can > be obtained directly here: > > http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h > > With the attached patch, libstdc++-v3 can succesfully be built with > NetBSD headers (along with --disable-libcilkrts). I noticed this has not been applied yet, nor seen a follow-up?, and also noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. Let me re-address this to libstdc++@gcc.gnu.org in the hope the maintainers there will have a look. Gerald
On 22.12.2019 00:36, Gerald Pfeifer wrote: > Hi Matthew, > > On Mon, 4 Feb 2019, Matthew Bauer wrote: >> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >> have changed their ctype.h definition. It was updated in their intree >> libstdc++-v3 but not in the GCC one. My understanding is this is a >> straightforward rewrite. I've attached my own patch, but the file can >> be obtained directly here: >> >> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >> >> With the attached patch, libstdc++-v3 can succesfully be built with >> NetBSD headers (along with --disable-libcilkrts). > > I noticed this has not been applied yet, nor seen a follow-up?, and also > noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. > > Let me re-address this to libstdc++@gcc.gnu.org in the hope the > maintainers there will have a look. > > Gerald > We (in NetBSD) wait for this to be merged.
On Sat, 21 Dec 2019 at 23:37, Gerald Pfeifer <gerald@pfeifer.com> wrote: > > Hi Matthew, > > On Mon, 4 Feb 2019, Matthew Bauer wrote: > > The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They > > have changed their ctype.h definition. It was updated in their intree > > libstdc++-v3 but not in the GCC one. My understanding is this is a > > straightforward rewrite. I've attached my own patch, but the file can > > be obtained directly here: > > > > http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h > > > > With the attached patch, libstdc++-v3 can succesfully be built with > > NetBSD headers (along with --disable-libcilkrts). > > I noticed this has not been applied yet, nor seen a follow-up?, and also > noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. That's why it was ignored then. > Let me re-address this to libstdc++@gcc.gnu.org in the hope the > maintainers there will have a look. I'll take a look ASAP.
On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >Hi Matthew, > >On Mon, 4 Feb 2019, Matthew Bauer wrote: >> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >> have changed their ctype.h definition. It was updated in their intree >> libstdc++-v3 but not in the GCC one. My understanding is this is a >> straightforward rewrite. I've attached my own patch, but the file can >> be obtained directly here: >> >> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >> >> With the attached patch, libstdc++-v3 can succesfully be built with >> NetBSD headers (along with --disable-libcilkrts). > >I noticed this has not been applied yet, nor seen a follow-up?, and also >noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. > >Let me re-address this to libstdc++@gcc.gnu.org in the hope the >maintainers there will have a look. > >Gerald >diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >index ff3ec893974..21eccf9fde1 100644 >--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >@@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > /// @brief Base class for ctype. > struct ctype_base > { >- // Non-standard typedefs. >- typedef const unsigned char* __to_type; > > // NB: Offsets into ctype<char>::_M_table force a particular size > // on the mask type. Because of this, we don't use an enum. >- typedef unsigned char mask; > > #ifndef _CTYPE_U >- static const mask upper = _U; >- static const mask lower = _L; >- static const mask alpha = _U | _L; >- static const mask digit = _N; >- static const mask xdigit = _N | _X; >- static const mask space = _S; >- static const mask print = _P | _U | _L | _N | _B; >- static const mask graph = _P | _U | _L | _N; >- static const mask cntrl = _C; >- static const mask punct = _P; >- static const mask alnum = _U | _L | _N; >+ // Non-standard typedefs. >+ typedef const unsigned char* __to_type; >+ >+ typedef unsigned char mask; >+ >+ static const mask upper = _U; >+ static const mask lower = _L; >+ static const mask alpha = _U | _L; >+ static const mask digit = _N; >+ static const mask xdigit = _N | _X; >+ static const mask space = _S; >+ static const mask print = _P | _U | _L | _N | _B; >+ static const mask graph = _P | _U | _L | _N; >+ static const mask cntrl = _C; >+ static const mask punct = _P; >+ static const mask alnum = _U | _L | _N; > #else >- static const mask upper = _CTYPE_U; >- static const mask lower = _CTYPE_L; >- static const mask alpha = _CTYPE_U | _CTYPE_L; >- static const mask digit = _CTYPE_N; >- static const mask xdigit = _CTYPE_N | _CTYPE_X; >- static const mask space = _CTYPE_S; >- static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N | _CTYPE_B; >- static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N; >- static const mask cntrl = _CTYPE_C; >- static const mask punct = _CTYPE_P; >- static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >+ typedef const unsigned short* __to_type; >+ >+ typedef unsigned short mask; I seem to recall looking at this previously and noting that the change to ctype_base::mask is an ABI break. It means that code compiled with old versions of GCC or on old versions of NetBSD will not be ABI compatible with code compiled by a new GCC on a new version of NetBSD. If the NetBSD maintainers are OK with that, then we can go ahead and change it.
On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >Hi Matthew, > >On Mon, 4 Feb 2019, Matthew Bauer wrote: >> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >> have changed their ctype.h definition. It was updated in their intree >> libstdc++-v3 but not in the GCC one. My understanding is this is a >> straightforward rewrite. I've attached my own patch, but the file can >> be obtained directly here: >> >> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >> >> With the attached patch, libstdc++-v3 can succesfully be built with >> NetBSD headers (along with --disable-libcilkrts). > >I noticed this has not been applied yet, nor seen a follow-up?, and also >noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. > >Let me re-address this to libstdc++@gcc.gnu.org in the hope the >maintainers there will have a look. > >Gerald >diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >index ff3ec893974..21eccf9fde1 100644 >--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >@@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > /// @brief Base class for ctype. > struct ctype_base > { >- // Non-standard typedefs. >- typedef const unsigned char* __to_type; > > // NB: Offsets into ctype<char>::_M_table force a particular size > // on the mask type. Because of this, we don't use an enum. >- typedef unsigned char mask; > > #ifndef _CTYPE_U >- static const mask upper = _U; >- static const mask lower = _L; >- static const mask alpha = _U | _L; >- static const mask digit = _N; >- static const mask xdigit = _N | _X; >- static const mask space = _S; >- static const mask print = _P | _U | _L | _N | _B; >- static const mask graph = _P | _U | _L | _N; >- static const mask cntrl = _C; >- static const mask punct = _P; >- static const mask alnum = _U | _L | _N; >+ // Non-standard typedefs. >+ typedef const unsigned char* __to_type; >+ >+ typedef unsigned char mask; >+ >+ static const mask upper = _U; >+ static const mask lower = _L; >+ static const mask alpha = _U | _L; >+ static const mask digit = _N; >+ static const mask xdigit = _N | _X; >+ static const mask space = _S; >+ static const mask print = _P | _U | _L | _N | _B; >+ static const mask graph = _P | _U | _L | _N; >+ static const mask cntrl = _C; >+ static const mask punct = _P; >+ static const mask alnum = _U | _L | _N; > #else >- static const mask upper = _CTYPE_U; >- static const mask lower = _CTYPE_L; >- static const mask alpha = _CTYPE_U | _CTYPE_L; >- static const mask digit = _CTYPE_N; >- static const mask xdigit = _CTYPE_N | _CTYPE_X; >- static const mask space = _CTYPE_S; >- static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N | _CTYPE_B; >- static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N; >- static const mask cntrl = _CTYPE_C; >- static const mask punct = _CTYPE_P; >- static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >+ typedef const unsigned short* __to_type; >+ >+ typedef unsigned short mask; >+ >+ static const mask upper = _CTYPE_U; >+ static const mask lower = _CTYPE_L; >+ static const mask alpha = _CTYPE_A; >+ static const mask digit = _CTYPE_D; >+ static const mask xdigit = _CTYPE_X; >+ static const mask space = _CTYPE_S; >+ static const mask print = _CTYPE_R; >+ static const mask graph = _CTYPE_G; >+ static const mask cntrl = _CTYPE_C; >+ static const mask punct = _CTYPE_P; >+ static const mask alnum = _CTYPE_A | _CTYPE_D; > #endif > #if __cplusplus >= 201103L >- static const mask blank = space; >+ static const mask blank = space; > #endif > }; > >diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >index ed3b7cd0d6a..33358e8f5d8 100644 >--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >@@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > // Information as gleaned from /usr/include/ctype.h > >- extern "C" const u_int8_t _C_ctype_[]; >- > const ctype_base::mask* > ctype<char>::classic_table() throw() >- { return _C_ctype_ + 1; } >+ { return _C_ctype_tab_ + 1; } The first patch attached to PR 64271 (i.e. https://gcc.gnu.org/bugzilla/attachment.cgi?id=34254 ) does that differently. Is it safe to make this change unconditionally? Who authored these patches? We don't seem to have a changelog, not even an author's name and email address, as required for any patch. Here was my previous review, where I mentioned the ABI break: https://gcc.gnu.org/ml/libstdc++/2014-12/msg00069.html I didn't get a reply.
On 06.01.2020 16:24, Jonathan Wakely wrote: > On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >> Hi Matthew, >> >> On Mon, 4 Feb 2019, Matthew Bauer wrote: >>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >>> have changed their ctype.h definition. It was updated in their intree >>> libstdc++-v3 but not in the GCC one. My understanding is this is a >>> straightforward rewrite. I've attached my own patch, but the file can >>> be obtained directly here: >>> >>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >>> >>> >>> With the attached patch, libstdc++-v3 can succesfully be built with >>> NetBSD headers (along with --disable-libcilkrts). >> >> I noticed this has not been applied yet, nor seen a follow-up?, and also >> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. >> >> Let me re-address this to libstdc++@gcc.gnu.org in the hope the >> maintainers there will have a look. >> >> Gerald > >> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >> index ff3ec893974..21eccf9fde1 100644 >> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> /// @brief Base class for ctype. >> struct ctype_base >> { >> - // Non-standard typedefs. >> - typedef const unsigned char* __to_type; >> >> // NB: Offsets into ctype<char>::_M_table force a particular size >> // on the mask type. Because of this, we don't use an enum. >> - typedef unsigned char mask; >> >> #ifndef _CTYPE_U >> - static const mask upper = _U; >> - static const mask lower = _L; >> - static const mask alpha = _U | _L; >> - static const mask digit = _N; >> - static const mask xdigit = _N | _X; >> - static const mask space = _S; >> - static const mask print = _P | _U | _L | _N | _B; >> - static const mask graph = _P | _U | _L | _N; >> - static const mask cntrl = _C; >> - static const mask punct = _P; >> - static const mask alnum = _U | _L | _N; >> + // Non-standard typedefs. >> + typedef const unsigned char* __to_type; >> + >> + typedef unsigned char mask; >> + >> + static const mask upper = _U; >> + static const mask lower = _L; >> + static const mask alpha = _U | _L; >> + static const mask digit = _N; >> + static const mask xdigit = _N | _X; >> + static const mask space = _S; >> + static const mask print = _P | _U | _L | _N | _B; >> + static const mask graph = _P | _U | _L | _N; >> + static const mask cntrl = _C; >> + static const mask punct = _P; >> + static const mask alnum = _U | _L | _N; >> #else >> - static const mask upper = _CTYPE_U; >> - static const mask lower = _CTYPE_L; >> - static const mask alpha = _CTYPE_U | _CTYPE_L; >> - static const mask digit = _CTYPE_N; >> - static const mask xdigit = _CTYPE_N | _CTYPE_X; >> - static const mask space = _CTYPE_S; >> - static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | >> _CTYPE_N | _CTYPE_B; >> - static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | >> _CTYPE_N; >> - static const mask cntrl = _CTYPE_C; >> - static const mask punct = _CTYPE_P; >> - static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >> + typedef const unsigned short* __to_type; >> + >> + typedef unsigned short mask; > > I seem to recall looking at this previously and noting that the change > to ctype_base::mask is an ABI break. It means that code compiled with > old versions of GCC or on old versions of NetBSD will not be ABI > compatible with code compiled by a new GCC on a new version of NetBSD. > > If the NetBSD maintainers are OK with that, then we can go ahead and > change it. > > We are fine with ABI breaks as we bump libstdc++ major on each upgrade in base.
On 06.01.2020 16:34, Jonathan Wakely wrote: > On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >> Hi Matthew, >> >> On Mon, 4 Feb 2019, Matthew Bauer wrote: >>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >>> have changed their ctype.h definition. It was updated in their intree >>> libstdc++-v3 but not in the GCC one. My understanding is this is a >>> straightforward rewrite. I've attached my own patch, but the file can >>> be obtained directly here: >>> >>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >>> >>> >>> With the attached patch, libstdc++-v3 can succesfully be built with >>> NetBSD headers (along with --disable-libcilkrts). >> >> I noticed this has not been applied yet, nor seen a follow-up?, and also >> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. >> >> Let me re-address this to libstdc++@gcc.gnu.org in the hope the >> maintainers there will have a look. >> >> Gerald > >> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >> index ff3ec893974..21eccf9fde1 100644 >> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> /// @brief Base class for ctype. >> struct ctype_base >> { >> - // Non-standard typedefs. >> - typedef const unsigned char* __to_type; >> >> // NB: Offsets into ctype<char>::_M_table force a particular size >> // on the mask type. Because of this, we don't use an enum. >> - typedef unsigned char mask; >> >> #ifndef _CTYPE_U >> - static const mask upper = _U; >> - static const mask lower = _L; >> - static const mask alpha = _U | _L; >> - static const mask digit = _N; >> - static const mask xdigit = _N | _X; >> - static const mask space = _S; >> - static const mask print = _P | _U | _L | _N | _B; >> - static const mask graph = _P | _U | _L | _N; >> - static const mask cntrl = _C; >> - static const mask punct = _P; >> - static const mask alnum = _U | _L | _N; >> + // Non-standard typedefs. >> + typedef const unsigned char* __to_type; >> + >> + typedef unsigned char mask; >> + >> + static const mask upper = _U; >> + static const mask lower = _L; >> + static const mask alpha = _U | _L; >> + static const mask digit = _N; >> + static const mask xdigit = _N | _X; >> + static const mask space = _S; >> + static const mask print = _P | _U | _L | _N | _B; >> + static const mask graph = _P | _U | _L | _N; >> + static const mask cntrl = _C; >> + static const mask punct = _P; >> + static const mask alnum = _U | _L | _N; >> #else >> - static const mask upper = _CTYPE_U; >> - static const mask lower = _CTYPE_L; >> - static const mask alpha = _CTYPE_U | _CTYPE_L; >> - static const mask digit = _CTYPE_N; >> - static const mask xdigit = _CTYPE_N | _CTYPE_X; >> - static const mask space = _CTYPE_S; >> - static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | >> _CTYPE_N | _CTYPE_B; >> - static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | >> _CTYPE_N; >> - static const mask cntrl = _CTYPE_C; >> - static const mask punct = _CTYPE_P; >> - static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >> + typedef const unsigned short* __to_type; >> + >> + typedef unsigned short mask; >> + >> + static const mask upper = _CTYPE_U; >> + static const mask lower = _CTYPE_L; >> + static const mask alpha = _CTYPE_A; >> + static const mask digit = _CTYPE_D; >> + static const mask xdigit = _CTYPE_X; >> + static const mask space = _CTYPE_S; >> + static const mask print = _CTYPE_R; >> + static const mask graph = _CTYPE_G; >> + static const mask cntrl = _CTYPE_C; >> + static const mask punct = _CTYPE_P; >> + static const mask alnum = _CTYPE_A | _CTYPE_D; >> #endif >> #if __cplusplus >= 201103L >> - static const mask blank = space; >> + static const mask blank = space; >> #endif >> }; >> >> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >> b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >> index ed3b7cd0d6a..33358e8f5d8 100644 >> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >> @@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> >> // Information as gleaned from /usr/include/ctype.h >> >> - extern "C" const u_int8_t _C_ctype_[]; >> - >> const ctype_base::mask* >> ctype<char>::classic_table() throw() >> - { return _C_ctype_ + 1; } >> + { return _C_ctype_tab_ + 1; } > > The first patch attached to PR 64271 (i.e. > https://gcc.gnu.org/bugzilla/attachment.cgi?id=34254 ) does that > differently. Is it safe to make this change unconditionally? > This file is NetBSD only (at least in theory) and we likely can do the right think without conditional build here. > Who authored these patches? We don't seem to have a changelog, not > even an author's name and email address, as required for any patch. > > Here was my previous review, where I mentioned the ABI break: > https://gcc.gnu.org/ml/libstdc++/2014-12/msg00069.html > I didn't get a reply. > Adding Maya, who can hopefully shed some light on it.
On 06/01/20 23:20 +0100, Kamil Rytarowski wrote: >On 06.01.2020 16:24, Jonathan Wakely wrote: >> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >>> Hi Matthew, >>> >>> On Mon, 4 Feb 2019, Matthew Bauer wrote: >>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >>>> have changed their ctype.h definition. It was updated in their intree >>>> libstdc++-v3 but not in the GCC one. My understanding is this is a >>>> straightforward rewrite. I've attached my own patch, but the file can >>>> be obtained directly here: >>>> >>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >>>> >>>> >>>> With the attached patch, libstdc++-v3 can succesfully be built with >>>> NetBSD headers (along with --disable-libcilkrts). >>> >>> I noticed this has not been applied yet, nor seen a follow-up?, and also >>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. >>> >>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the >>> maintainers there will have a look. >>> >>> Gerald >> >>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>> index ff3ec893974..21eccf9fde1 100644 >>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>> /// @brief Base class for ctype. >>> struct ctype_base >>> { >>> - // Non-standard typedefs. >>> - typedef const unsigned char* __to_type; >>> >>> // NB: Offsets into ctype<char>::_M_table force a particular size >>> // on the mask type. Because of this, we don't use an enum. >>> - typedef unsigned char mask; >>> >>> #ifndef _CTYPE_U >>> - static const mask upper = _U; >>> - static const mask lower = _L; >>> - static const mask alpha = _U | _L; >>> - static const mask digit = _N; >>> - static const mask xdigit = _N | _X; >>> - static const mask space = _S; >>> - static const mask print = _P | _U | _L | _N | _B; >>> - static const mask graph = _P | _U | _L | _N; >>> - static const mask cntrl = _C; >>> - static const mask punct = _P; >>> - static const mask alnum = _U | _L | _N; >>> + // Non-standard typedefs. >>> + typedef const unsigned char* __to_type; >>> + >>> + typedef unsigned char mask; >>> + >>> + static const mask upper = _U; >>> + static const mask lower = _L; >>> + static const mask alpha = _U | _L; >>> + static const mask digit = _N; >>> + static const mask xdigit = _N | _X; >>> + static const mask space = _S; >>> + static const mask print = _P | _U | _L | _N | _B; >>> + static const mask graph = _P | _U | _L | _N; >>> + static const mask cntrl = _C; >>> + static const mask punct = _P; >>> + static const mask alnum = _U | _L | _N; >>> #else >>> - static const mask upper = _CTYPE_U; >>> - static const mask lower = _CTYPE_L; >>> - static const mask alpha = _CTYPE_U | _CTYPE_L; >>> - static const mask digit = _CTYPE_N; >>> - static const mask xdigit = _CTYPE_N | _CTYPE_X; >>> - static const mask space = _CTYPE_S; >>> - static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>> _CTYPE_N | _CTYPE_B; >>> - static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>> _CTYPE_N; >>> - static const mask cntrl = _CTYPE_C; >>> - static const mask punct = _CTYPE_P; >>> - static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >>> + typedef const unsigned short* __to_type; >>> + >>> + typedef unsigned short mask; >> >> I seem to recall looking at this previously and noting that the change >> to ctype_base::mask is an ABI break. It means that code compiled with >> old versions of GCC or on old versions of NetBSD will not be ABI >> compatible with code compiled by a new GCC on a new version of NetBSD. >> >> If the NetBSD maintainers are OK with that, then we can go ahead and >> change it. >> >> > >We are fine with ABI breaks as we bump libstdc++ major on each upgrade >in base. That affects the libstdc++ built from NetBSD ports collection, but if somebody builds GCC themselves using the upstream sources (which is what these patches are trying to change) then they don't get that bumped major version. So if somebody builds GCC 9 themselves, and then builds GCC 10 themselves on the same machine, they'll get two versions of libstdc++.so that are not ABI compatible.
On 07/01/20 00:18 +0100, Kamil Rytarowski wrote: >On 06.01.2020 16:34, Jonathan Wakely wrote: >> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >>> Hi Matthew, >>> >>> On Mon, 4 Feb 2019, Matthew Bauer wrote: >>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >>>> have changed their ctype.h definition. It was updated in their intree >>>> libstdc++-v3 but not in the GCC one. My understanding is this is a >>>> straightforward rewrite. I've attached my own patch, but the file can >>>> be obtained directly here: >>>> >>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >>>> >>>> >>>> With the attached patch, libstdc++-v3 can succesfully be built with >>>> NetBSD headers (along with --disable-libcilkrts). >>> >>> I noticed this has not been applied yet, nor seen a follow-up?, and also >>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. >>> >>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the >>> maintainers there will have a look. >>> >>> Gerald >> >>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>> index ff3ec893974..21eccf9fde1 100644 >>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>> /// @brief Base class for ctype. >>> struct ctype_base >>> { >>> - // Non-standard typedefs. >>> - typedef const unsigned char* __to_type; >>> >>> // NB: Offsets into ctype<char>::_M_table force a particular size >>> // on the mask type. Because of this, we don't use an enum. >>> - typedef unsigned char mask; >>> >>> #ifndef _CTYPE_U >>> - static const mask upper = _U; >>> - static const mask lower = _L; >>> - static const mask alpha = _U | _L; >>> - static const mask digit = _N; >>> - static const mask xdigit = _N | _X; >>> - static const mask space = _S; >>> - static const mask print = _P | _U | _L | _N | _B; >>> - static const mask graph = _P | _U | _L | _N; >>> - static const mask cntrl = _C; >>> - static const mask punct = _P; >>> - static const mask alnum = _U | _L | _N; >>> + // Non-standard typedefs. >>> + typedef const unsigned char* __to_type; >>> + >>> + typedef unsigned char mask; >>> + >>> + static const mask upper = _U; >>> + static const mask lower = _L; >>> + static const mask alpha = _U | _L; >>> + static const mask digit = _N; >>> + static const mask xdigit = _N | _X; >>> + static const mask space = _S; >>> + static const mask print = _P | _U | _L | _N | _B; >>> + static const mask graph = _P | _U | _L | _N; >>> + static const mask cntrl = _C; >>> + static const mask punct = _P; >>> + static const mask alnum = _U | _L | _N; >>> #else >>> - static const mask upper = _CTYPE_U; >>> - static const mask lower = _CTYPE_L; >>> - static const mask alpha = _CTYPE_U | _CTYPE_L; >>> - static const mask digit = _CTYPE_N; >>> - static const mask xdigit = _CTYPE_N | _CTYPE_X; >>> - static const mask space = _CTYPE_S; >>> - static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>> _CTYPE_N | _CTYPE_B; >>> - static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>> _CTYPE_N; >>> - static const mask cntrl = _CTYPE_C; >>> - static const mask punct = _CTYPE_P; >>> - static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >>> + typedef const unsigned short* __to_type; >>> + >>> + typedef unsigned short mask; >>> + >>> + static const mask upper = _CTYPE_U; >>> + static const mask lower = _CTYPE_L; >>> + static const mask alpha = _CTYPE_A; >>> + static const mask digit = _CTYPE_D; >>> + static const mask xdigit = _CTYPE_X; >>> + static const mask space = _CTYPE_S; >>> + static const mask print = _CTYPE_R; >>> + static const mask graph = _CTYPE_G; >>> + static const mask cntrl = _CTYPE_C; >>> + static const mask punct = _CTYPE_P; >>> + static const mask alnum = _CTYPE_A | _CTYPE_D; >>> #endif >>> #if __cplusplus >= 201103L >>> - static const mask blank = space; >>> + static const mask blank = space; >>> #endif >>> }; >>> >>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >>> index ed3b7cd0d6a..33358e8f5d8 100644 >>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >>> @@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>> >>> // Information as gleaned from /usr/include/ctype.h >>> >>> - extern "C" const u_int8_t _C_ctype_[]; >>> - >>> const ctype_base::mask* >>> ctype<char>::classic_table() throw() >>> - { return _C_ctype_ + 1; } >>> + { return _C_ctype_tab_ + 1; } >> >> The first patch attached to PR 64271 (i.e. >> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34254 ) does that >> differently. Is it safe to make this change unconditionally? >> > >This file is NetBSD only (at least in theory) and we likely can do the >right think without conditional build here. Yes, it's NetBSD-specific, but it's used for multiple versions of NetBSD. The patch in PR 64271 only uses _C_ctype_tab_+1 when _CTYPE_BL is defined, i.e. for newer versions of NetBSD. For older versions, it still uses _C_ctype_. So my question is whether _C_ctype_tab_+1 does the right thing for all versions of NetBSD, or only for newer ones.
On 07.01.2020 01:26, Jonathan Wakely wrote: > On 06/01/20 23:20 +0100, Kamil Rytarowski wrote: >> On 06.01.2020 16:24, Jonathan Wakely wrote: >>> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >>>> Hi Matthew, >>>> >>>> On Mon, 4 Feb 2019, Matthew Bauer wrote: >>>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >>>>> have changed their ctype.h definition. It was updated in their intree >>>>> libstdc++-v3 but not in the GCC one. My understanding is this is a >>>>> straightforward rewrite. I've attached my own patch, but the file can >>>>> be obtained directly here: >>>>> >>>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >>>>> >>>>> >>>>> >>>>> With the attached patch, libstdc++-v3 can succesfully be built with >>>>> NetBSD headers (along with --disable-libcilkrts). >>>> >>>> I noticed this has not been applied yet, nor seen a follow-up?, and >>>> also >>>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. >>>> >>>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the >>>> maintainers there will have a look. >>>> >>>> Gerald >>> >>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>> index ff3ec893974..21eccf9fde1 100644 >>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>> /// @brief Base class for ctype. >>>> struct ctype_base >>>> { >>>> - // Non-standard typedefs. >>>> - typedef const unsigned char* __to_type; >>>> >>>> // NB: Offsets into ctype<char>::_M_table force a particular size >>>> // on the mask type. Because of this, we don't use an enum. >>>> - typedef unsigned char mask; >>>> >>>> #ifndef _CTYPE_U >>>> - static const mask upper = _U; >>>> - static const mask lower = _L; >>>> - static const mask alpha = _U | _L; >>>> - static const mask digit = _N; >>>> - static const mask xdigit = _N | _X; >>>> - static const mask space = _S; >>>> - static const mask print = _P | _U | _L | _N | _B; >>>> - static const mask graph = _P | _U | _L | _N; >>>> - static const mask cntrl = _C; >>>> - static const mask punct = _P; >>>> - static const mask alnum = _U | _L | _N; >>>> + // Non-standard typedefs. >>>> + typedef const unsigned char* __to_type; >>>> + >>>> + typedef unsigned char mask; >>>> + >>>> + static const mask upper = _U; >>>> + static const mask lower = _L; >>>> + static const mask alpha = _U | _L; >>>> + static const mask digit = _N; >>>> + static const mask xdigit = _N | _X; >>>> + static const mask space = _S; >>>> + static const mask print = _P | _U | _L | _N | _B; >>>> + static const mask graph = _P | _U | _L | _N; >>>> + static const mask cntrl = _C; >>>> + static const mask punct = _P; >>>> + static const mask alnum = _U | _L | _N; >>>> #else >>>> - static const mask upper = _CTYPE_U; >>>> - static const mask lower = _CTYPE_L; >>>> - static const mask alpha = _CTYPE_U | _CTYPE_L; >>>> - static const mask digit = _CTYPE_N; >>>> - static const mask xdigit = _CTYPE_N | _CTYPE_X; >>>> - static const mask space = _CTYPE_S; >>>> - static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>>> _CTYPE_N | _CTYPE_B; >>>> - static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>>> _CTYPE_N; >>>> - static const mask cntrl = _CTYPE_C; >>>> - static const mask punct = _CTYPE_P; >>>> - static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >>>> + typedef const unsigned short* __to_type; >>>> + >>>> + typedef unsigned short mask; >>> >>> I seem to recall looking at this previously and noting that the change >>> to ctype_base::mask is an ABI break. It means that code compiled with >>> old versions of GCC or on old versions of NetBSD will not be ABI >>> compatible with code compiled by a new GCC on a new version of NetBSD. >>> >>> If the NetBSD maintainers are OK with that, then we can go ahead and >>> change it. >>> >>> >> >> We are fine with ABI breaks as we bump libstdc++ major on each upgrade >> in base. > > That affects the libstdc++ built from NetBSD ports collection, but if > somebody builds GCC themselves using the upstream sources (which is > what these patches are trying to change) then they don't get that > bumped major version. > > So if somebody builds GCC 9 themselves, and then builds GCC 10 > themselves on the same machine, they'll get two versions of > libstdc++.so that are not ABI compatible. > > I know, but we don't support this in NetBSD. We ship with our own major numbers for GCC. If someone wants to maintain out of base or out of pkgsrc it will have likely more problems than ABI breakage.
On 07.01.2020 01:28, Jonathan Wakely wrote: > On 07/01/20 00:18 +0100, Kamil Rytarowski wrote: >> On 06.01.2020 16:34, Jonathan Wakely wrote: >>> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >>>> Hi Matthew, >>>> >>>> On Mon, 4 Feb 2019, Matthew Bauer wrote: >>>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >>>>> have changed their ctype.h definition. It was updated in their intree >>>>> libstdc++-v3 but not in the GCC one. My understanding is this is a >>>>> straightforward rewrite. I've attached my own patch, but the file can >>>>> be obtained directly here: >>>>> >>>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >>>>> >>>>> >>>>> >>>>> With the attached patch, libstdc++-v3 can succesfully be built with >>>>> NetBSD headers (along with --disable-libcilkrts). >>>> >>>> I noticed this has not been applied yet, nor seen a follow-up?, and >>>> also >>>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. >>>> >>>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the >>>> maintainers there will have a look. >>>> >>>> Gerald >>> >>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>> index ff3ec893974..21eccf9fde1 100644 >>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>> /// @brief Base class for ctype. >>>> struct ctype_base >>>> { >>>> - // Non-standard typedefs. >>>> - typedef const unsigned char* __to_type; >>>> >>>> // NB: Offsets into ctype<char>::_M_table force a particular size >>>> // on the mask type. Because of this, we don't use an enum. >>>> - typedef unsigned char mask; >>>> >>>> #ifndef _CTYPE_U >>>> - static const mask upper = _U; >>>> - static const mask lower = _L; >>>> - static const mask alpha = _U | _L; >>>> - static const mask digit = _N; >>>> - static const mask xdigit = _N | _X; >>>> - static const mask space = _S; >>>> - static const mask print = _P | _U | _L | _N | _B; >>>> - static const mask graph = _P | _U | _L | _N; >>>> - static const mask cntrl = _C; >>>> - static const mask punct = _P; >>>> - static const mask alnum = _U | _L | _N; >>>> + // Non-standard typedefs. >>>> + typedef const unsigned char* __to_type; >>>> + >>>> + typedef unsigned char mask; >>>> + >>>> + static const mask upper = _U; >>>> + static const mask lower = _L; >>>> + static const mask alpha = _U | _L; >>>> + static const mask digit = _N; >>>> + static const mask xdigit = _N | _X; >>>> + static const mask space = _S; >>>> + static const mask print = _P | _U | _L | _N | _B; >>>> + static const mask graph = _P | _U | _L | _N; >>>> + static const mask cntrl = _C; >>>> + static const mask punct = _P; >>>> + static const mask alnum = _U | _L | _N; >>>> #else >>>> - static const mask upper = _CTYPE_U; >>>> - static const mask lower = _CTYPE_L; >>>> - static const mask alpha = _CTYPE_U | _CTYPE_L; >>>> - static const mask digit = _CTYPE_N; >>>> - static const mask xdigit = _CTYPE_N | _CTYPE_X; >>>> - static const mask space = _CTYPE_S; >>>> - static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>>> _CTYPE_N | _CTYPE_B; >>>> - static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>>> _CTYPE_N; >>>> - static const mask cntrl = _CTYPE_C; >>>> - static const mask punct = _CTYPE_P; >>>> - static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >>>> + typedef const unsigned short* __to_type; >>>> + >>>> + typedef unsigned short mask; >>>> + >>>> + static const mask upper = _CTYPE_U; >>>> + static const mask lower = _CTYPE_L; >>>> + static const mask alpha = _CTYPE_A; >>>> + static const mask digit = _CTYPE_D; >>>> + static const mask xdigit = _CTYPE_X; >>>> + static const mask space = _CTYPE_S; >>>> + static const mask print = _CTYPE_R; >>>> + static const mask graph = _CTYPE_G; >>>> + static const mask cntrl = _CTYPE_C; >>>> + static const mask punct = _CTYPE_P; >>>> + static const mask alnum = _CTYPE_A | _CTYPE_D; >>>> #endif >>>> #if __cplusplus >= 201103L >>>> - static const mask blank = space; >>>> + static const mask blank = space; >>>> #endif >>>> }; >>>> >>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >>>> index ed3b7cd0d6a..33358e8f5d8 100644 >>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc >>>> @@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>> >>>> // Information as gleaned from /usr/include/ctype.h >>>> >>>> - extern "C" const u_int8_t _C_ctype_[]; >>>> - >>>> const ctype_base::mask* >>>> ctype<char>::classic_table() throw() >>>> - { return _C_ctype_ + 1; } >>>> + { return _C_ctype_tab_ + 1; } >>> >>> The first patch attached to PR 64271 (i.e. >>> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34254 ) does that >>> differently. Is it safe to make this change unconditionally? >>> >> >> This file is NetBSD only (at least in theory) and we likely can do the >> right think without conditional build here. > > Yes, it's NetBSD-specific, but it's used for multiple versions of > NetBSD. This change was introduced in 2013 and we do not support such old snapshots. If GCC wants to support older releases there it would be futile. > > The patch in PR 64271 only uses _C_ctype_tab_+1 when _CTYPE_BL is > defined, i.e. for newer versions of NetBSD. For older versions, it > still uses _C_ctype_. > > So my question is whether _C_ctype_tab_+1 does the right thing for all > versions of NetBSD, or only for newer ones. > The right thing today is to go unconditionally for _C_ctype_tab_ and abandon _C_ctype_ entirely. _C_ctype_ was u8 and did not support all inline semantics. We still keep _C_ctype_ binary compat for the time being (until next libc major bump), but it will go away in future (no timeline when).
On 07/01/20 01:30 +0100, Kamil Rytarowski wrote: >On 07.01.2020 01:26, Jonathan Wakely wrote: >> On 06/01/20 23:20 +0100, Kamil Rytarowski wrote: >>> On 06.01.2020 16:24, Jonathan Wakely wrote: >>>> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >>>>> Hi Matthew, >>>>> >>>>> On Mon, 4 Feb 2019, Matthew Bauer wrote: >>>>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >>>>>> have changed their ctype.h definition. It was updated in their intree >>>>>> libstdc++-v3 but not in the GCC one. My understanding is this is a >>>>>> straightforward rewrite. I've attached my own patch, but the file can >>>>>> be obtained directly here: >>>>>> >>>>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >>>>>> >>>>>> >>>>>> >>>>>> With the attached patch, libstdc++-v3 can succesfully be built with >>>>>> NetBSD headers (along with --disable-libcilkrts). >>>>> >>>>> I noticed this has not been applied yet, nor seen a follow-up?, and >>>>> also >>>>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. >>>>> >>>>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the >>>>> maintainers there will have a look. >>>>> >>>>> Gerald >>>> >>>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>>> index ff3ec893974..21eccf9fde1 100644 >>>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>> /// @brief Base class for ctype. >>>>> struct ctype_base >>>>> { >>>>> - // Non-standard typedefs. >>>>> - typedef const unsigned char* __to_type; >>>>> >>>>> // NB: Offsets into ctype<char>::_M_table force a particular size >>>>> // on the mask type. Because of this, we don't use an enum. >>>>> - typedef unsigned char mask; >>>>> >>>>> #ifndef _CTYPE_U >>>>> - static const mask upper = _U; >>>>> - static const mask lower = _L; >>>>> - static const mask alpha = _U | _L; >>>>> - static const mask digit = _N; >>>>> - static const mask xdigit = _N | _X; >>>>> - static const mask space = _S; >>>>> - static const mask print = _P | _U | _L | _N | _B; >>>>> - static const mask graph = _P | _U | _L | _N; >>>>> - static const mask cntrl = _C; >>>>> - static const mask punct = _P; >>>>> - static const mask alnum = _U | _L | _N; >>>>> + // Non-standard typedefs. >>>>> + typedef const unsigned char* __to_type; >>>>> + >>>>> + typedef unsigned char mask; >>>>> + >>>>> + static const mask upper = _U; >>>>> + static const mask lower = _L; >>>>> + static const mask alpha = _U | _L; >>>>> + static const mask digit = _N; >>>>> + static const mask xdigit = _N | _X; >>>>> + static const mask space = _S; >>>>> + static const mask print = _P | _U | _L | _N | _B; >>>>> + static const mask graph = _P | _U | _L | _N; >>>>> + static const mask cntrl = _C; >>>>> + static const mask punct = _P; >>>>> + static const mask alnum = _U | _L | _N; >>>>> #else >>>>> - static const mask upper = _CTYPE_U; >>>>> - static const mask lower = _CTYPE_L; >>>>> - static const mask alpha = _CTYPE_U | _CTYPE_L; >>>>> - static const mask digit = _CTYPE_N; >>>>> - static const mask xdigit = _CTYPE_N | _CTYPE_X; >>>>> - static const mask space = _CTYPE_S; >>>>> - static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>>>> _CTYPE_N | _CTYPE_B; >>>>> - static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>>>> _CTYPE_N; >>>>> - static const mask cntrl = _CTYPE_C; >>>>> - static const mask punct = _CTYPE_P; >>>>> - static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >>>>> + typedef const unsigned short* __to_type; >>>>> + >>>>> + typedef unsigned short mask; >>>> >>>> I seem to recall looking at this previously and noting that the change >>>> to ctype_base::mask is an ABI break. It means that code compiled with >>>> old versions of GCC or on old versions of NetBSD will not be ABI >>>> compatible with code compiled by a new GCC on a new version of NetBSD. >>>> >>>> If the NetBSD maintainers are OK with that, then we can go ahead and >>>> change it. >>>> >>>> >>> >>> We are fine with ABI breaks as we bump libstdc++ major on each upgrade >>> in base. >> >> That affects the libstdc++ built from NetBSD ports collection, but if >> somebody builds GCC themselves using the upstream sources (which is >> what these patches are trying to change) then they don't get that >> bumped major version. >> >> So if somebody builds GCC 9 themselves, and then builds GCC 10 >> themselves on the same machine, they'll get two versions of >> libstdc++.so that are not ABI compatible. >> >> > >I know, but we don't support this in NetBSD. We ship with our own major >numbers for GCC. If someone wants to maintain out of base or out of >pkgsrc it will have likely more problems than ABI breakage. But that's exactly what upstream GCC sources are (out of pkgsrc), and that's where I'm being asked to include this patch. NetBSD is free to not support it, of course, but we (GCC) have different schedules and different priorities. And if older versions of NetBSD are not supported, can we just drop the `#ifndef _CTYPE_U` branch in config/os/bsd/netbsd/ctype_base.h ? It seems wrong to keep support for old systems in one file and drop it in another file. I think we need the netbsd target maintainers (CC'd) to decide whether GCC should still support older releases or drop that support for GCC 10. Usually I'd say we need a period of deprecation, but if GCC doesn't currently build on NetBSD then maybe that's unnecessary.
On 07/01/20 15:40 +0000, Jonathan Wakely wrote: >On 07/01/20 01:30 +0100, Kamil Rytarowski wrote: >>On 07.01.2020 01:26, Jonathan Wakely wrote: >>>On 06/01/20 23:20 +0100, Kamil Rytarowski wrote: >>>>On 06.01.2020 16:24, Jonathan Wakely wrote: >>>>>On 22/12/19 09:36 +1000, Gerald Pfeifer wrote: >>>>>>Hi Matthew, >>>>>> >>>>>>On Mon, 4 Feb 2019, Matthew Bauer wrote: >>>>>>>The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They >>>>>>>have changed their ctype.h definition. It was updated in their intree >>>>>>>libstdc++-v3 but not in the GCC one. My understanding is this is a >>>>>>>straightforward rewrite. I've attached my own patch, but the file can >>>>>>>be obtained directly here: >>>>>>> >>>>>>>http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h >>>>>>> >>>>>>> >>>>>>> >>>>>>>With the attached patch, libstdc++-v3 can succesfully be built with >>>>>>>NetBSD headers (along with --disable-libcilkrts). >>>>>> >>>>>>I noticed this has not been applied yet, nor seen a follow-up?, and >>>>>>also >>>>>>noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org. >>>>>> >>>>>>Let me re-address this to libstdc++@gcc.gnu.org in the hope the >>>>>>maintainers there will have a look. >>>>>> >>>>>>Gerald >>>>> >>>>>>diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>>>>b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>>>>index ff3ec893974..21eccf9fde1 100644 >>>>>>--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>>>>+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h >>>>>>@@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>>> /// @brief Base class for ctype. >>>>>> struct ctype_base >>>>>> { >>>>>>- // Non-standard typedefs. >>>>>>- typedef const unsigned char* __to_type; >>>>>> >>>>>> // NB: Offsets into ctype<char>::_M_table force a particular size >>>>>> // on the mask type. Because of this, we don't use an enum. >>>>>>- typedef unsigned char mask; >>>>>> >>>>>>#ifndef _CTYPE_U >>>>>>- static const mask upper = _U; >>>>>>- static const mask lower = _L; >>>>>>- static const mask alpha = _U | _L; >>>>>>- static const mask digit = _N; >>>>>>- static const mask xdigit = _N | _X; >>>>>>- static const mask space = _S; >>>>>>- static const mask print = _P | _U | _L | _N | _B; >>>>>>- static const mask graph = _P | _U | _L | _N; >>>>>>- static const mask cntrl = _C; >>>>>>- static const mask punct = _P; >>>>>>- static const mask alnum = _U | _L | _N; >>>>>>+ // Non-standard typedefs. >>>>>>+ typedef const unsigned char* __to_type; >>>>>>+ >>>>>>+ typedef unsigned char mask; >>>>>>+ >>>>>>+ static const mask upper = _U; >>>>>>+ static const mask lower = _L; >>>>>>+ static const mask alpha = _U | _L; >>>>>>+ static const mask digit = _N; >>>>>>+ static const mask xdigit = _N | _X; >>>>>>+ static const mask space = _S; >>>>>>+ static const mask print = _P | _U | _L | _N | _B; >>>>>>+ static const mask graph = _P | _U | _L | _N; >>>>>>+ static const mask cntrl = _C; >>>>>>+ static const mask punct = _P; >>>>>>+ static const mask alnum = _U | _L | _N; >>>>>>#else >>>>>>- static const mask upper = _CTYPE_U; >>>>>>- static const mask lower = _CTYPE_L; >>>>>>- static const mask alpha = _CTYPE_U | _CTYPE_L; >>>>>>- static const mask digit = _CTYPE_N; >>>>>>- static const mask xdigit = _CTYPE_N | _CTYPE_X; >>>>>>- static const mask space = _CTYPE_S; >>>>>>- static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>>>>>_CTYPE_N | _CTYPE_B; >>>>>>- static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | >>>>>>_CTYPE_N; >>>>>>- static const mask cntrl = _CTYPE_C; >>>>>>- static const mask punct = _CTYPE_P; >>>>>>- static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; >>>>>>+ typedef const unsigned short* __to_type; >>>>>>+ >>>>>>+ typedef unsigned short mask; >>>>> >>>>>I seem to recall looking at this previously and noting that the change >>>>>to ctype_base::mask is an ABI break. It means that code compiled with >>>>>old versions of GCC or on old versions of NetBSD will not be ABI >>>>>compatible with code compiled by a new GCC on a new version of NetBSD. >>>>> >>>>>If the NetBSD maintainers are OK with that, then we can go ahead and >>>>>change it. >>>>> >>>>> >>>> >>>>We are fine with ABI breaks as we bump libstdc++ major on each upgrade >>>>in base. >>> >>>That affects the libstdc++ built from NetBSD ports collection, but if >>>somebody builds GCC themselves using the upstream sources (which is >>>what these patches are trying to change) then they don't get that >>>bumped major version. >>> >>>So if somebody builds GCC 9 themselves, and then builds GCC 10 >>>themselves on the same machine, they'll get two versions of >>>libstdc++.so that are not ABI compatible. >>> >>> >> >>I know, but we don't support this in NetBSD. We ship with our own major >>numbers for GCC. If someone wants to maintain out of base or out of >>pkgsrc it will have likely more problems than ABI breakage. > >But that's exactly what upstream GCC sources are (out of pkgsrc), and >that's where I'm being asked to include this patch. > >NetBSD is free to not support it, of course, but we (GCC) have >different schedules and different priorities. > >And if older versions of NetBSD are not supported, can we just drop >the `#ifndef _CTYPE_U` branch in config/os/bsd/netbsd/ctype_base.h ? > >It seems wrong to keep support for old systems in one file and drop it >in another file. For Jason and Krister's benefit, that last comment was referring to an earlier suggestion to not try to support old NetBSD releases, see https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html >I think we need the netbsd target maintainers (CC'd) to decide whether >GCC should still support older releases or drop that support for GCC >10. Usually I'd say we need a period of deprecation, but if GCC >doesn't currently build on NetBSD then maybe that's unnecessary. > > >
> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote: > > For Jason and Krister's benefit, that last comment was referring to > an earlier suggestion to not try to support old NetBSD releases, see > https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html > >> I think we need the netbsd target maintainers (CC'd) to decide whether >> GCC should still support older releases or drop that support for GCC >> 10. Usually I'd say we need a period of deprecation, but if GCC >> doesn't currently build on NetBSD then maybe that's unnecessary. The affected NetBSD versions are NetBSD 6 and earlier, which are EOL from the NetBSD perspective, so I think this is OK. -- thorpej
On 07/01/20 12:44 -0800, Jason Thorpe wrote: > >> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote: >> >> For Jason and Krister's benefit, that last comment was referring to >> an earlier suggestion to not try to support old NetBSD releases, see >> https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html >> >>> I think we need the netbsd target maintainers (CC'd) to decide whether >>> GCC should still support older releases or drop that support for GCC >>> 10. Usually I'd say we need a period of deprecation, but if GCC >>> doesn't currently build on NetBSD then maybe that's unnecessary. > >The affected NetBSD versions are NetBSD 6 and earlier, which are EOL from the NetBSD perspective, so I think this is OK. So is this patch OK then? Could somebody please test it on NetBSD? (Ideally on the oldest supported release, but anything is better than nothing). This differs from the patches posted by using _CTYPE_BL for the isblank class, which seems better than using _CTYPE_S.
On 10.01.2020 17:11, Jonathan Wakely wrote: > On 07/01/20 12:44 -0800, Jason Thorpe wrote: >> >>> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote: >>> >>> For Jason and Krister's benefit, that last comment was referring to >>> an earlier suggestion to not try to support old NetBSD releases, see >>> https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html >>> >>>> I think we need the netbsd target maintainers (CC'd) to decide whether >>>> GCC should still support older releases or drop that support for GCC >>>> 10. Usually I'd say we need a period of deprecation, but if GCC >>>> doesn't currently build on NetBSD then maybe that's unnecessary. >> >> The affected NetBSD versions are NetBSD 6 and earlier, which are EOL >> from the NetBSD perspective, so I think this is OK. > > So is this patch OK then? > Looks good to me. > Could somebody please test it on NetBSD? (Ideally on the oldest > supported release, but anything is better than nothing). > Works for me on: $ uname -a NetBSD chieftec 9.99.37 NetBSD 9.99.37 (GENERIC) #5: Mon Jan 13 15:39:58 CET 2020 root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64 I see no reason why would it break on older releases, but I don't have them handy for tests. > This differs from the patches posted by using _CTYPE_BL for the > isblank class, which seems better than using _CTYPE_S. > > _CTYPE_BL is the right bit for isblank().
> On Jan 14, 2020, at 8:49 AM, Kamil Rytarowski <n54@gmx.com> wrote: > > On 10.01.2020 17:11, Jonathan Wakely wrote: >> On 07/01/20 12:44 -0800, Jason Thorpe wrote: >>> >>>> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote: >>>> >>>> For Jason and Krister's benefit, that last comment was referring to >>>> an earlier suggestion to not try to support old NetBSD releases, see >>>> https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html >>>> >>>>> I think we need the netbsd target maintainers (CC'd) to decide whether >>>>> GCC should still support older releases or drop that support for GCC >>>>> 10. Usually I'd say we need a period of deprecation, but if GCC >>>>> doesn't currently build on NetBSD then maybe that's unnecessary. >>> >>> The affected NetBSD versions are NetBSD 6 and earlier, which are EOL >>> from the NetBSD perspective, so I think this is OK. >> >> So is this patch OK then? >> > > Looks good to me. Now that we have Kamil's confirmation that it's working for him, it also gets a thumbs-up from me. Thanks! > >> Could somebody please test it on NetBSD? (Ideally on the oldest >> supported release, but anything is better than nothing). >> > > Works for me on: > > $ uname -a > NetBSD chieftec 9.99.37 NetBSD 9.99.37 (GENERIC) #5: Mon Jan 13 15:39:58 > CET 2020 > root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64 > > I see no reason why would it break on older releases, but I don't have > them handy for tests. > >> This differs from the patches posted by using _CTYPE_BL for the >> isblank class, which seems better than using _CTYPE_S. >> >> > > _CTYPE_BL is the right bit for isblank(). > -- thorpej
On 14/01/20 08:56 -0800, Jason Thorpe wrote: > > >> On Jan 14, 2020, at 8:49 AM, Kamil Rytarowski <n54@gmx.com> wrote: >> >> On 10.01.2020 17:11, Jonathan Wakely wrote: >>> On 07/01/20 12:44 -0800, Jason Thorpe wrote: >>>> >>>>> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote: >>>>> >>>>> For Jason and Krister's benefit, that last comment was referring to >>>>> an earlier suggestion to not try to support old NetBSD releases, see >>>>> https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html >>>>> >>>>>> I think we need the netbsd target maintainers (CC'd) to decide whether >>>>>> GCC should still support older releases or drop that support for GCC >>>>>> 10. Usually I'd say we need a period of deprecation, but if GCC >>>>>> doesn't currently build on NetBSD then maybe that's unnecessary. >>>> >>>> The affected NetBSD versions are NetBSD 6 and earlier, which are EOL >>>> from the NetBSD perspective, so I think this is OK. >>> >>> So is this patch OK then? >>> >> >> Looks good to me. > >Now that we have Kamil's confirmation that it's working for him, it also gets a thumbs-up from me. > >Thanks! Thanks to everyone for their feedback. I've pushed the patch to GCC master now (I think it's reasonable to consider "doesn't build on NetBSD anymore" as a regression and so fixable at this point in GCC's development). Please let me know of any problems you find with the changes.
> On Jan 16, 2020, at 9:24 AM, Jonathan Wakely <jwakely@redhat.com> wrote: > > Thanks to everyone for their feedback. I've pushed the patch to GCC > master now (I think it's reasonable to consider "doesn't build on > NetBSD anymore" as a regression and so fixable at this point in GCC's > development). > > Please let me know of any problems you find with the changes. Great, thanks Jonathan! -- thorpej
diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h index ff3ec893974..21eccf9fde1 100644 --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// @brief Base class for ctype. struct ctype_base { - // Non-standard typedefs. - typedef const unsigned char* __to_type; // NB: Offsets into ctype<char>::_M_table force a particular size // on the mask type. Because of this, we don't use an enum. - typedef unsigned char mask; #ifndef _CTYPE_U - static const mask upper = _U; - static const mask lower = _L; - static const mask alpha = _U | _L; - static const mask digit = _N; - static const mask xdigit = _N | _X; - static const mask space = _S; - static const mask print = _P | _U | _L | _N | _B; - static const mask graph = _P | _U | _L | _N; - static const mask cntrl = _C; - static const mask punct = _P; - static const mask alnum = _U | _L | _N; + // Non-standard typedefs. + typedef const unsigned char* __to_type; + + typedef unsigned char mask; + + static const mask upper = _U; + static const mask lower = _L; + static const mask alpha = _U | _L; + static const mask digit = _N; + static const mask xdigit = _N | _X; + static const mask space = _S; + static const mask print = _P | _U | _L | _N | _B; + static const mask graph = _P | _U | _L | _N; + static const mask cntrl = _C; + static const mask punct = _P; + static const mask alnum = _U | _L | _N; #else - static const mask upper = _CTYPE_U; - static const mask lower = _CTYPE_L; - static const mask alpha = _CTYPE_U | _CTYPE_L; - static const mask digit = _CTYPE_N; - static const mask xdigit = _CTYPE_N | _CTYPE_X; - static const mask space = _CTYPE_S; - static const mask print = _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N | _CTYPE_B; - static const mask graph = _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N; - static const mask cntrl = _CTYPE_C; - static const mask punct = _CTYPE_P; - static const mask alnum = _CTYPE_U | _CTYPE_L | _CTYPE_N; + typedef const unsigned short* __to_type; + + typedef unsigned short mask; + + static const mask upper = _CTYPE_U; + static const mask lower = _CTYPE_L; + static const mask alpha = _CTYPE_A; + static const mask digit = _CTYPE_D; + static const mask xdigit = _CTYPE_X; + static const mask space = _CTYPE_S; + static const mask print = _CTYPE_R; + static const mask graph = _CTYPE_G; + static const mask cntrl = _CTYPE_C; + static const mask punct = _CTYPE_P; + static const mask alnum = _CTYPE_A | _CTYPE_D; #endif #if __cplusplus >= 201103L - static const mask blank = space; + static const mask blank = space; #endif }; diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc index ed3b7cd0d6a..33358e8f5d8 100644 --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc @@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Information as gleaned from /usr/include/ctype.h - extern "C" const u_int8_t _C_ctype_[]; - const ctype_base::mask* ctype<char>::classic_table() throw() - { return _C_ctype_ + 1; } + { return _C_ctype_tab_ + 1; } ctype<char>::ctype(__c_locale, const mask* __table, bool __del, size_t __refs) @@ -69,14 +67,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION char ctype<char>::do_toupper(char __c) const - { return ::toupper((int) __c); } + { return ::toupper((int)(unsigned char) __c); } const char* ctype<char>::do_toupper(char* __low, const char* __high) const { while (__low < __high) { - *__low = ::toupper((int) *__low); + *__low = ::toupper((int)(unsigned char) *__low); ++__low; } return __high; @@ -84,14 +82,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION char ctype<char>::do_tolower(char __c) const - { return ::tolower((int) __c); } + { return ::tolower((int)(unsigned char) __c); } const char* ctype<char>::do_tolower(char* __low, const char* __high) const { while (__low < __high) { - *__low = ::tolower((int) *__low); + *__low = ::tolower((int)(unsigned char) *__low); ++__low; } return __high; diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h index ace1120fba2..3234ce17c70 100644 --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h @@ -48,7 +48,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION is(const char* __low, const char* __high, mask* __vec) const { while (__low < __high) - *__vec++ = _M_table[*__low++]; + *__vec++ = _M_table[(unsigned char)*__low++]; return __high; }