libstdc++: Workaround glibc header on ia64-linux
Checks
Commit Message
We see:
```
FAIL: 17_intro/names.cc -std=gnu++17 (test for excess errors)
FAIL: 17_intro/names_pstl.cc -std=gnu++17 (test for excess errors)
FAIL: experimental/names.cc -std=gnu++17 (test for excess errors)
```
...on ia64-linux.
This is due to:
* /usr/include/bits/sigcontext.h:32-38:
```
32 struct __ia64_fpreg
33 {
34 union
35 {
36 unsigned long bits[2];
37 } u;
38 } __attribute__ ((__aligned__ (16)));
```
* /usr/include/sys/ucontext.h:39-45:
```
39 struct __ia64_fpreg_mcontext
40 {
41 union
42 {
43 unsigned long __ctx(bits)[2];
44 } __ctx(u);
45 } __attribute__ ((__aligned__ (16)));
```
...from glibc 2.39 (w/ia64 support re-added). See the discussion
starting on [1].
[1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654487.html
The following patch adds a workaround for this on the libstdc++
testsuite side.
Signed-off-by: Frank Scheiner <frank.scheiner@web.de>
---
libstdc++-v3/testsuite/17_intro/names.cc | 6 ++++++
1 file changed, 6 insertions(+)
--
2.45.2
Comments
On Mon, 30 Sept 2024 at 18:26, Frank Scheiner wrote:
>
> We see:
>
> ```
> FAIL: 17_intro/names.cc -std=gnu++17 (test for excess errors)
> FAIL: 17_intro/names_pstl.cc -std=gnu++17 (test for excess errors)
> FAIL: experimental/names.cc -std=gnu++17 (test for excess errors)
> ```
>
> ...on ia64-linux.
>
> This is due to:
>
> * /usr/include/bits/sigcontext.h:32-38:
> ```
> 32 struct __ia64_fpreg
> 33 {
> 34 union
> 35 {
> 36 unsigned long bits[2];
> 37 } u;
> 38 } __attribute__ ((__aligned__ (16)));
> ```
>
> * /usr/include/sys/ucontext.h:39-45:
> ```
> 39 struct __ia64_fpreg_mcontext
> 40 {
> 41 union
> 42 {
> 43 unsigned long __ctx(bits)[2];
> 44 } __ctx(u);
> 45 } __attribute__ ((__aligned__ (16)));
> ```
>
> ...from glibc 2.39 (w/ia64 support re-added). See the discussion
> starting on [1].
>
> [1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654487.html
>
> The following patch adds a workaround for this on the libstdc++
> testsuite side.
Thanks for the patch. Please CC libstdc++@gcc.gnu.org for all
libstdc++ patches, as per https://gcc.gnu.org/lists.html
It looks like the glibc header also defines "bits" without using the
implementation namespace. Is there a reason the re-added ia64 code in
the glibc header can't be fixed to use __bits and __u? Is there
software expecting to access those fields directly?
>
> Signed-off-by: Frank Scheiner <frank.scheiner@web.de>
> ---
> libstdc++-v3/testsuite/17_intro/names.cc | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/libstdc++-v3/testsuite/17_intro/names.cc
> b/libstdc++-v3/testsuite/17_intro/names.cc
> index 9b0ffcb50b2..b45aefe1ccf 100644
> --- a/libstdc++-v3/testsuite/17_intro/names.cc
> +++ b/libstdc++-v3/testsuite/17_intro/names.cc
> @@ -265,6 +265,12 @@
> #undef j
> #endif
>
> +#if defined (__linux__) && defined (__ia64__)
> +// <bits/sigcontext.h> defines __ia64_fpreg::u
> +// <sys/ucontext.h> defines __ia64_fpreg_mcontext::u
> +#undef u
> +#endif
> +
> #if defined (__linux__) && defined (__powerpc__)
> // <asm/types.h> defines __vector128::u
> #undef u
> --
> 2.45.2
>
Hi Jonathan,
On 01.10.24 11:28, Jonathan Wakely wrote:
> On Mon, 30 Sept 2024 at 18:26, Frank Scheiner wrote:
>> The following patch adds a workaround for this on the libstdc++
>> testsuite side.
>
> Thanks for the patch. Please CC libstdc++@gcc.gnu.org for all
> libstdc++ patches, as per https://gcc.gnu.org/lists.html
Thanks for the hint, will do. I also spotted a small typo in the title
("header" instead of "headers", as there are two glibc headers
involved), so I'll better send a V2 anyways.
> It looks like the glibc header also defines "bits" without using the
> implementation namespace. Is there a reason the re-added ia64 code in
> the glibc header can't be fixed to use __bits and __u?
No, I think we can do that. As it only affects C++ you proposed in [1]
to "just change the libstdc++ tests" though.
[1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654889.html
> Is there
> software expecting to access those fields directly?
Frankly, I have no idea. But I can run it through our toolchain
autobuilder (under CI on [2]) and in addition cross-build a T2 "base"
package selection ([3]) for ia64 with the changes included for "bits"
and "u" in the used glibc. The list of packages to be built is extensive
(as it also includes the "bootstrap" and "toolchain" lists).
If we don't spot a problem there, we might never do. Give me a few days
for the latter, because I never did that so far and it'll require two
runs to be able to compare the results.
[2]: http://epic-linux.org/
[3]: http://svn.exactcode.de/t2/trunk/target/generic/pkgsel/20-base.in
IIUC, if those changes in the glibc don't bite us, this patch here can
be dropped again, right? OTOH it seems to be like that in the glibc
since a while, so the gcc/libstdc++ patch could be kept for builds
against older glibc versions anyway. But let's first see what will happen...
Cheers,
Frank
On Tue, 1 Oct 2024 at 14:05, Frank Scheiner <frank.scheiner@web.de> wrote:
>
> Hi Jonathan,
>
> On 01.10.24 11:28, Jonathan Wakely wrote:
> > On Mon, 30 Sept 2024 at 18:26, Frank Scheiner wrote:
> >> The following patch adds a workaround for this on the libstdc++
> >> testsuite side.
> >
> > Thanks for the patch. Please CC libstdc++@gcc.gnu.org for all
> > libstdc++ patches, as per https://gcc.gnu.org/lists.html
>
> Thanks for the hint, will do. I also spotted a small typo in the title
> ("header" instead of "headers", as there are two glibc headers
> involved), so I'll better send a V2 anyways.
>
> > It looks like the glibc header also defines "bits" without using the
> > implementation namespace. Is there a reason the re-added ia64 code in
> > the glibc header can't be fixed to use __bits and __u?
>
> No, I think we can do that. As it only affects C++ you proposed in [1]
> to "just change the libstdc++ tests" though.
>
> [1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654889.html
Ah yes. I think the sys/ucontext.h case only affects C++ (and C when
_GNU_SOURCE is defined), but the sys/sigcontext.h case affects C too.
Both 'u' and 'bits' are used without the __ctx macro.
> > Is there
> > software expecting to access those fields directly?
>
> Frankly, I have no idea. But I can run it through our toolchain
> autobuilder (under CI on [2]) and in addition cross-build a T2 "base"
> package selection ([3]) for ia64 with the changes included for "bits"
> and "u" in the used glibc. The list of packages to be built is extensive
> (as it also includes the "bootstrap" and "toolchain" lists).
>
> If we don't spot a problem there, we might never do. Give me a few days
> for the latter, because I never did that so far and it'll require two
> runs to be able to compare the results.
>
> [2]: http://epic-linux.org/
>
> [3]: http://svn.exactcode.de/t2/trunk/target/generic/pkgsel/20-base.in
>
> IIUC, if those changes in the glibc don't bite us, this patch here can
> be dropped again, right? OTOH it seems to be like that in the glibc
> since a while, so the gcc/libstdc++ patch could be kept for builds
> against older glibc versions anyway. But let's first see what will happen...
I think we can change the libstdc++ test anyway, so it PASSes whether
or not any change happens for glibc.
Hi Jonathan, Joseph,
On 01.10.24 15:32, Jonathan Wakely wrote:
> On Tue, 1 Oct 2024 at 14:05, Frank Scheiner <frank.scheiner@web.de> wrote:
>> On 01.10.24 11:28, Jonathan Wakely wrote:
>>> On Mon, 30 Sept 2024 at 18:26, Frank Scheiner wrote:
>>> It looks like the glibc header also defines "bits" without using the
>>> implementation namespace. Is there a reason the re-added ia64 code in
>>> the glibc header can't be fixed to use __bits and __u?
>>
>> No, I think we can do that. As it only affects C++ you proposed in [1]
>> to "just change the libstdc++ tests" though.
>>
>> [1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654889.html
>
> Ah yes. I think the sys/ucontext.h case only affects C++ (and C when
> _GNU_SOURCE is defined), but the sys/sigcontext.h case affects C too.
> Both 'u' and 'bits' are used without the __ctx macro.
I looked at the error messages for the tests ([2]) again and for 'u',
even when used with the __ctx macro in "ucontext.h", it seems to be an
issue:
```
[...]
compiler exited with status 1
FAIL: 17_intro/names.cc -std=gnu++17 (test for excess errors)
Excess errors:
/usr/include/bits/sigcontext.h:37: error: expected unqualified-id before
';' token
/usr/include/bits/sigcontext.h:37: error: expected ')' before ';' token
/usr/include/sys/ucontext.h:44: error: expected unqualified-id before
';' token
/usr/include/sys/ucontext.h:44: error: expected ')' before ';' token
[...]
```by
[2]:
https://gcc.gnu.org/pipermail/gcc-patches/attachments/20240614/880a99c1/attachment-0001.md
Though I don't understand why. From the error message it sounds like 'u'
was replaced with '(' before the __ctx macro could do its job.
But Joseph also wrote that it "prepends __ in standards conformance
modes" in [3]. And this might not be the case for these specific tests,
so the __ctx macro might not have any effect here.
[3]: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654883.html
BTW, the two glibc headers are available here for reference:
*
https://github.com/linux-ia64/glibc-ia64/blob/release/2.39/master-w-ia64/sysdeps/unix/sysv/linux/ia64/bits/sigcontext.h
*
https://github.com/linux-ia64/glibc-ia64/blob/release/2.39/master-w-ia64/sysdeps/unix/sysv/linux/ia64/sys/ucontext.h
...names.cc here:
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/testsuite/17_intro/names.cc;h=9b0ffcb50b2e07a5ff2c2bd480b9eecd402c78d3;hb=HEAD
>> IIUC, if those changes in the glibc don't bite us, this patch here can
>> be dropped again, right? OTOH it seems to be like that in the glibc
>> since a while, so the gcc/libstdc++ patch could be kept for builds
>> against older glibc versions anyway. But let's first see what will happen...
>
>
> I think we can change the libstdc++ test anyway, so it PASSes whether
> or not any change happens for glibc.
Agreed.
Cheers,
Frank
On Tue, 1 Oct 2024 at 16:53, Frank Scheiner <frank.scheiner@web.de> wrote:
>
> Hi Jonathan, Joseph,
>
> On 01.10.24 15:32, Jonathan Wakely wrote:
> > On Tue, 1 Oct 2024 at 14:05, Frank Scheiner <frank.scheiner@web.de> wrote:
> >> On 01.10.24 11:28, Jonathan Wakely wrote:
> >>> On Mon, 30 Sept 2024 at 18:26, Frank Scheiner wrote:
> >>> It looks like the glibc header also defines "bits" without using the
> >>> implementation namespace. Is there a reason the re-added ia64 code in
> >>> the glibc header can't be fixed to use __bits and __u?
> >>
> >> No, I think we can do that. As it only affects C++ you proposed in [1]
> >> to "just change the libstdc++ tests" though.
> >>
> >> [1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654889.html
> >
> > Ah yes. I think the sys/ucontext.h case only affects C++ (and C when
> > _GNU_SOURCE is defined), but the sys/sigcontext.h case affects C too.
> > Both 'u' and 'bits' are used without the __ctx macro.
>
> I looked at the error messages for the tests ([2]) again and for 'u',
> even when used with the __ctx macro in "ucontext.h", it seems to be an
> issue:
>
> ```
> [...]
> compiler exited with status 1
> FAIL: 17_intro/names.cc -std=gnu++17 (test for excess errors)
> Excess errors:
> /usr/include/bits/sigcontext.h:37: error: expected unqualified-id before
> ';' token
> /usr/include/bits/sigcontext.h:37: error: expected ')' before ';' token
> /usr/include/sys/ucontext.h:44: error: expected unqualified-id before
> ';' token
> /usr/include/sys/ucontext.h:44: error: expected ')' before ';' token
> [...]
> ```by
>
> [2]:
> https://gcc.gnu.org/pipermail/gcc-patches/attachments/20240614/880a99c1/attachment-0001.md
>
> Though I don't understand why. From the error message it sounds like 'u'
> was replaced with '(' before the __ctx macro could do its job.
>
> But Joseph also wrote that it "prepends __ in standards conformance
> modes" in [3]. And this might not be the case for these specific tests,
> so the __ctx macro might not have any effect here.
It has no effect here. G++ unconditionally defines _GNU_SOURCE which
means the __ctx macro does nothing.
>
> [3]: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654883.html
>
> BTW, the two glibc headers are available here for reference:
>
> *
> https://github.com/linux-ia64/glibc-ia64/blob/release/2.39/master-w-ia64/sysdeps/unix/sysv/linux/ia64/bits/sigcontext.h
>
> *
> https://github.com/linux-ia64/glibc-ia64/blob/release/2.39/master-w-ia64/sysdeps/unix/sysv/linux/ia64/sys/ucontext.h
>
> ...names.cc here:
>
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/testsuite/17_intro/names.cc;h=9b0ffcb50b2e07a5ff2c2bd480b9eecd402c78d3;hb=HEAD
>
> >> IIUC, if those changes in the glibc don't bite us, this patch here can
> >> be dropped again, right? OTOH it seems to be like that in the glibc
> >> since a while, so the gcc/libstdc++ patch could be kept for builds
> >> against older glibc versions anyway. But let's first see what will happen...
> >
> >
> > I think we can change the libstdc++ test anyway, so it PASSes whether
> > or not any change happens for glibc.
>
> Agreed.
>
> Cheers,
> Frank
>
On 01.10.24 18:02, Jonathan Wakely wrote:
> On Tue, 1 Oct 2024 at 16:53, Frank Scheiner <frank.scheiner@web.de> wrote:
>> Though I don't understand why. From the error message it sounds like 'u'
>> was replaced with '(' before the __ctx macro could do its job.
>>
>> But Joseph also wrote that it "prepends __ in standards conformance
>> modes" in [3]. And this might not be the case for these specific tests,
>> so the __ctx macro might not have any effect here.
>
> It has no effect here. G++ unconditionally defines _GNU_SOURCE which
> means the __ctx macro does nothing.
Thanks for the clarification. I could retrace that via [1].
[1]:
https://github.com/linux-ia64/glibc-ia64/blob/release/2.39/master-w-ia64/include/features.h#L201
In the meantime I also ran the build tests (cross-build a T2 "base"
package selection) with and w/o modifications on the glibc side ('bits'
=> '__bits' and 'u' => '__u' in [2] and [3]) and couldn't find some
obvious differences.
[2]:
https://github.com/linux-ia64/glibc-ia64/blob/master-epic/sysdeps/unix/sysv/linux/ia64/bits/sigcontext.h
[3]:
https://github.com/linux-ia64/glibc-ia64/blob/master-epic/sysdeps/unix/sysv/linux/ia64/sys/ucontext.h
****
Jonathan, thanks for pointing me at these failing libstdc++ tests and
their possible reasons. I'll send v2 of the patch right away.
Cheers,
Frank
b/libstdc++-v3/testsuite/17_intro/names.cc
@@ -265,6 +265,12 @@
#undef j
#endif
+#if defined (__linux__) && defined (__ia64__)
+// <bits/sigcontext.h> defines __ia64_fpreg::u
+// <sys/ucontext.h> defines __ia64_fpreg_mcontext::u
+#undef u
+#endif
+
#if defined (__linux__) && defined (__powerpc__)
// <asm/types.h> defines __vector128::u
#undef u