reported at https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647635.html On x86, this compiles into movdqa which segfaults on unaligned access. This kind of failure has been seen when running against glibc 2.39, which incidentally changed the printf implementation to move away from alloca() for this data to instead append it at the end of an existing "scratch buffer", with arbitrary alignment, whereas alloca() was probably more likely to be naturally aligned. Tested by adding the patch to the Ubuntu gcc-14 package in https://launchpad.net/~schopin/+archive/ubuntu/libquadmath Signed-off-by: Simon Chopin <simon.chopin@canonical.com> --- libquadmath/printf/printf_fp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libquadmath/printf/printf_fp.c b/libquadmath/printf/printf_fp.c index 8effcee88fa..d86aa650d38 100644 --- a/libquadmath/printf/printf_fp.c +++ b/libquadmath/printf/printf_fp.c @@ -363,7 +363,7 @@ __quadmath_printf_fp (struct __quadmath_printf_file *fp, /* Fetch the argument value. */ { - fpnum = **(const __float128 **) args[0]; + memcpy(&fpnum, *(void* const *) args[0], sizeof(fpnum)); /* Check for special values: not a number or infinity. */ if (isnanq (fpnum))
The question is whether the caller misbehaves according to the ABI here? There's likely a known alignment present we could re-instantiate with a __builtin_assume_aligned?
From what I can see, glibc uses there the same thing as libquadmath does, so why is it ok on the glibc side and not on the libquadmath side? I mean https://sourceware.org/git/?p=glibc.git;a=blob;f=stdio-common/printf_fp.c;h=e75706f089bba3baabbcfb6bcf41514bad0a9dcb;hb=HEAD#l222 and https://sourceware.org/git/?p=glibc.git;a=blob;f=stdio-common/printf_fp.c;h=e75706f089bba3baabbcfb6bcf41514bad0a9dcb;hb=HEAD#l191
Is the stack properly aligned at this point?
This looks like a glibc bug to me.
Without a test case it is hard to tell.
Looks like the issue is that args_pa_user is not kept aligned. On the other hand, flt128_va already uses memcpy, so it does not expect the memory to be aligned.
Note also that in glibc, _Float128 support in printf code can only be used in limited circumstances: either on powerpc64le, as one of the multiple supported long double formats there, or through the sharing of the printf code with the implementation of strfromf128. In particular, there are no glibc printf formats corresponding directly to _FloatN / _FloatNx types. There was support in principle at the WG14 meeting in Strasbourg in January for having some form of printf/scanf support for such types in C2y, but major work is still needed on the wording that was proposed in N3184.
I guess we should go with the above patch after fixing formatting, but it isn't enough, printf_fphex.c has similar code. Even in glibc which doesn't support printing _Float128 nor any other type which would require similar alignment, the hooks only register a function to fill in some mem and allows specification of size, but can't specify alignment.
Adding myself here as I need hex format for gfortran EX format specifiers.
Created attachment 57853 [details] gcc14-pr114533.patch Untested fix. Unfortunately, we don't have any testsuite for libquadmath, hope it will be tested during libgfortran testing.
while not a test case, that was seen when running autopkg tests of the evolver package against glibc 2.39 packages. https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/2052929 the failing evolver test is: echo "g 5; v; r ; g 10; v;" | evolver-nox-q cube
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:8455d6f6cd43b7b143ab9ee19437452fceba9cc9 commit r14-9769-g8455d6f6cd43b7b143ab9ee19437452fceba9cc9 Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Apr 3 10:02:35 2024 +0200 libquadmath: Don't assume the storage for __float128 arguments is aligned [PR114533] With the register_printf_type/register_printf_modifier/register_printf_specifier APIs the C library is just told the size of the argument and is provided with a callback to fetch the argument from va_list using va_arg into C library provided memory. The C library isn't told what alignment requirement it has, but we were using direct load of a __float128 value from that memory which assumes __alignof (__float128) alignment. The following patch fixes that by using memcpy instead. I haven't been able to reproduce an actual crash, tried #include <quadmath.h> #include <stdlib.h> #include <stdio.h> int main () { __float128 r; int prec = 20; int width = 46; char buf[128]; r = 2.0q; r = sqrtq (r); int n = quadmath_snprintf (buf, sizeof buf, "%+-#*.20Qe", width, r); if ((size_t) n < sizeof buf) printf ("%s\n", buf); /* Prints: +1.41421356237309504880e+00 */ quadmath_snprintf (buf, sizeof buf, "%Qa", r); if ((size_t) n < sizeof buf) printf ("%s\n", buf); /* Prints: 0x1.6a09e667f3bcc908b2fb1366ea96p+0 */ n = quadmath_snprintf (NULL, 0, "%+-#46.*Qe", prec, r); if (n > -1) { char *str = malloc (n + 1); if (str) { quadmath_snprintf (str, n + 1, "%+-#46.*Qe", prec, r); printf ("%s\n", str); /* Prints: +1.41421356237309504880e+00 */ } free (str); } printf ("%+-#*.20Qe\n", width, r); printf ("%Qa\n", r); printf ("%+-#46.*Qe\n", prec, r); printf ("%d %Qe %d %Qe %d %Qe\n", 1, r, 2, r, 3, r); return 0; } In any case, I think memcpy for loading from it is right. 2024-04-03 Simon Chopin <simon.chopin@canonical.com> Jakub Jelinek <jakub@redhat.com> PR libquadmath/114533 * printf/printf_fp.c (__quadmath_printf_fp): Use memcpy to copy __float128 out of args. * printf/printf_fphex.c (__quadmath_printf_fphex): Likewise. Signed-off-by: Simon Chopin <simon.chopin@canonical.com>
The releases/gcc-13 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:cc39bd532d4de1ba0b2785246fb6fdd63ec2e92c commit r13-8625-gcc39bd532d4de1ba0b2785246fb6fdd63ec2e92c Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Apr 3 10:02:35 2024 +0200 libquadmath: Don't assume the storage for __float128 arguments is aligned [PR114533] With the register_printf_type/register_printf_modifier/register_printf_specifier APIs the C library is just told the size of the argument and is provided with a callback to fetch the argument from va_list using va_arg into C library provided memory. The C library isn't told what alignment requirement it has, but we were using direct load of a __float128 value from that memory which assumes __alignof (__float128) alignment. The following patch fixes that by using memcpy instead. I haven't been able to reproduce an actual crash, tried #include <quadmath.h> #include <stdlib.h> #include <stdio.h> int main () { __float128 r; int prec = 20; int width = 46; char buf[128]; r = 2.0q; r = sqrtq (r); int n = quadmath_snprintf (buf, sizeof buf, "%+-#*.20Qe", width, r); if ((size_t) n < sizeof buf) printf ("%s\n", buf); /* Prints: +1.41421356237309504880e+00 */ quadmath_snprintf (buf, sizeof buf, "%Qa", r); if ((size_t) n < sizeof buf) printf ("%s\n", buf); /* Prints: 0x1.6a09e667f3bcc908b2fb1366ea96p+0 */ n = quadmath_snprintf (NULL, 0, "%+-#46.*Qe", prec, r); if (n > -1) { char *str = malloc (n + 1); if (str) { quadmath_snprintf (str, n + 1, "%+-#46.*Qe", prec, r); printf ("%s\n", str); /* Prints: +1.41421356237309504880e+00 */ } free (str); } printf ("%+-#*.20Qe\n", width, r); printf ("%Qa\n", r); printf ("%+-#46.*Qe\n", prec, r); printf ("%d %Qe %d %Qe %d %Qe\n", 1, r, 2, r, 3, r); return 0; } In any case, I think memcpy for loading from it is right. 2024-04-03 Simon Chopin <simon.chopin@canonical.com> Jakub Jelinek <jakub@redhat.com> PR libquadmath/114533 * printf/printf_fp.c (__quadmath_printf_fp): Use memcpy to copy __float128 out of args. * printf/printf_fphex.c (__quadmath_printf_fphex): Likewise. Signed-off-by: Simon Chopin <simon.chopin@canonical.com> (cherry picked from commit 8455d6f6cd43b7b143ab9ee19437452fceba9cc9)
Fixed for 13.3 too.
The releases/gcc-12 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:9987fe67cf6211515d8ebf6528cc83c77dfb5bf3 commit r12-10517-g9987fe67cf6211515d8ebf6528cc83c77dfb5bf3 Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Apr 3 10:02:35 2024 +0200 libquadmath: Don't assume the storage for __float128 arguments is aligned [PR114533] With the register_printf_type/register_printf_modifier/register_printf_specifier APIs the C library is just told the size of the argument and is provided with a callback to fetch the argument from va_list using va_arg into C library provided memory. The C library isn't told what alignment requirement it has, but we were using direct load of a __float128 value from that memory which assumes __alignof (__float128) alignment. The following patch fixes that by using memcpy instead. I haven't been able to reproduce an actual crash, tried #include <quadmath.h> #include <stdlib.h> #include <stdio.h> int main () { __float128 r; int prec = 20; int width = 46; char buf[128]; r = 2.0q; r = sqrtq (r); int n = quadmath_snprintf (buf, sizeof buf, "%+-#*.20Qe", width, r); if ((size_t) n < sizeof buf) printf ("%s\n", buf); /* Prints: +1.41421356237309504880e+00 */ quadmath_snprintf (buf, sizeof buf, "%Qa", r); if ((size_t) n < sizeof buf) printf ("%s\n", buf); /* Prints: 0x1.6a09e667f3bcc908b2fb1366ea96p+0 */ n = quadmath_snprintf (NULL, 0, "%+-#46.*Qe", prec, r); if (n > -1) { char *str = malloc (n + 1); if (str) { quadmath_snprintf (str, n + 1, "%+-#46.*Qe", prec, r); printf ("%s\n", str); /* Prints: +1.41421356237309504880e+00 */ } free (str); } printf ("%+-#*.20Qe\n", width, r); printf ("%Qa\n", r); printf ("%+-#46.*Qe\n", prec, r); printf ("%d %Qe %d %Qe %d %Qe\n", 1, r, 2, r, 3, r); return 0; } In any case, I think memcpy for loading from it is right. 2024-04-03 Simon Chopin <simon.chopin@canonical.com> Jakub Jelinek <jakub@redhat.com> PR libquadmath/114533 * printf/printf_fp.c (__quadmath_printf_fp): Use memcpy to copy __float128 out of args. * printf/printf_fphex.c (__quadmath_printf_fphex): Likewise. Signed-off-by: Simon Chopin <simon.chopin@canonical.com> (cherry picked from commit 8455d6f6cd43b7b143ab9ee19437452fceba9cc9)
Should be fixed for 12.4+ too.
The releases/gcc-11 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:df41dbfd22528b1241668af21a979204b876fb67 commit r11-11500-gdf41dbfd22528b1241668af21a979204b876fb67 Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Apr 3 10:02:35 2024 +0200 libquadmath: Don't assume the storage for __float128 arguments is aligned [PR114533] With the register_printf_type/register_printf_modifier/register_printf_specifier APIs the C library is just told the size of the argument and is provided with a callback to fetch the argument from va_list using va_arg into C library provided memory. The C library isn't told what alignment requirement it has, but we were using direct load of a __float128 value from that memory which assumes __alignof (__float128) alignment. The following patch fixes that by using memcpy instead. I haven't been able to reproduce an actual crash, tried #include <quadmath.h> #include <stdlib.h> #include <stdio.h> int main () { __float128 r; int prec = 20; int width = 46; char buf[128]; r = 2.0q; r = sqrtq (r); int n = quadmath_snprintf (buf, sizeof buf, "%+-#*.20Qe", width, r); if ((size_t) n < sizeof buf) printf ("%s\n", buf); /* Prints: +1.41421356237309504880e+00 */ quadmath_snprintf (buf, sizeof buf, "%Qa", r); if ((size_t) n < sizeof buf) printf ("%s\n", buf); /* Prints: 0x1.6a09e667f3bcc908b2fb1366ea96p+0 */ n = quadmath_snprintf (NULL, 0, "%+-#46.*Qe", prec, r); if (n > -1) { char *str = malloc (n + 1); if (str) { quadmath_snprintf (str, n + 1, "%+-#46.*Qe", prec, r); printf ("%s\n", str); /* Prints: +1.41421356237309504880e+00 */ } free (str); } printf ("%+-#*.20Qe\n", width, r); printf ("%Qa\n", r); printf ("%+-#46.*Qe\n", prec, r); printf ("%d %Qe %d %Qe %d %Qe\n", 1, r, 2, r, 3, r); return 0; } In any case, I think memcpy for loading from it is right. 2024-04-03 Simon Chopin <simon.chopin@canonical.com> Jakub Jelinek <jakub@redhat.com> PR libquadmath/114533 * printf/printf_fp.c (__quadmath_printf_fp): Use memcpy to copy __float128 out of args. * printf/printf_fphex.c (__quadmath_printf_fphex): Likewise. Signed-off-by: Simon Chopin <simon.chopin@canonical.com> (cherry picked from commit 8455d6f6cd43b7b143ab9ee19437452fceba9cc9)
Fixed for 11.5 as well.