[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 --- Comment #14 from Jakub Jelinek --- Fixed for 13.3 too.
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533
--- Comment #13 from GCC Commits ---
The releases/gcc-13 branch has been updated by Jakub Jelinek
:
https://gcc.gnu.org/g:cc39bd532d4de1ba0b2785246fb6fdd63ec2e92c
commit r13-8625-gcc39bd532d4de1ba0b2785246fb6fdd63ec2e92c
Author: Jakub Jelinek
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
#include
#include
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
Jakub Jelinek
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
(cherry picked from commit 8455d6f6cd43b7b143ab9ee19437452fceba9cc9)
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533
--- Comment #12 from GCC Commits ---
The master branch has been updated by Jakub Jelinek :
https://gcc.gnu.org/g:8455d6f6cd43b7b143ab9ee19437452fceba9cc9
commit r14-9769-g8455d6f6cd43b7b143ab9ee19437452fceba9cc9
Author: Jakub Jelinek
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
#include
#include
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
Jakub Jelinek
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
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 --- Comment #11 from Matthias Klose --- 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
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 Jakub Jelinek changed: What|Removed |Added Ever confirmed|0 |1 Status|UNCONFIRMED |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #10 from Jakub Jelinek --- Created attachment 57853 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57853=edit gcc14-pr114533.patch Untested fix. Unfortunately, we don't have any testsuite for libquadmath, hope it will be tested during libgfortran testing.
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 Jerry DeLisle changed: What|Removed |Added CC||jvdelisle at gcc dot gnu.org --- Comment #9 from Jerry DeLisle --- Adding myself here as I need hex format for gfortran EX format specifiers.
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 --- Comment #8 from Jakub Jelinek --- 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.
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 --- Comment #7 from Joseph S. Myers --- 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.
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 Andreas Schwab changed: What|Removed |Added Status|WAITING |UNCONFIRMED Ever confirmed|1 |0 --- Comment #6 from Andreas Schwab --- 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.
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 Andreas Schwab changed: What|Removed |Added Status|UNCONFIRMED |WAITING Last reconfirmed||2024-04-02 Ever confirmed|0 |1 --- Comment #5 from Andreas Schwab --- Without a test case it is hard to tell.
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 --- Comment #4 from Florian Weimer --- This looks like a glibc bug to me.
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 --- Comment #3 from Andreas Schwab --- Is the stack properly aligned at this point?
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 --- Comment #2 from Jakub Jelinek --- >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
[Bug libquadmath/114533] libquadmath: printf: fix misaligned access on args
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114533 Richard Biener changed: What|Removed |Added Keywords||ABI CC||jakub at gcc dot gnu.org, ||rguenth at gcc dot gnu.org --- Comment #1 from Richard Biener --- 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?
